Alexey Serbin 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 20: (5 comments) http://gerrit.cloudera.org:8080/#/c/10983/19//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10983/19//COMMIT_MSG@19 PS19, Line 19: until > nit: only one 'l' Done http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/cfile/cfile_reader.cc File src/kudu/cfile/cfile_reader.cc: http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/cfile/cfile_reader.cc@794 PS19, Line 794: 8192 > nit: should probably be using FLAGS_max_encoded_key_size_bytes here FLAGS_max_encoded_key_size_bytes is not available here and I'm not sure I want to move the definition of that flag in here or re-shuffle libraries only to remove it in the follow-up patch: as we discussed offline, it's better to use arena passed as a for these methods from upper level (i.e. from cfile_set). http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/cfile_set.h File src/kudu/tablet/cfile_set.h: http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/cfile_set.h@303 PS19, Line 303: & cur_enc_key, : KuduPartialRow* p_row, > nit: reverse sigil-space order Done http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/cfile_set.h@354 PS19, Line 354: il > nit: here too Done http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/index_skipscan-test.cc File src/kudu/tablet/index_skipscan-test.cc: http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/index_skipscan-test.cc@168 PS19, Line 168: : Status s = writer.Insert(row); : // As keys are inserted randomly, retry row insertion in case : // the current row insertion failed due to duplicate value. : if (s.IsAlreadyPresent()) { : continue; : } else { : CHECK_OK(s); > Does it matter what these values are? It seems like the predicate column wi No, it's not that important. Yes, there will be only three different values and that's what was desired, yep. The random approach was exercised by the other generator, but if you like randomness here as well, I'll add that. The only important point here is to have not less than num_rows/3 values matching the predicate. -- 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: 20 Gerrit-Owner: Anupama Gupta <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[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: Mon, 20 Aug 2018 17:50:01 +0000 Gerrit-HasComments: Yes
