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:47:07 GMT
Tim Armstrong has posted comments on this change.

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


Patch Set 6:

(1 comment)

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.
> hmm, not sure I understand that.  if the frontend gets the right definition
The problem is that we have isConstant() in the frontend and IsConstant() in the backend that
do the same analysis. There's really no reason the backend should be doing this kind of analysis
in the first place, and if the two definitions get out of sync, bad things can happen.

E.g. the DCHECKs in AggregateFunctions::OffsetFnInit() can fire if the FE thinks that an expression
is constant, but the BE disagrees.


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