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

ReduceExpressionsRule tries to reduce SemiJoin condition to non-equi condition

    Details

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

      Description

      Hive's unit test that reproduces the problem: vector_mapjoin_reduce
      Stack Trace:

      Unexpected exception java.lang.AssertionError
      	at org.apache.calcite.rel.core.SemiJoin.copy(SemiJoin.java:82)
      	at org.apache.calcite.rel.core.SemiJoin.copy(SemiJoin.java:42)
      	at org.apache.calcite.rel.rules.ReduceExpressionsRule$3.onMatch(ReduceExpressionsRule.java:224)
      	at org.apache.calcite.plan.AbstractRelOptPlanner.fireRule(AbstractRelOptPlanner.java:326)
      	at org.apache.calcite.plan.hep.HepPlanner.applyRule(HepPlanner.java:515)
      	at org.apache.calcite.plan.hep.HepPlanner.applyRules(HepPlanner.java:392)
      	at org.apache.calcite.plan.hep.HepPlanner.executeInstruction(HepPlanner.java:285)
      	at org.apache.calcite.plan.hep.HepInstruction$RuleCollection.execute(HepInstruction.java:72)
      	at org.apache.calcite.plan.hep.HepPlanner.executeProgram(HepPlanner.java:207)
      	at org.apache.calcite.plan.hep.HepPlanner.findBestExp(HepPlanner.java:194)
      

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        ReduceExpressionsRule tries to reduce SemiJoin condition to non-equi condition. If the reduced condition is no longer an equi-condition (e.g. "l.deptno = r.deptno" becomes "l.deptno = 100" because we have a predicate "r.deptno = 100") then the rule should not try to change the SemiJoin.

        The same applies to other types of join that must be equi-joins.

        Show
        julianhyde Julian Hyde added a comment - ReduceExpressionsRule tries to reduce SemiJoin condition to non-equi condition. If the reduced condition is no longer an equi-condition (e.g. "l.deptno = r.deptno" becomes "l.deptno = 100" because we have a predicate "r.deptno = 100") then the rule should not try to change the SemiJoin. The same applies to other types of join that must be equi-joins.
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/f036de87 .
        Hide
        vladimirsitnikov Vladimir Sitnikov added a comment -

        Julian,

        I suppose your fix is a workaround.
        I prefer we would either create a new ticket or at least leave a relevant comment in such cases.

        It would be nice if Calcite could propagate dpetno=200 to the e2, however the expected plan you've added does not include that.

        select e1.sal from (select * from emp where deptno = 200) as e1
        where e1.deptno in (
          select e2.deptno from emp e2 where e2.sal = 100)
        Show
        vladimirsitnikov Vladimir Sitnikov added a comment - Julian, I suppose your fix is a workaround. I prefer we would either create a new ticket or at least leave a relevant comment in such cases. It would be nice if Calcite could propagate dpetno=200 to the e2, however the expected plan you've added does not include that. select e1.sal from ( select * from emp where deptno = 200) as e1 where e1.deptno in ( select e2.deptno from emp e2 where e2.sal = 100)
        Hide
        julianhyde Julian Hyde added a comment -

        I was approaching it as a bug - it used to throw AssertionError, and after my fix it does not. Functional improvements should be logged as separate jira cases.

        In general I'm not in favor of TODO comments in the code. They go stale and confuse people. And, there is a temptation to use a TODO as an excuse to commit incomplete functionality. The place for wish lists is Jira.

        Show
        julianhyde Julian Hyde added a comment - I was approaching it as a bug - it used to throw AssertionError, and after my fix it does not. Functional improvements should be logged as separate jira cases. In general I'm not in favor of TODO comments in the code. They go stale and confuse people. And, there is a temptation to use a TODO as an excuse to commit incomplete functionality. The place for wish lists is Jira.
        Hide
        julianhyde Julian Hyde added a comment -

        Closing now that 1.0.0-incubating has been released.

        Show
        julianhyde Julian Hyde added a comment - Closing now that 1.0.0-incubating has been released.

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            jcamachorodriguez Jesus Camacho Rodriguez
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development