Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9223 )

Change subject: IMPALA-6392: Consistent explain format for parquet predicate 
statistics
......................................................................


Patch Set 5:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-6392: Consistent explain format for parquet predicate 
statistics
           :
> shorten this to fit on one line
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@10
PS2, Line 10: to output each
> mention that this only affects EXPLAIN_LEVEL=2+
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@13
PS2, Line 13: Before:
> Could you include a couple lines showing what the new format looks like?
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@230
PS2, Line 230: M
> extra whitespace
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@232
PS2, Line 232:
> extra whitespace
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1099
PS2, Line 1099:   // Helper method that prints dictionary min max original 
conjucts by tuple descriptor.
> Brief comment here, like the one for getDictionaryConjunctsExplainString()
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1107
PS2, Line 1107: ut.app
> This can fit on the previous line (we wrap at 90)
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1109
PS2, Line 1109: else {
> Since this is only used once, no need to make a variable, just include it i
Done.


http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1111
PS2, Line 1111: prefix
> fits on previous line
The previous line will be 91-width long if we move prefix to the previous line.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 18:59:25 +0000
Gerrit-HasComments: Yes

Reply via email to