You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by za...@apache.org on 2023/04/24 20:36:26 UTC

[calcite] branch main updated: [CALICTE-5646] JoinDeriveIsNotNullFilterRule incorrectly handles COALESCE in join condition

This is an automated email from the ASF dual-hosted git repository.

zabetak pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
     new 9360b889db [CALICTE-5646] JoinDeriveIsNotNullFilterRule incorrectly handles COALESCE in join condition
9360b889db is described below

commit 9360b889dbe1742896745366920e5aa77436ba5e
Author: Leonid Chistov <lc...@querifylabs.com>
AuthorDate: Fri Apr 14 12:48:14 2023 +0300

    [CALICTE-5646] JoinDeriveIsNotNullFilterRule incorrectly handles COALESCE in join condition
    
    Consider nullability of inputs in isolation for inferring strongness of
    the join predicate. Inferring nullability from all inputs leads to
    wrong transformation as demonstrated in the test cases.
    
    Use Strong#isNotTrue instead of Strong#isNull for enlarging the
    transformation scope. In theory, this allows the transformation to be
    applied even when predicate is FALSE and not only when UNKNOWN.
    
    Close apache/calcite#3149
---
 .../main/java/org/apache/calcite/plan/Strong.java  |  9 +--
 .../rel/rules/JoinDeriveIsNotNullFilterRule.java   |  6 +-
 .../org/apache/calcite/test/RelOptRulesTest.java   | 27 +++++++++
 .../org/apache/calcite/test/RelOptRulesTest.xml    | 67 ++++++++++++++++++++++
 4 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/Strong.java b/core/src/main/java/org/apache/calcite/plan/Strong.java
