Haijie Hong has posted comments on this change.

Change subject: KUDU-1643 Prune hash partitions based on IN-list predicates
......................................................................


Patch Set 11:

(8 comments)

> (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 3 BUCKETS,
> The SQL string needs to be updated.
Done


Line 535:   auto pb = PartitionSchemaPB();
> Clearing a new PB isn't necessary.
Done


Line 578:   Check({ ColumnPredicate::InList(schema.column(0), &a_values) }, 18);
> We simplify 1-element IN list predicates to an equality, so this isn't actu
Done


PS8, Line 617: c
> 3
Done


PS8, Line 618: m
> 3
Done


http://gerrit.cloudera.org:8080/#/c/5176/8/src/kudu/common/partition_pruner.cc
File src/kudu/common/partition_pruner.cc:

Line 156: vector<bool> PartitionPruner::SearchHashOfColumnCombination(const 
PartitionSchema& partition_schema,
> I think this method would be simpler if it had an additional inner loop and
Done


Line 174:       } else if (predicate->predicate_type() == 
PredicateType::InList) {
> This shouldn't happen any more because we pre-check, right?
Done


Line 303:         break;
> This for loop can be simplified to 
Done


-- 
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: 11
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

Reply via email to