impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4617: remove IsConstant() analysis from be
Date Wed, 25 Jan 2017 22:31:32 GMT
Alex Behm has posted comments on this change.

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


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5415/5/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

Line 262:    * @see org.apache.impala.parser.ParseNode#analyze(org.apache.impala.parser.Analyzer)
> This seemed like the easiest way to enforce the invariant that isAnalyzed_ 
Agree it is the easiest way for now. There's potential for more cleanup, but we can do that
later.


Line 274:    */
> I did attempt to do that before but ran into some problems. Unfortunately i
You are right, we can't fix this yet, unfortunately. The reason is that some exprs may contain
unresolved select-list aliases which can't be analyzed (need to be substituted first).


Line 824:    */
Preconditions.checkState(!isAnalyzed_);


Line 975: 
> Missed the second part of the comment. I added that, except it's actually t
Agree. We need to fix that issue of unanalyzed exprs separately.


http://gerrit.cloudera.org:8080/#/c/5415/8/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

Line 983:    * - Functions that are not truly constant, but that we don't want to re-evaluate
I'd say now() is truly constant according to your definition (query scope).

now() must return the same value within the same query, those are its semantics. It's not
really about wanting to avoid multiple evaluations.


-- 
To view, visit http://gerrit.cloudera.org:8080/5415
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I24b4c9d6e8e57fd73d906a9c36200e1a84497b90
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message