You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2022/08/08 23:37:06 UTC
[impala] 09/27: IMPALA-11323: Don't evaluate constants-only inferred predicates
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch branch-4.1.1
in repository https://gitbox.apache.org/repos/asf/impala.git
commit b2e05224b94bf2dc96920c1a09922eb502ef1fcb
Author: Steve Carlin <sc...@cloudera.com>
AuthorDate: Wed Jun 1 12:29:56 2022 -0700
IMPALA-11323: Don't evaluate constants-only inferred predicates
IMPALA-10182 fixed the problem of creating inferred predicates when
both sides of an equality predicate came from the same slot.
Inferred predicates also should not be created when both sides
of an equality predicate are constant values which do not have
scan slots.
Change-Id: If1cd4559dda406d2d38703ed594b70b41ed336fd
Reviewed-on: http://gerrit.cloudera.org:8080/18579
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Aman Sinha <am...@cloudera.com>
---
.../main/java/org/apache/impala/analysis/Expr.java | 39 +++++++++++++++++-----
.../java/org/apache/impala/planner/PlanNode.java | 19 ++++++++---
.../queries/QueryTest/inline-view.test | 16 ++++++++-
3 files changed, 61 insertions(+), 13 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 76175d51c..0bc470053 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -1642,24 +1642,47 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
}
/**
- * Returns the descriptor of the scan slot that directly or indirectly produces
- * the values of 'this' SlotRef. Traverses the source exprs of intermediate slot
- * descriptors to resolve materialization points (e.g., aggregations).
- * Returns null if 'e' or any source expr of 'e' is not a SlotRef or cast SlotRef.
- */
- public SlotDescriptor findSrcScanSlot() {
+ * Returns the source expression for this expression. Traverses the source
+ * exprs of intermediate slot descriptors to resolve materialization points
+ * (e.g., aggregations). Returns null if there are multiple source Exprs
+ * mapped to the expression at any given point.
+ */
+ public Expr findSrcExpr() {
+ // If the source expression is a constant expression, it won't have a scanSlotRef
+ // and we can return this.
+ if (isConstant()) {
+ return this;
+ }
SlotRef slotRef = unwrapSlotRef(false);
if (slotRef == null) return null;
SlotDescriptor slotDesc = slotRef.getDesc();
- if (slotDesc.isScanSlot()) return slotDesc;
+ if (slotDesc.isScanSlot()) return slotRef;
if (slotDesc.getSourceExprs().size() == 1) {
- return slotDesc.getSourceExprs().get(0).findSrcScanSlot();
+ return slotDesc.getSourceExprs().get(0).findSrcExpr();
}
// No known source expr, or there are several source exprs meaning the slot is
// has no single source table.
return null;
}
+ /**
+ * Returns the descriptor of the scan slot that directly or indirectly produces
+ * the values of 'this' SlotRef. Traverses the source exprs of intermediate slot
+ * descriptors to resolve materialization points (e.g., aggregations).
+ * Returns null if 'e' or any source expr of 'e' is not a SlotRef or cast SlotRef.
+ */
+ public SlotDescriptor findSrcScanSlot() {
+ Expr sourceExpr = findSrcExpr();
+ if (sourceExpr == null) {
+ return null;
+ }
+ SlotRef slotRef = sourceExpr.unwrapSlotRef(false);
+ if (slotRef == null) {
+ return null;
+ }
+ return slotRef.getDesc();
+ }
+
/**
* Pushes negation to the individual operands of a predicate
* tree rooted at 'root'.
diff --git a/fe/src/main/java/org/apache/impala/planner/PlanNode.java b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
index b59acedc3..2dc21afe2 100644
--- a/fe/src/main/java/org/apache/impala/planner/PlanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
@@ -574,12 +574,23 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
// Check if this is an inferred identity predicate i.e for c1 = c2 both
// sides are pointing to the same source slot. In such cases it is wrong
// to add the predicate to the SELECT node because it will incorrectly
- // eliminate rows with NULL values.
+ // eliminate rows with NULL values. We also check if both sides are pointing
+ // to equal constant values.
for (Expr e : conjuncts) {
if (e instanceof BinaryPredicate && ((BinaryPredicate) e).isInferred()) {
- SlotDescriptor lhs = ((BinaryPredicate) e).getChild(0).findSrcScanSlot();
- SlotDescriptor rhs = ((BinaryPredicate) e).getChild(1).findSrcScanSlot();
- if (lhs != null && rhs != null && lhs.equals(rhs)) continue;
+ Expr lhsSrcExpr = ((BinaryPredicate) e).getChild(0).findSrcExpr();
+ Expr rhsSrcExpr = ((BinaryPredicate) e).getChild(1).findSrcExpr();
+ if (lhsSrcExpr != null && rhsSrcExpr != null) {
+ if (lhsSrcExpr.isConstant() && rhsSrcExpr.isConstant() &&
+ lhsSrcExpr.equals(rhsSrcExpr)) {
+ continue;
+ }
+ if (lhsSrcExpr instanceof SlotRef && rhsSrcExpr instanceof SlotRef) {
+ SlotRef lhsSlotRef = (SlotRef) lhsSrcExpr;
+ SlotRef rhsSlotRef = (SlotRef) rhsSrcExpr;
+ if (lhsSlotRef.getDesc().equals(rhsSlotRef.getDesc())) continue;
+ }
+ }
}
finalConjuncts.add(e);
}
diff --git a/testdata/workloads/functional-query/queries/QueryTest/inline-view.test b/testdata/workloads/functional-query/queries/QueryTest/inline-view.test
index 2cbda7bba..5c0cc185d 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/inline-view.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/inline-view.test
@@ -578,4 +578,18 @@ NULL,NULL
NULL,NULL
---- TYPES
INT, INT
-====
\ No newline at end of file
+====
+---- QUERY
+# IMPALA-11323: Constant expressions should get filtered out so they
+# don't get placed in the select node in the planner.
+with t as (select 1 a), v as
+ (select distinct a, cast(null as smallint) b, cast(null as smallint) c from t)
+select distinct a,b,c from v
+ union all
+select distinct a,b,c from v;
+---- RESULTS
+1,NULL,NULL
+1,NULL,NULL
+---- TYPES
+TINYINT, SMALLINT, SMALLINT
+====