Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1381

Function quantifier should be retained in a cloned Sql call

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8.0, 1.6.0
    • Fix Version/s: 1.10.0
    • Component/s: core
    • Labels:
      None

      Description

      I found Calcite can’t execute queries like select nullif(count(distinct colunm),0) from table, and dumped ‘Plan after trimming unused fields’ like this:

      LogicalProject(NAME=[$0], GENDER=[CASE(=($1, 0), null, CAST($2):BIGINT)])
        LogicalAggregate(group=[{0}], agg#0=[COUNT(DISTINCT $1)], agg#1=[COUNT($1)])
          LogicalProject(NAME=[$1], GENDER=[$3])
            CsvTableScan(table=[[SALES, EMPS]], fields=[[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]])
      

      Then, I checked SqlNullifFunction rewriteCall method which used SqlNode.clone(SqlParserPos pos) method to create SqlCase call.

      Finally, I guess the root cause may be that the clone(SqlParserPos pos) method discard the functionQuantifier which holds such as ‘distinct’ symbol when create new SqlCall instance.

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        Thanks for the patch.

        I think you should fix SqlCall.clone as well as SqlBasicCall.clone.

        We need a test case. Maybe add a query in an existing .iq file.

        Show
        julianhyde Julian Hyde added a comment - Thanks for the patch. I think you should fix SqlCall.clone as well as SqlBasicCall.clone. We need a test case. Maybe add a query in an existing .iq file.
        Hide
        zhengd zhengdong added a comment -

        Sorry for misunderstanding your advice in email.
        Could you help to review the new patch file I have attached?

        And I don't have any knowledge of how to use .iq file. Could you help to add the test case or explain further more about how should I do?

        Show
        zhengd zhengdong added a comment - Sorry for misunderstanding your advice in email. Could you help to review the new patch file I have attached? And I don't have any knowledge of how to use .iq file. Could you help to add the test case or explain further more about how should I do?
        Hide
        julianhyde Julian Hyde added a comment -

        The patch looks good.

        .iq files are run using an engine called Quidem and are all invoked from QuidemTest. But of course you don't have to use Quidem; you can add a case to any of the existing tests.

        Show
        julianhyde Julian Hyde added a comment - The patch looks good. .iq files are run using an engine called Quidem and are all invoked from QuidemTest . But of course you don't have to use Quidem; you can add a case to any of the existing tests.
        Hide
        zhengd zhengdong added a comment -

        I have added the test sql in agg.iq file and resubmited the patch file. Thanks Julian!

        Show
        zhengd zhengdong added a comment - I have added the test sql in agg.iq file and resubmited the patch file. Thanks Julian!
        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/8d9a5d24 . Thanks for the patch, zhengdong !
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Resolved in release 1.10.0 (2016-10-12).

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.10.0 (2016-10-12).

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            zhengd zhengdong
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development