You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Ho (Code Review)" <ge...@cloudera.org> on 2017/01/20 02:01:35 UTC

[Impala-ASF-CR] IMPALA-4192: Disentangle Expr and ExprContext

Hello Tim Armstrong,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5483

to look at the new patch set (#4).

Change subject: IMPALA-4192: Disentangle Expr and ExprContext
......................................................................

IMPALA-4192: Disentangle Expr and ExprContext

This change cleans up some entangled logic between Expr and
ExprContext's initialization. In general, we stored static
states of an expression in Expr and thread-private / evaluation
related states in ExprContext.

This change cleans up the followings:

1. FunctionContext should be managed by ExprContext. They reside
   in a vector inside ExprContext. When creating the expression tree,
   each sub-expression requiring a FunctionContext will be assigned
   an index into the vector. The index is stoed in the Expr object.
   ExprContext::Prepare() will allocate and prepare the FunctionContext
   for all sub-expressions in the expression tree.

2. Move the field 'output_scale_' to ExprContext. This field is needed
   to format the output in the table writer correctly for the case of
   RoundUpTo(). However, this field can only be derived much later in
   the initialization so in the spirit of not modifying the Expr after
   it has been initialized, let's move this field out to ExprContext
   until IMPALA-4743 and IMPALA-4720 are fixed.

3. Rename Expr::Prepare(), Expr::Open() and Expr::Close() to
   ExprContext class instead as they really operate soly on
   ExprContext but not Expr.

This clean up is a step towards sharing expression states among plan
fragment instances.

This change also fixes a bug in Expr::GetFnContextError().
The old code only returned the error in the top level
FunctionContext, ignorning the error set by its descendants.

Change-Id: Iefdc9aeeba033355cb9497e3a5d2363627dcf2f3
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exec/aggregation-node.cc
M be/src/exec/analytic-eval-node.cc
M be/src/exec/exec-node.cc
M be/src/exec/hash-join-node.cc
M be/src/exec/hash-table-test.cc
M be/src/exec/hbase-table-sink.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scanner.cc
M be/src/exec/kudu-table-sink.cc
M be/src/exec/nested-loop-join-node.cc
M be/src/exec/old-hash-table-test.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/sort-exec-exprs.cc
M be/src/exec/union-node.cc
M be/src/exec/unnest-node.cc
M be/src/exprs/agg-fn-evaluator.cc
M be/src/exprs/case-expr.cc
M be/src/exprs/case-expr.h
M be/src/exprs/expr-codegen-test.cc
M be/src/exprs/expr-context.cc
M be/src/exprs/expr-context.h
M be/src/exprs/expr.cc
M be/src/exprs/expr.h
M be/src/exprs/hive-udf-call.cc
M be/src/exprs/hive-udf-call.h
M be/src/exprs/is-not-empty-predicate.cc
M be/src/exprs/is-not-empty-predicate.h
M be/src/exprs/scalar-fn-call.cc
M be/src/exprs/scalar-fn-call.h
M be/src/exprs/slot-ref.cc
M be/src/exprs/slot-ref.h
M be/src/exprs/tuple-is-null-predicate.cc
M be/src/exprs/tuple-is-null-predicate.h
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/data-stream-test.cc
M be/src/runtime/descriptors.cc
M be/src/runtime/row-batch.h
M be/src/service/fe-support.cc
M common/thrift/ImpalaInternalService.thrift
46 files changed, 650 insertions(+), 528 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/5483/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iefdc9aeeba033355cb9497e3a5d2363627dcf2f3
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>