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>