ZhangYao 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) > (4 comments) > > Could you add an end-to-end test that exercises the scan path in > tablet_service.cc too? Added :) 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 t Done 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 Done 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. Done 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? Done -- 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: Sat, 04 Jan 2020 04:59:49 +0000 Gerrit-HasComments: Yes
