[impala] 02/02: IMPALA-8909: Fix incorrectly omitting implicit cast in pre-insert sort

[ta...@apache.org]

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

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

commit 796f5cc43d545a645f9eed3d13934375f31a3ce8
Author: Csaba Ringhofer <cs...@cloudera.com>
AuthorDate: Thu Feb 13 18:59:07 2020 +0100

    IMPALA-8909: Fix incorrectly omitting implicit cast in pre-insert sort
    
    Inserts can add a sort node that orders the rows by partitioning
    and Kudu primary key columns (aka. clustered insert). The issue
    occurred when the target column was a timestamp and the source
    was an expression that returned a string (e.g. concat()). Impala
    adds an implicit cast to convert the strings to timestamps before
    sorting, but this cast was incorrectly removed later during expression
    substitution.
    
    This led to hitting a DCHECK in debug builds and a (not too
    informative) error message in release mode.
    
    Note that the cast in question is not visible in EXPLAIN outputs.
    Explain should contain implicit casts from explain_level=2 since
    https://gerrit.cloudera.org/#/c/11719/ , but it is still not shown
    in some expressions. I consider this to be a separate issue.
    
    Testing:
    - added an EE test that used to crash
    - ran planner / sort / kudu_insert tests
    
    Change-Id: Icca8ab1456a3b840a47833119c9d4fd31a1fff90
    Reviewed-on: http://gerrit.cloudera.org:8080/15217
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 fe/src/main/java/org/apache/impala/planner/SortNode.java    |  8 +++++---
 .../functional-query/queries/QueryTest/kudu_insert.test     | 13 +++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/planner/SortNode.java b/fe/src/main/java/org/apache/impala/planner/SortNode.java
index c05969c..ba894c3 100644
--- a/fe/src/main/java/org/apache/impala/planner/SortNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/SortNode.java
@@ -143,13 +143,15 @@ public class SortNode extends PlanNode {
       outputSmap_.put(slotExprs.get(i), new SlotRef(sortTupleSlots.get(i)));
     }
     ExprSubstitutionMap childSmap = getCombinedChildSmap();
+    // Preserve type as resolvedTupleExprs_ will be used to materialize the tuple and the
+    // layout is already calculated.
     resolvedTupleExprs_ =
-        Expr.substituteList(resolvedTupleExprs_, childSmap, analyzer, false);
+        Expr.substituteList(resolvedTupleExprs_, childSmap, analyzer, true);
 
     // Remap the ordering exprs to the tuple materialized by this sort node. The mapping
     // is a composition of the childSmap and the outputSmap_ because the child node may
-    // have also remapped its input (e.g., as in a a series of (sort->analytic)* nodes).
-    // Parent nodes have have to do the same so set the composition as the outputSmap_.
+    // have also remapped its input (e.g., as in a series of (sort->analytic)* nodes).
+    // Parent nodes have to do the same so set the composition as the outputSmap_.
     outputSmap_ = ExprSubstitutionMap.compose(childSmap, outputSmap_, analyzer);
 
     info_.substituteSortExprs(outputSmap_, analyzer);
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
index 981a734..d2b734e 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
@@ -320,6 +320,19 @@ NumRowErrors: 0
 row_regex: .*EXEC_SINGLE_NODE_ROWS_THRESHOLD=0.*
 ====
 ---- QUERY
+create table timestampkey (t timestamp primary key)
+  partition by hash(t) PARTITIONS 2 stored as kudu;
+====
+---- QUERY
+# Regression test for IMPALA-8909 (used to hit a DCHECK in debug builds).
+insert into timestampkey select concat(string_col, " ") from functional.alltypessmall;
+---- RESULTS
+: 0
+---- RUNTIME_PROFILE
+NumModifiedRows: 0
+NumRowErrors: 100
+====
+---- QUERY
 # Table with default values
 create table tbl_with_defaults (a int primary key, b int null default 10,
   c int not null default 100, d int default 1000, e int null, f int not null,