incubator-impala git commit: IMPALA-5650: Make sum_init_zero a SUM function

[ta...@apache.org]

Repository: incubator-impala
Updated Branches:
  refs/heads/master 1d8274a89 -> e6c02db97


IMPALA-5650: Make sum_init_zero a SUM function

The recent Parquet count(*) optimization (IMPALA-5036) introduced a new
aggregate function sum_init_zero(). This caused an issue for non
partitioned aggregation node, which checks that the function type is
either count, min, max, sum or ndv. Since sum_init_zero() was not
labelled as a sum() function, a DCHECK was hit. This issue is fixed by
labelling sum_init_zero() as a sum() function.

Testing:
- Verified that we no longer hit a DCHECK when running a query with a
  Parquet count(*) optimization with legacy agg node enabled.

Change-Id: Ibca53e3ef6bad5283550100b33db55d674cff0b9
Reviewed-on: http://gerrit.cloudera.org:8080/7404
Reviewed-by: Michael Ho <kw...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: e6c02db9737e39b3a239e0ea8f92a2fd35f3a8fb
Parents: 1d8274a
Author: Taras Bobrovytsky <tb...@cloudera.com>
Authored: Tue Jul 11 18:38:46 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Jul 14 00:59:08 2017 +0000

----------------------------------------------------------------------
 be/src/exec/partitioned-aggregation-node.cc | 13 +++++++++----
 be/src/exprs/agg-fn.cc                      |  2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e6c02db9/be/src/exec/partitioned-aggregation-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/partitioned-aggregation-node.cc b/be/src/exec/partitioned-aggregation-node.cc
index e5ab0f3..7849394 100644
--- a/be/src/exec/partitioned-aggregation-node.cc
+++ b/be/src/exec/partitioned-aggregation-node.cc
@@ -1587,10 +1587,15 @@ Status PartitionedAggregationNode::CodegenUpdateSlot(LlvmCodeGen* codegen,
     } else {
       result = builder.CreateAdd(dst_value, src.GetVal());
     }
-    // Dst may have been NULL, make sure to unset the NULL bit.
-    DCHECK(slot_desc->is_nullable());
-    slot_desc->CodegenSetNullIndicator(
-        codegen, &builder, agg_tuple_arg, codegen->false_value());
+
+    if (slot_desc->is_nullable()) {
+      slot_desc->CodegenSetNullIndicator(
+          codegen, &builder, agg_tuple_arg, codegen->false_value());
+    } else {
+      // 'slot_desc' is not nullable if the aggregate function is sum_init_zero(),
+      // because the slot is initialized to be zero and the null bit is nonexistent.
+      DCHECK_EQ(agg_fn->fn_name(), "sum_init_zero");
+    }
   } else {
     // The remaining cases are implemented using the UDA interface.
     // Create intermediate argument 'dst' from 'dst_value'

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e6c02db9/be/src/exprs/agg-fn.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/agg-fn.cc b/be/src/exprs/agg-fn.cc
index 6e1d75f..68ea7bf 100644
--- a/be/src/exprs/agg-fn.cc
+++ b/be/src/exprs/agg-fn.cc
@@ -50,7 +50,7 @@ AggFn::AggFn(const TExprNode& tnode, const SlotDescriptor& intermediate_slot_des
     agg_op_ = MIN;
   } else if (fn_name == "max") {
     agg_op_ = MAX;
-  } else if (fn_name == "sum") {
+  } else if (fn_name == "sum" || fn_name == "sum_init_zero") {
     agg_op_ = SUM;
   } else if (fn_name == "avg") {
     agg_op_ = AVG;