Andrew Wong has posted comments on this change. Change subject: KUDU-1880 Prevent eager ignoring of NULLs ......................................................................
Patch Set 9: (9 comments) http://gerrit.cloudera.org:8080/#/c/6029/6/src/kudu/common/column_materialization_context.h File src/kudu/common/column_materialization_context.h: PS6, Line 79: urn pred_ && pred_- > If I understand correctly, you need to be able to tell whether there is an Done http://gerrit.cloudera.org:8080/#/c/6029/6/src/kudu/tablet/all_types-scan-correctness-test.cc File src/kudu/tablet/all_types-scan-correctness-test.cc: PS6, Line 12: RowOpsBase > how about RowOpsBase? (for instance I'll likely want to add a Timestamp tes Done PS6, Line 79: > > don't need the extra space anymore (since c++11) here and everywhere else Done PS6, Line 98: , > comma after "repeated" and after "non-null" Done PS6, Line 133: dinality, and null_upper. R > thanks for doing this. it looks soooo much better. Done PS6, Line 133: // cardinality, and null_upper. RowOps is a specialized Row > could you add a summary of the test to the class header? (not too much deta Thanks for the recommendation! I agree it looks waaaay better! Plus it's always nice to learn new C++ techniques. Will add a couple sentences. PS6, Line 254: > we should likely be explicit (i.e set the encoding that AUTO_ENCODING resol Done PS6, Line 283: > spaces (two) not tabs Done PS6, Line 284: > same Done -- To view, visit http://gerrit.cloudera.org:8080/6029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib418a4fcc2794ce2f686e864f51834fb4fb8b048 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes