Thomas Tauber-Marshall 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 5: (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 need to truncate timestamp. mention that this is for Kudu http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@346 PS5, Line 346: return null; Lets add a Log.warn with the error message here. http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@644 PS5, Line 644: if (filterType == TRuntimeFilterType.BLOOM Add a brief comment here saying something like "For timestamp bloom filters we also generate a RuntimeFilter with the src timestamp truncated for Kudu scan node targets" http://gerrit.cloudera.org:8080/#/c/16094/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@773 PS5, Line 773: targetExpr.getType().isTimestamp() && !filter.isTimestampTruncation() I think it would make this 'if' clearer if you surrounded this with parentheses, to show that these conditions are related. -- 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: 5 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 00:00:59 +0000 Gerrit-HasComments: Yes
