Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10983 )
Change subject: Kudu-1291 Efficiently support predicates on non-prefix key components ...................................................................... Patch Set 5: (5 comments) still reviewing, here are some initial comments http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/cfile/cfile_reader.h File src/kudu/cfile/cfile_reader.h: http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/cfile/cfile_reader.h@340 PS5, Line 340: set_current_value document this parameter in the header file comment http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/tablet/cfile_set.cc File src/kudu/tablet/cfile_set.cc: http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/tablet/cfile_set.cc@413 PS5, Line 413: // Initialize predicate column id to an upperbound value nit: add punctuation to all your comments (add a period at the end of the sentence) http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/tablet/cfile_set.cc@511 PS5, Line 511: // Increment "num_cols" no of columns this kind of api doc should go into the header file, not the implementation http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/tablet/cfile_set.cc@527 PS5, Line 527: &exact_match since we are not using this, can we just make this /* exact_match= */ nullptr ? http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/tablet/index_skipscan-test.cc File src/kudu/tablet/index_skipscan-test.cc: http://gerrit.cloudera.org:8080/#/c/10983/5/src/kudu/tablet/index_skipscan-test.cc@400 PS5, Line 400: int predicate_col_id = 2; please add a random test case that does not put the predicate on the last column -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 5 Gerrit-Owner: Anupama Gupta <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Anupama Gupta <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Thu, 02 Aug 2018 00:41:02 +0000 Gerrit-HasComments: Yes
