Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/16094 )
Change subject: IMPALA-9691: Support Kudu Timestamp and Date bloom filter ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java: http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@199 PS5, Line 199: // If set, indicates that the filter is targeted for Kudu scan node with source > mention that this is for Kudu Ok, will fix it. http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@346 PS5, Line 346: } catch (AnalysisException e) { > Lets add a Log.warn with the error message here. Will add log message. http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@644 PS5, Line 644: joinNode, filterType, bloomFilterSizeLimits_, > Add a brief comment here saying something like "For timestamp bloom filters Will add comments. http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@773 PS5, Line 773: nue; > I think it would make this 'if' clearer if you surrounded this with parenth Will fix it. -- To view, visit http://gerrit.cloudera.org:8080/16094 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c1e9bcc9fd6d79a39f25eaa3396188fc0a52a48 Gerrit-Change-Number: 16094 Gerrit-PatchSet: 6 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Fri, 26 Jun 2020 01:16:05 +0000 Gerrit-HasComments: Yes
