David Ribeiro Alves has posted comments on this change. Change subject: KUDU-237 (part 1) - Support proper mutation encoding for reinserts ......................................................................
Patch Set 16: (4 comments) http://gerrit.cloudera.org:8080/#/c/4791/16/src/kudu/common/row_changelist-test.cc File src/kudu/common/row_changelist-test.cc: Line 150: EXPECT_EQ(string("REINSERT col2=world, col3=12345, col4=NULL"), > I'm surprised not to see col1=hello here. Can you add a comment why it's no yeah, added the note you suggested. http://gerrit.cloudera.org:8080/#/c/4791/16/src/kudu/common/row_changelist.h File src/kudu/common/row_changelist.h: Line 221: // it to work for REINSERT and UPDATE. > nit: can you say 'both REINSERT and UPDATE'? Done Line 250: const ColumnSchema& col_schema = schema->column_by_id(col_id); > this is producing another map lookup, I think better to just use schema->co Done http://gerrit.cloudera.org:8080/#/c/4791/16/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: Line 634: namespace { // anonymous namespace > this '// anonymous namespace' comment belongs with the closing brace below Done -- To view, visit http://gerrit.cloudera.org:8080/4791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2b0a95e3f618bf20eb18ca4f4b03b9e2ce5704e8 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
