Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16859 )
Change subject: [master] Range specific hashing at table creation time. ...................................................................... Patch Set 5: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/16859/5/src/kudu/common/partition-test.cc File src/kudu/common/partition-test.cc: http://gerrit.cloudera.org:8080/#/c/16859/5/src/kudu/common/partition-test.cc@1166 PS5, Line 1166: { : {0}, string("a1\0\0\0\0c1", 8), string("a2\0\0\0\0c2", 8), : string("\0\0\0\0" "a1\0\0\0\0c1", 12), string("\0\0\0\0" "a2\0\0\0\0c2", 12) : }, nit: the GSG is a bit fuzz here since there are multiple layers of initializer lists, but i'd suggest: { {0}, string, string, // four spaces string, string // four extra from previous line since this is a continuation of the previous line }, https://google.github.io/styleguide/cppguide.html#Braced_Initializer_List_Format http://gerrit.cloudera.org:8080/#/c/16859/5/src/kudu/common/partition-test.cc@1218 PS5, Line 1218: if (d1.range_key_start == d2.range_key_start) { : return d1.partition_key_start < d2.partition_key_start; : } : return d1.range_key_start < d2.range_key_start; Aren't all of the partition_key_start values unique? Can't we just return d1.partition_key_start < d2.partition_key_start? -- To view, visit http://gerrit.cloudera.org:8080/16859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Gerrit-Change-Number: 16859 Gerrit-PatchSet: 5 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Thu, 17 Dec 2020 01:24:50 +0000 Gerrit-HasComments: Yes
