Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19794 )
Change subject: [cpp-client] KUDU-3455 Reduce space complexity and speed up hash partition pruning for in-list predicate ...................................................................... Patch Set 13: (6 comments) Thanks for your advices. You can review it again. http://gerrit.cloudera.org:8080/#/c/19794/12/src/kudu/common/partition_pruner-test.cc File src/kudu/common/partition_pruner-test.cc: http://gerrit.cloudera.org:8080/#/c/19794/12/src/kudu/common/partition_pruner-test.cc@984 PS12, Line 984: generate some in-list predicates for these key columns. The old algorith > Sorry for the late reply. I will take a further more review of this patch a The unit tests which tables with hash bucket partitions has tested the old algorithm. At this, they are also tested the new algorithms, such as PartitionPrunerTest::TestHashPruning, PartitionPrunerTest::TestPruning, PartitionPrunerTest::TestHashSchemasPerRangeWithPartialPrimaryKeyRangePruning. They can make sure the new algorithm's results are the same as the old one, in other words, they has make sure the correctness of new algorithm. So based on the old one algorithm is correct, I randomly construct a lots of in-list cases to cover more combinations to which is impossible to check all the combinations, make sure the general sensors and at the same time compare with the performance. http://gerrit.cloudera.org:8080/#/c/19794/13/src/kudu/common/partition_pruner-test.cc File src/kudu/common/partition_pruner-test.cc: http://gerrit.cloudera.org:8080/#/c/19794/13/src/kudu/common/partition_pruner-test.cc@1026 PS13, Line 1026: vector<KuduPartialRow>() > nit: can be replaced by '{}'. Done http://gerrit.cloudera.org:8080/#/c/19794/13/src/kudu/common/partition_pruner-test.cc@1047 PS13, Line 1047: CHECK_EQ(v1.size(), v2.size()); : for (int i = 0; i < v1.size(); i++) { : ASSERT_EQ(v1[i], v2[i]); : } > nit: they can be replaced by ASSERT_EQ(v1, v2); Done http://gerrit.cloudera.org:8080/#/c/19794/13/src/kudu/common/partition_pruner-test.cc@1069 PS13, Line 1069: kMaxSafeLength > kMaxInListLength? Done http://gerrit.cloudera.org:8080/#/c/19794/13/src/kudu/common/partition_pruner-test.cc@1084 PS13, Line 1084: temp_values[temp_values.size() - 1] > nit: temp_values.back() ? Done http://gerrit.cloudera.org:8080/#/c/19794/13/src/kudu/common/partition_pruner-test.cc@1085 PS13, Line 1085: j++; > nit: Move it to line 1081 which is more conventional. Done -- To view, visit http://gerrit.cloudera.org:8080/19794 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4bea5c10b4ac2c62b85625fe9d2a33ceb4fb2e9 Gerrit-Change-Number: 19794 Gerrit-PatchSet: 13 Gerrit-Owner: Yuqi Du <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Mon, 26 Jun 2023 07:12:29 +0000 Gerrit-HasComments: Yes
