[Impala-ASF-CR] IMPALA-9035: Simplify casting string to timestamp.

["Gabor Kaszab (Code Review)" <ge...@cloudera.org>]

Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14896 )

Change subject: IMPALA-9035: Simplify casting string to timestamp.
......................................................................


Patch Set 2:

(4 comments)

Hey,
Sorry for the delay. Most of us were off at the end of the year. I have some minor comments. Once you take care of them I'm fine to +2.

http://gerrit.cloudera.org:8080/#/c/14896/1/fe/src/main/java/org/apache/impala/rewrite/SimplifyCastStringToTimestamp.java
File fe/src/main/java/org/apache/impala/rewrite/SimplifyCastStringToTimestamp.java:

http://gerrit.cloudera.org:8080/#/c/14896/1/fe/src/main/java/org/apache/impala/rewrite/SimplifyCastStringToTimestamp.java@56
PS1, Line 56: simplified
> nit: simplified_expr would be a more verbose name
Any comment on this?


http://gerrit.cloudera.org:8080/#/c/14896/1/fe/src/main/java/org/apache/impala/rewrite/SimplifyCastStringToTimestamp.java@66
PS1, Line 66:                                           Lists.newArrayList(fce.getChildren()));
The preferred indentation for broken lines is 4 spaces.


http://gerrit.cloudera.org:8080/#/c/14896/2/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:

http://gerrit.cloudera.org:8080/#/c/14896/2/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@786
PS2, Line 786:         null);
nit: this fits into the line above


http://gerrit.cloudera.org:8080/#/c/14896/2/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@788
PS2, Line 788:         null);
nit: this fits into the line above



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ed72d6e7886eaf50d2be60cf45170ffaef5e72d
Gerrit-Change-Number: 14896
Gerrit-PatchSet: 2
Gerrit-Owner: Zhou Xu <pe...@gmail.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zhou Xu <pe...@gmail.com>
Gerrit-Comment-Date: Thu, 02 Jan 2020 12:27:57 +0000
Gerrit-HasComments: Yes