You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by bo...@apache.org on 2024/11/22 14:39:28 UTC

(impala) 01/04: IMPALA-13540: Calcite planner: fix wrong results for set operators

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

boroknagyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 265479f068ffc39342205fb6fa6fc403bcfea6e5
Author: Steve Carlin <sc...@cloudera.com>
AuthorDate: Mon Nov 11 06:54:03 2024 -0800

    IMPALA-13540: Calcite planner: fix wrong results for set operators
    
    Calcite treats the intersect set operator with higher precedence
    when compared with the except and union set operators. Impala treats
    all the precedences equally (favoring left operators over right).
    
    The following query was failing
    select 100 union select 101 intersect select 101
    
    Calcite was returning 2 rows here, performing the intersect before
    the union. Impala does the union first and returned one row.
    
    To fix this, new custom operators were created for the set operators
    where all set operators have equal precedence.
    
    Change-Id: Ic52661a30cc90534ea1a20868799edf9ceed13b6
    Reviewed-on: http://gerrit.cloudera.org:8080/22052
    Reviewed-by: Michael Smith <mi...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../src/main/codegen/templates/Parser.jj           | 19 ++++++++++---------
 .../operators/ImpalaCustomOperatorTable.java       | 22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/java/calcite-planner/src/main/codegen/templates/Parser.jj b/java/calcite-planner/src/main/codegen/templates/Parser.jj
index 128750f30..d7c558812 100644
--- a/java/calcite-planner/src/main/codegen/templates/Parser.jj
+++ b/java/calcite-planner/src/main/codegen/templates/Parser.jj
@@ -122,6 +122,7 @@ import org.apache.calcite.util.SourceStringReader;
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.trace.CalciteTrace;
 import org.apache.impala.calcite.type.ImpalaSqlIntervalQualifier;
+import org.apache.impala.calcite.operators.ImpalaCustomOperatorTable;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -7730,16 +7731,16 @@ SqlBinaryOperator BinaryQueryOperator() :
     (
         <UNION>
         (
-            <ALL> { return SqlStdOperatorTable.UNION_ALL; }
-        |   <DISTINCT> { return SqlStdOperatorTable.UNION; }
-        |   { return SqlStdOperatorTable.UNION; }
+            <ALL> { return ImpalaCustomOperatorTable.UNION_ALL; }
+        |   <DISTINCT> { return ImpalaCustomOperatorTable.UNION; }
+        |   { return ImpalaCustomOperatorTable.UNION; }
         )
     |
         <INTERSECT>
         (
-            <ALL> { return SqlStdOperatorTable.INTERSECT_ALL; }
-        |   <DISTINCT> { return SqlStdOperatorTable.INTERSECT; }
-        |   { return SqlStdOperatorTable.INTERSECT; }
+            <ALL> { return ImpalaCustomOperatorTable.INTERSECT_ALL; }
+        |   <DISTINCT> { return ImpalaCustomOperatorTable.INTERSECT; }
+        |   { return ImpalaCustomOperatorTable.INTERSECT; }
         )
     |
         (
@@ -7752,9 +7753,9 @@ SqlBinaryOperator BinaryQueryOperator() :
             }
         )
         (
-            <ALL> { return SqlStdOperatorTable.EXCEPT_ALL; }
-        |   <DISTINCT> { return SqlStdOperatorTable.EXCEPT; }
-        |   { return SqlStdOperatorTable.EXCEPT; }
+            <ALL> { return ImpalaCustomOperatorTable.EXCEPT_ALL; }
+        |   <DISTINCT> { return ImpalaCustomOperatorTable.EXCEPT; }
+        |   { return ImpalaCustomOperatorTable.EXCEPT; }
         )
     )
 }
diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
index d3e3b45e7..9bd41e538 100644
--- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
+++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
@@ -24,6 +24,7 @@ import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.SqlBinaryOperator;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlOperatorBinding;
+import org.apache.calcite.sql.SqlSetOperator;
 import org.apache.calcite.sql.fun.ImpalaGroupingFunction;
 import org.apache.calcite.sql.fun.SqlMonotonicBinaryOperator;
 import org.apache.calcite.sql.fun.SqlCountAggFunction;
@@ -224,6 +225,27 @@ public class ImpalaCustomOperatorTable extends ReflectiveSqlOperatorTable {
 
   public static final ImpalaDecodeFunction DECODE = ImpalaDecodeFunction.INSTANCE;
 
+  // Override all the set operators. Calcite places intersect with greater precedence
+  // over union and except which is the SQL standard. Impala sets equal precedence for
+  // all the set operators.
+  public static final SqlSetOperator UNION =
+      new SqlSetOperator("UNION", SqlKind.UNION, 12, false);
+
+  public static final SqlSetOperator UNION_ALL =
+      new SqlSetOperator("UNION ALL", SqlKind.UNION, 12, true);
+
+  public static final SqlSetOperator EXCEPT =
+      new SqlSetOperator("EXCEPT", SqlKind.EXCEPT, 12, false);
+
+  public static final SqlSetOperator EXCEPT_ALL =
+      new SqlSetOperator("EXCEPT ALL", SqlKind.EXCEPT, 12, true);
+
+  public static final SqlSetOperator INTERSECT =
+      new SqlSetOperator("INTERSECT", SqlKind.INTERSECT, 12, false);
+
+  public static final SqlSetOperator INTERSECT_ALL =
+      new SqlSetOperator("INTERSECT ALL", SqlKind.INTERSECT, 12, true);
+
   public static ImpalaCustomOperatorTable instance() {
     return INSTANCE.get();
   }