Dan Burkert has posted comments on this change. Change subject: KUDU-1643 Prune hash partitions based on IN-list predicates ......................................................................
Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/5176/8/src/kudu/common/partition_pruner-test.cc File src/kudu/common/partition_pruner-test.cc: Line 525: // DISTRIBUTE BY HASH(a) INTO 2 BUCKETS, The SQL string needs to be updated. Line 535: pb.mutable_range_schema()->Clear(); Clearing a new PB isn't necessary. Line 578: Check({ ColumnPredicate::InList(schema.column(0), &a_values) }, 9); We simplify 1-element IN list predicates to an equality, so this isn't actually testing the IN list pruning. PS8, Line 617: 2 3 PS8, Line 618: 2 3 http://gerrit.cloudera.org:8080/#/c/5176/8/src/kudu/common/partition_pruner.cc File src/kudu/common/partition_pruner.cc: Line 156: void PartitionPruner::SearchHashOfColumnCombination(const PartitionSchema& partition_schema, I think this method would be simpler if it had an additional inner loop and an accumulating vector<string> instead of being recursive. The 'col_offset', 'hash_bucket_count', 'encoded_string' variables could be made into locals instead of parameters, and the hash_bucket_bitset could be returned instead of taken as an out-param. Line 174: if (predicate == nullptr) return; This shouldn't happen any more because we pre-check, right? Line 303: for (int col_offset = 0; col_offset < hash_bucket_schema.column_ids.size(); col_offset++) { This for loop can be simplified to for (ColumnId column_id : hash_bucket_schema.column_ids) { const ColumnSchema& column = schema.column_by_id(column_id); ... -- To view, visit http://gerrit.cloudera.org:8080/5176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I578ac3c5e6770b13b0bebe01127d7d8263aceb36 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Haijie Hong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
