Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-1861: Simplify conditionals with constant conditions
......................................................................


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5585/10/fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java
File fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java:

Line 38:     if (!expr.isAnalyzed()) return expr;
> nit: I think this can be rewritten as below to avoid nesting.
I don't think that's really an improvement, since this function is obviously 
going to extended to handle different expr types in the future, in which case 
it'll have to be changed back to how I have it now anyways.

I added a TOCO noting this.


Line 43:     }
> nit: Extraneous newline (here and elsewhere in this patch).
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id70aaf9fd99f64bd98175b7e2dbba28f350e7d3b
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to