David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1893 Ensure evaluation of added columns ......................................................................
Patch Set 6: (9 comments) http://gerrit.cloudera.org:8080/#/c/6129/6/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: PS6, Line 599: type; think you missed my comment in the previous rev about getting the type name from the type info. while you're at it please fix the method below too http://gerrit.cloudera.org:8080/#/c/6129/6/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: PS6, Line 200: Otherwuse typo http://gerrit.cloudera.org:8080/#/c/6129/6/src/kudu/tablet/all_types-scan-correctness-test.cc File src/kudu/tablet/all_types-scan-correctness-test.cc: PS6, Line 37: const int& value, const bool& altered only noticed this now, but we usually pass primitives by value (i.e. int value, bool altered), not by reference PS6, Line 59: const int& value same here and everywhere else PS6, Line 89: 32 pull up a constant for this, please PS6, Line 120: cur+1 formatting PS6, Line 257: void RunTests() { pull out a RunUnalteredTabletTests and then RunTests runs both unaltered and altered in sequence PS6, Line 327: 2000 use local constant or /* comment */ PS6, Line 332: 3 all these magic numbers make the test hard to follow, please use constants throughout -- To view, visit http://gerrit.cloudera.org:8080/6129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Ram Mettu <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
