Jean-Daniel Cryans has posted comments on this change. Change subject: Add ColumnPredicate class ......................................................................
Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/2137/3/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: Line 46: I'd like to see more documentation on what this class really does, explaining the core concepts, why we need it. Line 107: set nit: sets, or change the "does" below to "do" Line 109: evalutes nit: evaluates Line 122: // Returns true if the column predicates are equivalent. Worth mentioning that it returns false if the columns aren't the same, while in other places we CHECK? I see that's how you want it internally, but it seems inconsistent if not documented. -- To view, visit http://gerrit.cloudera.org:8080/2137 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I72cac35d7a168f96c2ee52afd284489e99755e3f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
