diff --git a/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java b/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java index f67ac8b53..e303a11ef 100644 --- a/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java +++ b/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java @@ -55,6 +55,8 @@ public class SlotDescriptor { private boolean isMaterialized_ = false; // if false, this slot cannot be NULL + // Note: it is still possible that a SlotRef pointing to this descriptor could have a + // NULL value if the entire tuple is NULL, for example as the result of an outer join. private boolean isNullable_ = true; // physical layout parameters diff --git a/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java b/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java index 287b4be1b..6d0b38c8f 100644 --- a/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java +++ b/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java @@ -103,8 +103,6 @@ public class SimplifyConditionalsRule implements ExprRewriteRule { * Simplify COALESCE by skipping leading nulls and applying the following transformations: * COALESCE(null, a, b) -> COALESCE(a, b); * COALESCE(, a, b) -> , when literal is not NullLiteral; - * COALESCE(, a, b) -> , - * when the partition column does not contain NULL. */ private Expr simplifyCoalesceFunctionCallExpr(FunctionCallExpr expr) { int numChildren = expr.getChildren().size(); @@ -113,7 +111,7 @@ public class SimplifyConditionalsRule implements ExprRewriteRule { Expr childExpr = expr.getChildren().get(i); // Skip leading nulls. if (childExpr.isNullLiteral()) continue; - if ((i == numChildren - 1) || canSimplifyCoalesceUsingChild(childExpr)) { + if ((i == numChildren - 1) || childExpr.isLiteral()) { result = childExpr; } else if (i == 0) { result = expr; @@ -126,32 +124,6 @@ public class SimplifyConditionalsRule implements ExprRewriteRule { return result; } - /** - * Checks if the given child expr is nullable. Returns true if one of the following holds: - * child is a non-NULL literal; - * child is a possibly cast SlotRef against a non-nullable slot; - * child is a possible cast SlotRef against a partition column that does not contain NULL. - */ - private boolean canSimplifyCoalesceUsingChild(Expr child) { - if (child.isLiteral() && !child.isNullLiteral()) return true; - - SlotRef slotRef = child.unwrapSlotRef(false); - if (slotRef == null) return false; - SlotDescriptor slotDesc = slotRef.getDesc(); - if (!slotDesc.getIsNullable()) return true; - // Check partition column using partition metadata. - if (slotDesc.getParent().getTable() instanceof HdfsTable - && slotDesc.getColumn() != null - && slotDesc.getParent().getTable().isClusteringColumn(slotDesc.getColumn())) { - HdfsTable table = (HdfsTable) slotDesc.getParent().getTable(); - // Return true if the partition column does not have a NULL value. - if (table.getNullPartitionIds(slotDesc.getColumn().getPosition()).isEmpty()) { - return true; - } - } - return false; - } - private Expr simplifyFunctionCallExpr(FunctionCallExpr expr) { FunctionName fnName = expr.getFnName(); diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java index e49c6525f..4ff50a33b 100644 --- a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java @@ -398,20 +398,9 @@ public class ExprRewriteRulesTest extends FrontendTestBase { RewritesOk("coalesce(1 + 2, id, year)", rules, "3"); RewritesOk("coalesce(null is null, bool_col)", rules, "TRUE"); RewritesOk("coalesce(10 + null, id, year)", rules, "coalesce(id, year)"); - // If the leading parameter is partition column, try to rewrite with partition metadata. - RewritesOk("coalesce(year, id)", rule, "year"); - RewritesOk("coalesce(year, bigint_col)", rule, "year"); - RewritesOk("coalesce(cast(year as string), string_col)", rule, "CAST(year AS STRING)"); - RewritesOk("coalesce(id, year)", rule, null); - RewritesOk("coalesce(null, year, id)", rule, "year"); - // If the leading partition column has NULL value, do not rewrite. - RewritesOk("functional.alltypesagg", "coalesce(year, id)", rule, "year"); - RewritesOk("functional.alltypesagg", "coalesce(day, id)", rule, null); - // If the leading column is not nullable, rewrite to the column. - RewritesOk("functional_kudu.alltypessmall", "coalesce(id, year)", rule, "id"); - RewritesOk("functional_kudu.alltypessmall", "coalesce(cast(id as string), string_col)", rule, - "CAST(id AS STRING)"); - RewritesOk("functional_kudu.alltypessmall", "coalesce(null, id, year)", rule, "id"); + // Don't rewrite based on nullability of slots. TODO (IMPALA-5753). + RewritesOk("coalesce(year, id)", rule, null); + RewritesOk("functional_kudu.alltypessmall", "coalesce(id, year)", rule, null); } @Test diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test index d618a2497..9b1a1a73d 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test +++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test @@ -2741,4 +2741,26 @@ select if (true, 0, sum(id)) from functional.alltypestiny 0 ---- TYPES BIGINT -==== \ No newline at end of file +==== +---- QUERY +# IMPALA-5725: test coalesce() is not rewritten when its first parameter is a non-nullable +# Kudu column, but can have NULL values due to an outer join. +select coalesce(b.id, a.id), b.id, a.id +from functional_kudu.alltypes a left join functional_kudu.alltypestiny b on a.id = b.id +where a.id = 100 +---- RESULTS +100,NULL,100 +---- TYPES +INT,INT,INT +==== +---- QUERY +# IMPALA-5725: test coalesce() is not rewritten when its first parameter is an HDFS +# partition col with no NULL partitions, but can have NULL values due to an outer join. +select coalesce(b.year, a.id), b.id, a.id +from functional.alltypes a left join functional.alltypestiny b on a.id = b.id +where a.id = 100 +---- RESULTS +100,NULL,100 +---- TYPES +INT,INT,INT +====