impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4586: don't constant fold in backend
Date Wed, 07 Dec 2016 19:08:06 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4586: don't constant fold in backend
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5391/6//COMMIT_MSG
Commit Message:

PS6, Line 26: In a future change we should remove the IsConstant()
            : analysis logic from the backend entirely and pass the information from
            : the frontend.
once we do that, would we effectively revert this patch?


http://gerrit.cloudera.org:8080/#/c/5391/6/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:

Line 124:     Preconditions.checkState(!expr.contains(SlotRef.class));
why is this a better assert? isConstant() is more restrictive, right?

I guess what you're trying to say with this renaming and assert adjustments is that the machinery
can evaluate anything without slotrefs, even though it might not be appropriate to do so?
 (i.e. we should still only try to evaluate isConstant() exprs, right?)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0c76e3c8a8d92749256c312080ecd7aac5d99ce7
Gerrit-PatchSet: 6
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