impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4192: Avoid updating Expr's states from ExprContext
Date Tue, 03 Jan 2017 20:12:57 GMT
Michael Ho has uploaded a new patch set (#2).

Change subject: IMPALA-4192: Avoid updating Expr's states from ExprContext
......................................................................

IMPALA-4192: Avoid updating Expr's states from ExprContext

This change cleans up some entangled logic between Expr
and ExprContext's initialization. In particular, Expr::Prepare()
should initialize Expr's states only. Thread private states
in ExprContext should only be updated in ExprContext::Prepare().
Likewise, ExprContext's initialization should not modify the
Expr's states (except for the shared library cache). This change
does the following:

1. Don't allocate FunctionContext in Expr::Prepare().
   Allocate them in ExprContext::Prepare() instead. This allows
   Expr::Prepare() to be moved out of ExprContext::Prepare()
   in future changes.
2. Move the field 'output_scale_' to ExprContext.

This clean up enables a follow up step to create a single
Expr instance shared by multiple fragment instances, with
each instance's private state being stored in ExprContext.

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/exec/plan-root-sink.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-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/row-batch.h
M be/src/service/fe-support.cc
M common/thrift/ImpalaInternalService.thrift
21 files changed, 312 insertions(+), 269 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/5483/2
-- 
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>

Mime
View raw message