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

Predicate Pull up above Project enhancement

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.0
    • Fix Version/s: 1.6.0
    • Component/s: core
    • Labels:
      None

      Description

      Predicate Pull up on Project can also pull up deterministic functions whose arguments are all literals.

      Ex: select r1.x from (select cast('10' as int) as x, y from r1 where y<10)r1 join r2 on r1.x=r2.x;

      Currently deterministic functions involving constants are ignored which results in missed transitive inference.

        Issue Links

          Activity

          Hide
          jni Jinfeng Ni added a comment -

          Is it possible to apply ReduceExpression to those constant expressions first, such that in the above case, the project just has the int literal of 10?

          Show
          jni Jinfeng Ni added a comment - Is it possible to apply ReduceExpression to those constant expressions first, such that in the above case, the project just has the int literal of 10?
          Hide
          jpullokkaran Laljo John Pullokkaran added a comment -

          Cast of literal is not reduced.
          Also this is more generic; for udf's/generic functions it is useful to capture the equivalence and propagate it.

          Show
          jpullokkaran Laljo John Pullokkaran added a comment - Cast of literal is not reduced. Also this is more generic; for udf's/generic functions it is useful to capture the equivalence and propagate it.
          Hide
          julianhyde Julian Hyde added a comment -

          I agree with Laljo John Pullokkaran on the need to form predicates on constant expressions, not just atoms.

          Your code is similar to RexProgram.isConstant(RexNode), except that it does not occur within the context of a RexProgram. I think you should move your isDeterministicFuncOnLiterals method to RexUtil, rename it isConstant, and use a static inner class very similar to RexProgram.ConstantFinder. Note that ConstantFinder returns Boolean values rather than throwing (which will satisfy Vladimir Sitnikov's concerns) and also treats references to bind variables as constants (which I also think is a good thing).

          Show
          julianhyde Julian Hyde added a comment - I agree with Laljo John Pullokkaran on the need to form predicates on constant expressions, not just atoms. Your code is similar to RexProgram.isConstant(RexNode) , except that it does not occur within the context of a RexProgram . I think you should move your isDeterministicFuncOnLiterals method to RexUtil , rename it isConstant , and use a static inner class very similar to RexProgram.ConstantFinder . Note that ConstantFinder returns Boolean values rather than throwing (which will satisfy Vladimir Sitnikov 's concerns) and also treats references to bind variables as constants (which I also think is a good thing).
          Hide
          julianhyde Julian Hyde added a comment -

          Laljo John Pullokkaran, PR looks good. I rebased and did some minor fix up, including addressing Vladimir Sitnikov's concern. Please review https://github.com/julianhyde/calcite/tree/993-pull-up.

          Show
          julianhyde Julian Hyde added a comment - Laljo John Pullokkaran , PR looks good. I rebased and did some minor fix up, including addressing Vladimir Sitnikov 's concern. Please review https://github.com/julianhyde/calcite/tree/993-pull-up .
          Hide
          jpullokkaran Laljo John Pullokkaran added a comment -

          Julian Hyde Yes that looks good.

          Show
          jpullokkaran Laljo John Pullokkaran added a comment - Julian Hyde Yes that looks good.
          Hide
          julianhyde Julian Hyde added a comment -

          I just realized there's a problem. Suppose that the expression is constant but evaluates to null. For example,

          SELECT NULLIF(1, 1) AS c FROM t

          It is not valid to infer the predicate c = NULLIF(1, 1). Only c IS NOT DISTINCT FROM NULLIF(1, 1) is valid. But I bet that we can't use predicates based on IS NOT DISTINCT FROM as widely as we can use =.

          Show
          julianhyde Julian Hyde added a comment - I just realized there's a problem. Suppose that the expression is constant but evaluates to null. For example, SELECT NULLIF(1, 1) AS c FROM t It is not valid to infer the predicate c = NULLIF(1, 1) . Only c IS NOT DISTINCT FROM NULLIF(1, 1) is valid. But I bet that we can't use predicates based on IS NOT DISTINCT FROM as widely as we can use = .
          Hide
          jpullokkaran Laljo John Pullokkaran added a comment - - edited

          Julian Hyde You are right; we would need to handle NULL generating Functions differently.
          The problem is more wider it seems.
          "Case Stmts", UDFs that may generate NULL.

          May be we should restrict this to builtin functions that can be guaranteed to produce non null values given non null literals.

          I will work on it over the weekend.

          Show
          jpullokkaran Laljo John Pullokkaran added a comment - - edited Julian Hyde You are right; we would need to handle NULL generating Functions differently. The problem is more wider it seems. "Case Stmts", UDFs that may generate NULL. May be we should restrict this to builtin functions that can be guaranteed to produce non null values given non null literals. I will work on it over the weekend.
          Hide
          julianhyde Julian Hyde added a comment -

          I have a solution in the branch. If we deduce the type as NOT NULL we will generate '=', otherwise generate 'IS NOT DISTINCT FROM'. We can (and do) deduce NOT NULL in the main use case, namely CAST. So, IS NOT DISTINCT FROM will not occur much in practice.

          Show
          julianhyde Julian Hyde added a comment - I have a solution in the branch. If we deduce the type as NOT NULL we will generate '=', otherwise generate 'IS NOT DISTINCT FROM'. We can (and do) deduce NOT NULL in the main use case, namely CAST. So, IS NOT DISTINCT FROM will not occur much in practice.
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/9be2bdb9 , with some clean up in http://git-wip-us.apache.org/repos/asf/calcite/commit/8281668f .
          Hide
          julianhyde Julian Hyde added a comment -

          Resolved in release 1.6.0 (2016-01-22).

          Show
          julianhyde Julian Hyde added a comment - Resolved in release 1.6.0 (2016-01-22).

            People

            • Assignee:
              jpullokkaran Laljo John Pullokkaran
              Reporter:
              jpullokkaran Laljo John Pullokkaran
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development