[impala] 03/03: IMPALA-11811: Avoid storing unregistered predicate objects in a Map

[bo...@apache.org]

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 11068d9aebd83be59773f2b1eaa5682053d4428b
Author: Andrew Sherman <as...@cloudera.com>
AuthorDate: Wed Dec 21 09:40:37 2022 -0800

    IMPALA-11811: Avoid storing unregistered predicate objects in a Map
    
    Within the extractIcebergConjuncts() method we are tracking conjuncts
    which are identity conjuncts by storing them in a temporary Map. The
    conjuncts are Expr objects which have a hashCode() method based on
    their id_ field, which is only present when they are registered. If the
    id_ field is null, then the hashCode() will throw, and hence
    unregistered predicates cannot be stored in a Map. Some predicates
    produced by getBoundPredicates() are explicitly not registered.
    
    Change extractIcebergConjuncts() to track the identity conjuncts using
    a boolean array, which tracks the index of the identity conjuncts in
    conjuncts_ List.
    
    Print the name of the Class in the Expr.hashCode() error to aid future
    debugging.
    
    TESTING:
    
    Add a query which causes an unregistered predicate Expr to be seen
    during Iceberg scan planning.
    
    Change-Id: I103e3b8b06b5a1d12214241fd5907e5192d682ce
    Reviewed-on: http://gerrit.cloudera.org:8080/19390
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 fe/src/main/java/org/apache/impala/analysis/Expr.java      |  3 ++-
 .../java/org/apache/impala/planner/IcebergScanPlanner.java | 14 +++++++++-----
 .../functional-query/queries/QueryTest/iceberg-query.test  |  9 +++++++++
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java
index 0bc470053..722d99df4 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -1031,7 +1031,8 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
   @Override
   public int hashCode() {
     if (id_ == null) {
-      throw new UnsupportedOperationException("Expr.hashCode() is not implemented");
+      throw new UnsupportedOperationException(
+          "Expr.hashCode() is not implemented in " + this.getClass().getName());
     } else {
       return id_.asInt();
     }
diff --git a/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java b/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
index 730ce3af9..4b289e500 100644
--- a/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
+++ b/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
@@ -463,15 +463,18 @@ public class IcebergScanPlanner {
   private void extractIcebergConjuncts() throws ImpalaException {
     boolean isPartitionColumnIncluded = false;
     Map<SlotId, SlotDescriptor> idToSlotDesc = new HashMap<>();
-    Set<Expr> identityConjuncts = new HashSet<>();
+    // Track identity conjuncts by their index in conjuncts_.
+    // The array values are initialized to false.
+    boolean[] identityConjunctIndex = new boolean[conjuncts_.size()];
     for (SlotDescriptor slotDesc : tblRef_.getDesc().getSlots()) {
       idToSlotDesc.put(slotDesc.getId(), slotDesc);
     }
-    for (Expr expr : conjuncts_) {
+    for (int i = 0; i < conjuncts_.size(); i++) {
+      Expr expr = conjuncts_.get(i);
       if (isPartitionColumnIncluded(expr, idToSlotDesc)) {
         isPartitionColumnIncluded = true;
         if (isIdentityPartitionIncluded(expr, idToSlotDesc)) {
-          identityConjuncts.add(expr);
+          identityConjunctIndex[i] = true;
         }
       }
     }
@@ -480,9 +483,10 @@ public class IcebergScanPlanner {
       nonIdentityConjuncts_ = conjuncts_;
       return;
     }
-    for (Expr expr : conjuncts_) {
+    for (int i = 0; i < conjuncts_.size(); i++) {
+      Expr expr = conjuncts_.get(i);
       if (tryConvertIcebergPredicate(expr)) {
-        if (!identityConjuncts.contains(expr)) {
+        if (!identityConjunctIndex[i]) {
           nonIdentityConjuncts_.add(expr);
         }
       } else {
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test
index 1c281bd1e..a6b28cfc8 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test
@@ -1151,3 +1151,12 @@ INT, TIMESTAMP
 ---- RUNTIME_PROFILE
 aggregation(SUM, NumRowGroups): 3
 ====
+---- QUERY
+# IMPALA-11811 query that generates unregistered predicates.
+select i.id from  iceberg_partitioned i, iceberg_partitioned j
+where i.action in ('view') and j.id=1 and j.id=i.id;
+---- RESULTS
+1
+---- TYPES
+int
+====