index 5e6612d520..935e7c8f6a 100644
--- a/core/src/main/java/org/apache/calcite/plan/Strong.java
+++ b/core/src/main/java/org/apache/calcite/plan/Strong.java
@@ -36,8 +36,8 @@ import java.util.Map;
 
 /** Utilities for strong predicates.
  *
- * <p>A predicate is strong (or null-rejecting) if it is UNKNOWN if any of its
- * inputs is UNKNOWN.</p>
+ * <p>A predicate is strong (or null-rejecting) with regards to selected subset of inputs
+ * if it is UNKNOWN if all inputs in selected subset are UNKNOWN.</p>
  *
  * <p>By the way, UNKNOWN is just the boolean form of NULL.</p>
  *
@@ -79,7 +79,7 @@ public class Strong {
   }
 
   /** Returns whether the analyzed expression will definitely not return true
-   * (equivalently, will definitely not return null or false) if
+   * (equivalently, will definitely return null or false) if
    * all of a given set of input columns are null. */
   public static boolean isNotTrue(RexNode node, ImmutableBitSet nullColumns) {
     return of(nullColumns).isNotTrue(node);
@@ -149,7 +149,8 @@ public class Strong {
     return operands.stream().allMatch(Strong::isStrong);
   }
 
-  /** Returns whether an expression is definitely not true. */
+  /** Returns whether the analyzed expression will definitely not return true
+   * (equivalently, will definitely return null or false). */
   public boolean isNotTrue(RexNode node) {
     switch (node.getKind()) {
     //TODO Enrich with more possible cases?
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/JoinDeriveIsNotNullFilterRule.java b/core/src/main/java/org/apache/calcite/rel/rules/JoinDeriveIsNotNullFilterRule.java
index bb57f79b3e..24074afddb 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/JoinDeriveIsNotNullFilterRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/JoinDeriveIsNotNullFilterRule.java
@@ -62,9 +62,9 @@ public class JoinDeriveIsNotNullFilterRule
     final RelMetadataQuery mq = call.getMetadataQuery();
 
     final ImmutableBitSet.Builder notNullableKeys = ImmutableBitSet.builder();
-    RelOptUtil.conjunctions(join.getCondition()).forEach(node -> {
-      if (Strong.isStrong(node)) {
-        notNullableKeys.addAll(RelOptUtil.InputFinder.bits(node));
+    RelOptUtil.InputFinder.bits(join.getCondition()).forEach(bit -> {
+      if (Strong.isNotTrue(join.getCondition(), ImmutableBitSet.of(bit))) {
+        notNullableKeys.set(bit);
       }
     });
     final List<Integer> leftKeys = new ArrayList<>();
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index ef814a91dd..4fd8e82711 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -530,6 +530,33 @@ class RelOptRulesTest extends RelOptTestBase {
     sql(sql).withRule(CoreRules.JOIN_DERIVE_IS_NOT_NULL_FILTER_RULE).check();
   }
 
+  /** As {@link #testJoinDeriveIsNotNullFilterRule1()};
+   *  should not create IS NOT NULL filter if join condition is not strong wrt each key. */
+  @Test void testJoinDeriveIsNotNullFilterRule13() {
+    final String sql = "select t1.deptno from empnullables t1 inner join\n"
+        + "empnullables t2 on coalesce(t1.ename, t2.ename) = 'abc'";
+    sql(sql).withRule(CoreRules.JOIN_DERIVE_IS_NOT_NULL_FILTER_RULE).checkUnchanged();
+  }
+
+  /** As {@link #testJoinDeriveIsNotNullFilterRule1()};
+   *  should not create IS NOT NULL filter if join condition is not strong wrt each key. */
+  @Test void testJoinDeriveIsNotNullFilterRule14() {
+    final String sql = "select t1.deptno from empnullables t1 inner join\n"
+        + "empnullables t2 on nvl(t1.ename, t2.ename) = 'abc'";
+    sql(sql)
+        .withFactory(t ->
+            t.withOperatorTable(opTab -> SqlValidatorTest.operatorTableFor(SqlLibrary.ORACLE)))
+        .withRule(CoreRules.JOIN_DERIVE_IS_NOT_NULL_FILTER_RULE).checkUnchanged();
+  }
+
+  /** As {@link #testJoinDeriveIsNotNullFilterRule1()};
+   *  should create IS NOT NULL filter only for the first operand of NULLIF. */
+  @Test void testJoinDeriveIsNotNullFilterRule15() {
+    final String sql = "select t1.deptno from empnullables t1 inner join\n"
+        + "empnullables t2 on nullif(t1.ename, t2.ename) = 'abc'";
+    sql(sql).withRule(CoreRules.JOIN_DERIVE_IS_NOT_NULL_FILTER_RULE).check();
+  }
+
   @Test void testStrengthenJoinType() {
     // The "Filter(... , right.c IS NOT NULL)" above a left join is pushed into
     // the join, makes it an inner join, and then disappears because c is NOT
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index 162e3f35c9..91c54a496b 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -4995,6 +4995,73 @@ LogicalProject(DEPTNO=[$7])
       LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
     LogicalFilter(condition=[IS NOT NULL($1)])
       LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testJoinDeriveIsNotNullFilterRule13">
+    <Resource name="sql">
+      <![CDATA[select t1.deptno from empnullables t1 inner join
+empnullables t2 on coalesce(t1.ename, t2.ename) = 'abc']]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(DEPTNO=[$7])
+  LogicalJoin(condition=[=(CASE(IS NOT NULL($1), $1, $10), 'abc')], joinType=[inner])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+]]>
+    </Resource>
+    <Resource name="planAfter">
+      <![CDATA[
+LogicalProject(DEPTNO=[$7])
+  LogicalJoin(condition=[=(CASE(IS NOT NULL($1), $1, $10), 'abc')], joinType=[inner])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testJoinDeriveIsNotNullFilterRule14">
+    <Resource name="sql">
+      <![CDATA[select t1.deptno from empnullables t1 inner join
+empnullables t2 on nvl(t1.ename, t2.ename) = 'abc']]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(DEPTNO=[$7])
+  LogicalJoin(condition=[=(CASE(IS NOT NULL($1), $1, $10), 'abc')], joinType=[inner])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+]]>
+    </Resource>
+    <Resource name="planAfter">
+      <![CDATA[
+LogicalProject(DEPTNO=[$7])
+  LogicalJoin(condition=[=(CASE(IS NOT NULL($1), $1, $10), 'abc')], joinType=[inner])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testJoinDeriveIsNotNullFilterRule15">
+    <Resource name="sql">
+      <![CDATA[select t1.deptno from empnullables t1 inner join
+empnullables t2 on nullif(t1.ename, t2.ename) = 'abc']]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(DEPTNO=[$7])
+  LogicalJoin(condition=[=(CASE(=($1, $10), null:VARCHAR(20), $1), 'abc')], joinType=[inner])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+]]>
+    </Resource>
+    <Resource name="planAfter">
+      <![CDATA[
+LogicalProject(DEPTNO=[$7])
+  LogicalJoin(condition=[=(CASE(=($1, $10), null:VARCHAR(20), $1), 'abc')], joinType=[inner])
+    LogicalFilter(condition=[IS NOT NULL($1)])
+      LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
+    LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
 ]]>
     </Resource>
   </TestCase>