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-4586: don't constant fold in backend
Date Wed, 07 Dec 2016 19:24:30 GMT
Tim Armstrong 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?
I think we'd keep this patch, since we'd still only want to cache literals.


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'm just trying to reduce the number of places we conflate the "isConstant()" and no-slotrefs
concepts. IMO this is just the mechanism to evaluate exprs from the FE, so it shouldn't relate
to analysis of expr semantics (i.e. isConstant()). 

I also switched the corresponding backend DCHECK. I can defer this change to a later patch
if you think it would be better to keep this patchset small.


-- 
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