You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/07/13 06:04:03 UTC

[51/51] [abbrv] impala git commit: IMPALA-7288: Fix Codegen Crash in FinalizeModule()

IMPALA-7288: Fix Codegen Crash in FinalizeModule()

Currently codegen crashed during FinalizeModule() where it tries to
clean up half-baked handcrafted functions. This happens only for the
cases where the code generating the handcrafted IR calls
eraseFromParent() on failure which also deletes the memory held by the
function pointer and therefore causes a crash during clean up in
FinalizeModule().

Testing:
Added regression tests that verify that failure code paths in
the previously offending methods don't crash Impala.

Change-Id: I2f0b527909a9fb3090996bb7510e4d58350c21b0
Reviewed-on: http://gerrit.cloudera.org:8080/10933
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/540611e8
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/540611e8
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/540611e8

Branch: refs/heads/master
Commit: 540611e863fe99b3d3ae35f8b94a745a68b9eba2
Parents: e2aafae
Author: Bikramjeet Vig <bi...@cloudera.com>
Authored: Thu Jul 12 14:43:07 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Jul 13 04:07:19 2018 +0000

----------------------------------------------------------------------
 be/src/exec/hash-table.cc        |  2 --
 be/src/exec/hdfs-avro-scanner.cc |  1 -
 be/src/exec/hdfs-scanner.cc      |  1 -
 tests/query_test/test_codegen.py | 17 +++++++++++++++++
 4 files changed, 17 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/540611e8/be/src/exec/hash-table.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hash-table.cc b/be/src/exec/hash-table.cc
index 38e0d26..47a9a72 100644
--- a/be/src/exec/hash-table.cc
+++ b/be/src/exec/hash-table.cc
@@ -763,7 +763,6 @@ Status HashTableCtx::CodegenEvalRow(
     llvm::Function* expr_fn;
     Status status = exprs[i]->GetCodegendComputeFn(codegen, &expr_fn);
     if (!status.ok()) {
-      (*fn)->eraseFromParent(); // deletes function
       *fn = NULL;
       return Status(Substitute(
           "Problem with HashTableCtx::CodegenEvalRow(): $0", status.GetDetail()));
@@ -1113,7 +1112,6 @@ Status HashTableCtx::CodegenEquals(
     llvm::Function* expr_fn;
     Status status = build_exprs_[i]->GetCodegendComputeFn(codegen, &expr_fn);
     if (!status.ok()) {
-      (*fn)->eraseFromParent(); // deletes function
       *fn = NULL;
       return Status(
           Substitute("Problem with HashTableCtx::CodegenEquals: $0", status.GetDetail()));

http://git-wip-us.apache.org/repos/asf/impala/blob/540611e8/be/src/exec/hdfs-avro-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-avro-scanner.cc b/be/src/exec/hdfs-avro-scanner.cc
index 9a92763..51d4ed6 100644
--- a/be/src/exec/hdfs-avro-scanner.cc
+++ b/be/src/exec/hdfs-avro-scanner.cc
@@ -841,7 +841,6 @@ Status HdfsAvroScanner::CodegenMaterializeTuple(const HdfsScanNodeBase* node,
         bail_out_block, this_val, pool_val, tuple_val, data_val, data_end_val);
     if (!status.ok()) {
       VLOG_QUERY << status.GetDetail();
-      helper_fn->eraseFromParent();
       return status;
     }
 

http://git-wip-us.apache.org/repos/asf/impala/blob/540611e8/be/src/exec/hdfs-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scanner.cc b/be/src/exec/hdfs-scanner.cc
index da44e9a..8947138 100644
--- a/be/src/exec/hdfs-scanner.cc
+++ b/be/src/exec/hdfs-scanner.cc
@@ -491,7 +491,6 @@ Status HdfsScanner::CodegenWriteCompleteTuple(const HdfsScanNodeBase* node,
         stringstream ss;
         ss << "Failed to codegen conjunct: " << status.GetDetail();
         state->LogError(ErrorMsg(TErrorCode::GENERAL, ss.str()));
-        fn->eraseFromParent();
         return status;
       }
       if (node->materialized_slots().size() + conjunct_idx

http://git-wip-us.apache.org/repos/asf/impala/blob/540611e8/tests/query_test/test_codegen.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_codegen.py b/tests/query_test/test_codegen.py
index a4c02f5..5bc0cc6 100644
--- a/tests/query_test/test_codegen.py
+++ b/tests/query_test/test_codegen.py
@@ -56,3 +56,20 @@ class TestCodegen(ImpalaTestSuite):
   def test_datastream_sender_codegen(self, vector):
     """Test the KrpcDataStreamSender's codegen logic"""
     self.run_test_case('QueryTest/datastream-sender-codegen', vector)
+
+  def test_codegen_failure_for_char_type(self, vector):
+    """IMPALA-7288: Regression tests for the codegen failure path when working with a
+    CHAR column type"""
+    # Test failure path in HashTableCtx::CodegenEquals().
+    result = self.execute_query("select 1 from functional.chars_tiny t1, "
+                                "functional.chars_tiny t2 "
+                                "where t1.cs = cast(t2.cs as string)");
+    assert "Codegen Disabled: Problem with HashTableCtx::CodegenEquals: ScalarFnCall" \
+           " Codegen not supported for CHAR" in str(result.runtime_profile)
+
+    # Test failure path in HashTableCtx::CodegenEvalRow().
+    result = self.execute_query("select 1 from functional.chars_tiny t1, "
+                                "functional.chars_tiny t2 where t1.cs = "
+                                "FROM_TIMESTAMP(cast(t2.cs as string), 'yyyyMMdd')");
+    assert "Codegen Disabled: Problem with HashTableCtx::CodegenEvalRow(): ScalarFnCall" \
+           " Codegen not supported for CHAR" in str(result.runtime_profile)