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 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/10983/10/src/kudu/cfile/cfile_reader.h File src/kudu/cfile/cfile_reader.h: http://gerrit.cloudera.org:8080/#/c/10983/10/src/kudu/cfile/cfile_reader.h@59 PS10, Line 59: static const int kPrimaryKeyMaxSize = 16*1024; > nit: is this a hard limit or just a target? How is this enforced? Great question. This is from http://kudu.apache.org/docs/known_issues.html#_primary_keys and we haven't investigated how it is enforced yet or whether there is another constant available for this, although I grepped around and didn't find one. http://gerrit.cloudera.org:8080/#/c/10983/10/src/kudu/cfile/cfile_reader.h@473 PS10, Line 473: // Currently, it is assumed that CFileSet::Iterator::SkipToNextScan(size_t *remaining) : // has exclusive access to use this variable. This breaks encapsulation. We should document something along these lines in CFileSet::Iterator itself with respect to the validx_iter_ variable; this class should not reference a separate "client" class that uses it, which would be especially surprising considering this is a private variable. http://gerrit.cloudera.org:8080/#/c/10983/10/src/kudu/tablet/cfile_set.h File src/kudu/tablet/cfile_set.h: http://gerrit.cloudera.org:8080/#/c/10983/10/src/kudu/tablet/cfile_set.h@353 PS10, Line 353: // To track whether the lower bound prefix key rolled over between the first and : // second seek to determine the lower bound key. How about change this to: // Whether the skip scan optimization has searched the current prefix for a predicate match or whether the prefix has changed since its last check. -- 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: 10 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: Tue, 07 Aug 2018 19:43:24 +0000 Gerrit-HasComments: Yes
