David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1623. Properly handle UPSERTS that only include PK column ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/4441/2/src/kudu/tablet/key_value_test_schema.h File src/kudu/tablet/key_value_test_schema.h: Line 33: struct KeyValueTestRow { nit: I find this choice of name confusing. How about: ExpectedTestKeyValueRow or something? http://gerrit.cloudera.org:8080/#/c/4441/2/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: PS2, Line 462: bet typo Line 465: if (buf.size() == 0) { it would be slightly clearer to do enc->is_empty() instead of testing the buffer size http://gerrit.cloudera.org:8080/#/c/4441/2/src/kudu/tablet/tablet_random_access-test.cc File src/kudu/tablet/tablet_random_access-test.cc: Line 119: VLOG(1) << RowOperationsPB::Type_Name(op.type) << " " << op.row->ToString(); add log output before this to explain what this output is? Line 169: int val, hum, re wrapping in this case don't we usually align the variables with the first one, or am I misremembering? -- To view, visit http://gerrit.cloudera.org:8080/4441 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic878f6e51ead5bf91335ddb47536e7c29de11ac4 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
