Adar Dembo has posted comments on this change. Change subject: WIP: Expose row format flags in KuduScanner ......................................................................
Patch Set 6: (9 comments) http://gerrit.cloudera.org:8080/#/c/6624/6/src/kudu/client/client.cc File src/kudu/client/client.cc: PS6, Line 1247: Raw mode "Row format flags" PS6, Line 1254: raw mode "row format" http://gerrit.cloudera.org:8080/#/c/6624/6/src/kudu/client/client.h File src/kudu/client/client.h: PS6, Line 2053: 0; Should this be NO_FLAGS? http://gerrit.cloudera.org:8080/#/c/6624/6/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: Line 545: if (row_format_flags_ != KuduScanner::NO_FLAGS) { Why is this check a LOG(FATAL) one here but a DCHECK_EQ() one in KuduScanBatch::Data::row()? PS6, Line 546: Format modifier Row format http://gerrit.cloudera.org:8080/#/c/6624/6/src/kudu/client/scanner-internal.h File src/kudu/client/scanner-internal.h: PS6, Line 272: modifiers flags Line 304: int64_t row_format_flags_; Nit: since every other data member here is documented, could you document this too? http://gerrit.cloudera.org:8080/#/c/6624/6/src/kudu/tserver/tserver.proto File src/kudu/tserver/tserver.proto: PS6, Line 274: IN Nit: in (or did you capitalize this for a reason?) PS6, Line 275: 0 Shouldn't this be RowFormatFlags::NO_FLAGS? -- To view, visit http://gerrit.cloudera.org:8080/6624 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I043b6514dc5fc307fc9c94eb41f3ae79796ba273 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
