Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14503 )
Change subject: KUDU-2980: fix ORDERED scans when key columns are misordered in projection ...................................................................... Patch Set 4: Code-Review+1 (4 comments) LGTM, just a nit and some questions. http://gerrit.cloudera.org:8080/#/c/14503/4/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/14503/4/src/kudu/client/client-test.cc@6391 PS4, Line 6391: // Generate a somewhat random schema with half of the columns in the primary key. Maybe I'm missing something, but isn't this not random at all, in that this generates the same schema every time? It's cycling through the exact same types with the same starting type, and so it's wholly defined by that type ordering and a couple of constants, no? http://gerrit.cloudera.org:8080/#/c/14503/4/src/kudu/client/client-test.cc@6445 PS4, Line 6445: // Insert some rows with randomized keys, flushing the tablet after each one. Do you think it's worth flushing less frequently than once per row so we get rowsets that span more than one key? http://gerrit.cloudera.org:8080/#/c/14503/4/src/kudu/client/client-test.cc@6485 PS4, Line 6485: all_col_names Just verifying my understanding -- this is where the magic of this test is, right? We may end up predicating on a key column that isn't being projected, and so previously, a key being a part of missing_cols could be reordered to the end? http://gerrit.cloudera.org:8080/#/c/14503/4/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/14503/4/src/kudu/tserver/tablet_service.cc@2332 PS4, Line 2332: It doesn't know which of its columns are key columns. nit: maybe rephrase this since it's not true for ORDERED scans? -- To view, visit http://gerrit.cloudera.org:8080/14503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb213f466c7d01b6953a863d8aa53a34b5ac8893 Gerrit-Change-Number: 14503 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Comment-Date: Wed, 23 Oct 2019 05:45:04 +0000 Gerrit-HasComments: Yes
