Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14945 )
Change subject: KUDU-3032 Prevent selecting unnecessarily columns after scan optimization ...................................................................... Patch Set 2: (4 comments) Could you add an end-to-end test that exercises the scan path in tablet_service.cc too? http://gerrit.cloudera.org:8080/#/c/14945/2/src/kudu/common/scan_spec.h File src/kudu/common/scan_spec.h: http://gerrit.cloudera.org:8080/#/c/14945/2/src/kudu/common/scan_spec.h@73 PS2, Line 73: // Build missing column from subtraction between predicates and projection. Nit: how about "Get columns that are present in the predicates but not in the projection?" http://gerrit.cloudera.org:8080/#/c/14945/2/src/kudu/common/scan_spec.h@74 PS2, Line 74: std::vector<ColumnSchema> BuildMissingColumn(const Schema& projection); Nit: rename to GetMissingColumns http://gerrit.cloudera.org:8080/#/c/14945/2/src/kudu/common/scan_spec.cc File src/kudu/common/scan_spec.cc: http://gerrit.cloudera.org:8080/#/c/14945/2/src/kudu/common/scan_spec.cc@165 PS2, Line 165: std::unordered_set<string> missing_col_names; Add a using for this. http://gerrit.cloudera.org:8080/#/c/14945/2/src/kudu/common/scan_spec.cc@168 PS2, Line 168: predicate.column().name() This is repeated three times; could you store it in a local cref? -- To view, visit http://gerrit.cloudera.org:8080/14945 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82c30c5272b6fc114b710df9d6c31d5d1e319e31 Gerrit-Change-Number: 14945 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: ZhangYao <[email protected]> Gerrit-Comment-Date: Sun, 29 Dec 2019 18:57:55 +0000 Gerrit-HasComments: Yes
