impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <>
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:

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.
File fe/src/main/java/org/apache/impala/service/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I0c76e3c8a8d92749256c312080ecd7aac5d99ce7
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message