impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4617: remove IsConstant() analysis from be
Date Fri, 16 Dec 2016 16:26:48 GMT
Tim Armstrong has uploaded a new patch set (#3).

Change subject: IMPALA-4617: remove IsConstant() analysis from be
......................................................................

IMPALA-4617: remove IsConstant() analysis from be

This change avoids the need to duplicate the logic in Expr.getConstant()
in the frontend and Expr::GetConstant() in the backend. Instead it is
plumbed through from the frontend.

To make it easier to reason about the state of isAnalyzed_ and
isConstantCached_, made isAnalyzed_ private and refactored
analyze() so that isAnalyzed_ is only set at the end of analysis, not in
the middle of it.

I considered going further and only allowing isConstant() to be called
on analyzed expressions, but there are multiple places in the code that
depend on this working in non-trivial ways.

There should be no behavioural changes as a result of this patch, aside
from a minor fix for "limit count(*)" where an internal error message
was produced instead of the expected one about constant expressions.

Testing:
Ran exhaustive tests. Added a regression test for "limit count(*)".

Change-Id: I24b4c9d6e8e57fd73d906a9c36200e1a84497b90
---
M be/src/exec/hdfs-table-sink.cc
M be/src/exprs/expr.cc
M be/src/exprs/expr.h
M be/src/exprs/literal.cc
M be/src/exprs/null-literal.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.h
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Predicate.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
37 files changed, 198 insertions(+), 220 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5415/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5415
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I24b4c9d6e8e57fd73d906a9c36200e1a84497b90
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>

Mime
View raw message