impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-1861: Simplify conditionals with constant conditions
Date Wed, 18 Jan 2017 21:04:42 GMT
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 <tmarshall@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message