Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/19220 )
Change subject: IMPALA-11707: Fix global runtime IN-list filter of numeric types are AlwaysFalse ...................................................................... Patch Set 2: (6 comments) Looks good! http://gerrit.cloudera.org:8080/#/c/19220/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19220/2//COMMIT_MSG@21 PS2, Line 21: Tests nit. Core tests? http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.h File be/src/util/in-list-filter.h: http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.h@143 PS2, Line 143: pair<boost::unordered_set<StringValue>::iterator, bool> nit may add a comment describing this pair. http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.h@144 PS2, Line 144: insert Seems it can be replaced by emplace() to a) maintain consistency with the insert() at line 149, and b) keep only the unique values in "values". http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.h@164 PS2, Line 164: size nit. numOfStrings(). http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.cc File be/src/util/in-list-filter.cc: http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.cc@193 PS2, Line 193: int_val good catch. http://gerrit.cloudera.org:8080/#/c/19220/2/testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test File testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test: http://gerrit.cloudera.org:8080/#/c/19220/2/testdata/workloads/functional-query/queries/QueryTest/in_list_filters.test@179 PS2, Line 179: RUNTIME_PROFILE Looks like we could beef up the new tests a little bit to demonstrate that global filters are shipped to the scan and applied. -- To view, visit http://gerrit.cloudera.org:8080/19220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie90b2bce5e5ec6f6906ce9d2090b0ab19d48cc78 Gerrit-Change-Number: 19220 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 15 Nov 2022 01:43:19 +0000 Gerrit-HasComments: Yes
