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

Scalar sub-query and aggregate function in SELECT or HAVING clause gives AssertionError

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.0-incubating
    • Fix Version/s: 1.3.0-incubating
    • Component/s: None
    • Labels:
      None

      Description

      This may be a regression caused by CALCITE-694. Following query hits an AssertionError:

       SELECT emp.deptno FROM emp group by emp.deptno having max(emp.empno) > (select min(emp.empno) from emp);
      

      Stack trace:

      java.lang.AssertionError: null
      	at org.apache.calcite.sql2rel.RelStructuredTypeFlattener.getNewForOldInput(RelStructuredTypeFlattener.java:291)
      	at org.apache.calcite.sql2rel.RelStructuredTypeFlattener$RewriteRexShuttle.visitInputRef(RelStructuredTypeFlattener.java:725)
      	at org.apache.calcite.sql2rel.RelStructuredTypeFlattener$RewriteRexShuttle.visitInputRef(RelStructuredTypeFlattener.java:722)
      	at org.apache.calcite.rex.RexInputRef.accept(RexInputRef.java:112)
      	at org.apache.calcite.rex.RexShuttle.visitList(RexShuttle.java:134)
      	at org.apache.calcite.rex.RexShuttle.visitCall(RexShuttle.java:83)
      	at org.apache.calcite.sql2rel.RelStructuredTypeFlattener$RewriteRexShuttle.visitCall(RelStructuredTypeFlattener.java:795)
      	at org.apache.calcite.sql2rel.RelStructuredTypeFlattener$RewriteRexShuttle.visitCall(RelStructuredTypeFlattener.java:722)
      

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        Resolved in release 1.3.0-incubating (2015-05-30).

        Show
        julianhyde Julian Hyde added a comment - Resolved in release 1.3.0-incubating (2015-05-30).
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/f98d567f .
        Hide
        julianhyde Julian Hyde added a comment -

        I've made that exact fix in https://github.com/julianhyde/incubator-calcite/commit/f98d567fa6843762a830a272e06f2d74908ce440 and am now testing. You were right to introduce AggFinder.

        Show
        julianhyde Julian Hyde added a comment - I've made that exact fix in https://github.com/julianhyde/incubator-calcite/commit/f98d567fa6843762a830a272e06f2d74908ce440 and am now testing. You were right to introduce AggFinder.
        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        Or, another proposal could be using AggFinder visitor for both select & having clause.
        Julian Hyde So would you like me to work on that?

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - Or, another proposal could be using AggFinder visitor for both select & having clause. Julian Hyde So would you like me to work on that?
        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        That makes sense. I can keep working on this one.

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - That makes sense. I can keep working on this one.
        Hide
        julianhyde Julian Hyde added a comment -

        I figured that what is good for the SELECT clause should be good for the HAVING clause. There is no reason to use AggFunctionsFinder for HAVING but not SELECT. Sure enough, Sean Hsuan-Yi Chu's original patch fixes

        SELECT emp.deptno FROM emp group by emp.deptno having max(emp.empno) > (select min(emp.empno) from emp);

        but the problem still exists for

        SELECT emp.deptno, max(emp.empno) > (select min(emp.empno) from emp) as b FROM emp group by emp.deptno;

        Can we work on a fix for that too?

        Show
        julianhyde Julian Hyde added a comment - I figured that what is good for the SELECT clause should be good for the HAVING clause. There is no reason to use AggFunctionsFinder for HAVING but not SELECT. Sure enough, Sean Hsuan-Yi Chu 's original patch fixes SELECT emp.deptno FROM emp group by emp.deptno having max(emp.empno) > (select min(emp.empno) from emp); but the problem still exists for SELECT emp.deptno, max(emp.empno) > (select min(emp.empno) from emp) as b FROM emp group by emp.deptno; Can we work on a fix for that too?
        Hide
        amansinha100 Aman Sinha added a comment -

        Since 1.3 is imminent, we could keep Sean Hsuan-Yi Chu's original patch for CALCITE-694 and refine it post 1.3 if needed.

        Show
        amansinha100 Aman Sinha added a comment - Since 1.3 is imminent, we could keep Sean Hsuan-Yi Chu 's original patch for CALCITE-694 and refine it post 1.3 if needed.
        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        1. This issue did not happen before the commit of CALCITE-694.

        2. I tried this same query on my original patch. It worked out.

        So there must be a fundamental difference between
        "adding the whole HAVING" and "using visitor to take individual aggregate functions only"

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - 1. This issue did not happen before the commit of CALCITE-694 . 2. I tried this same query on my original patch. It worked out. So there must be a fundamental difference between "adding the whole HAVING" and "using visitor to take individual aggregate functions only"
        Hide
        julianhyde Julian Hyde added a comment -

        Sean Hsuan-Yi Chu Can you investigate this? I would like to know ASAP whether this worked before your patch for CALCITE-694. I want to know whether we should back out your patch.

        If you can fix this before 1.3 (we plan to stabilize tomorrow, freeze in a few days) that would be even better.

        Show
        julianhyde Julian Hyde added a comment - Sean Hsuan-Yi Chu Can you investigate this? I would like to know ASAP whether this worked before your patch for CALCITE-694 . I want to know whether we should back out your patch. If you can fix this before 1.3 (we plan to stabilize tomorrow, freeze in a few days) that would be even better.

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            amansinha100 Aman Sinha
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development