impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <>
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/
M be/src/exprs/
M be/src/exprs/
M be/src/exprs/case-expr.h
M be/src/exprs/
M be/src/exprs/expr-context.h
M be/src/exprs/
M be/src/exprs/expr.h
M be/src/exprs/
M be/src/exprs/hive-udf-call.h
M be/src/exprs/
M be/src/exprs/is-not-empty-predicate.h
M be/src/exprs/
M be/src/exprs/scalar-fn-call.h
M be/src/exprs/
M be/src/exprs/slot-ref.h
M be/src/exprs/
M be/src/exprs/tuple-is-null-predicate.h
M be/src/runtime/row-batch.h
M be/src/service/
M common/thrift/ImpalaInternalService.thrift
21 files changed, 312 insertions(+), 269 deletions(-)

  git pull ssh:// refs/changes/83/5483/2
To view, visit
To unsubscribe, visit

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iefdc9aeeba033355cb9497e3a5d2363627dcf2f3
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <>
Gerrit-Reviewer: Tim Armstrong <>

View raw message