Andrew Wong has posted comments on this change. Change subject: KUDU-1893 Ensure evaluation of added columns ......................................................................
Patch Set 6: (11 comments) http://gerrit.cloudera.org:8080/#/c/6129/2/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: PS2, Line 599: type; > Done Is there a particular reason to favor using type_info()->name() vs just the DataType enum itself here? I had the input be DataType instead of TypeInfo since it aligns more with EvaluateCell<DataType>() 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 Done 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 Done 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 va Done PS6, Line 59: const int& value > same here and everywhere else Done PS6, Line 89: 32 > pull up a constant for this, please Done PS6, Line 120: cur+1 > formatting Done PS6, Line 257: void RunTests() { > pull out a RunUnalteredTabletTests and then RunTests runs both unaltered an Done PS6, Line 327: 2000 > use local constant or /* comment */ Done PS6, Line 332: 3 > all these magic numbers make the test hard to follow, please use constants Done http://gerrit.cloudera.org:8080/#/c/6129/2/src/kudu/tablet/tablet-scan-correctness-test.cc File src/kudu/tablet/tablet-scan-correctness-test.cc: PS2, Line 475: : : : : : : : : : : : : : : : Will change this to SCOPED_TRACE -- 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
