Quanlong Huang 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 3: (5 comments) Thanks for your feedbacks! 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? Just the test section of the commit message 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: unordered_set<StringValue> values; > nit may add a comment describing this pair. Done http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.h@144 PS2, Line 144: > Seems it can be replaced by emplace() to a) maintain consistency with the i Done http://gerrit.cloudera.org:8080/#/c/19220/2/be/src/util/in-list-filter.h@164 PS2, Line 164: nd(S > nit. numOfStrings(). We need to use the same method name as it's used in IN_LIST_FILTER_INSERT_BATCH: #define IN_LIST_FILTER_INSERT_BATCH(TYPE, SLOT_TYPE, PB_VAL_METHOD) \ template<> \ void InListFilterImpl<TYPE, SLOT_TYPE>::InsertBatch(const ColumnValueBatchPB& batch) { \ ... DCHECK_EQ(total_entries_, values_.size()); \ } 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: Src. Node Tgt > Looks like we could beef up the new tests a little bit to demonstrate that Done -- 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: 3 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qfc...@hotmail.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Fri, 18 Nov 2022 02:19:10 +0000 Gerrit-HasComments: Yes
