Abhishek Chennaka has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19698 )

Change subject: [tools] KUDU-1945 Print auto-incrementing counter in kudu wal 
dump
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/19698/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19698/4/src/kudu/tools/kudu-tool-test.cc@2726
PS4, Line 2726: }
              :
              : TEST_F(ToolTest, TestWalDumpWithAutoIncrementingColumn) {
              :   c
> Is this a duplicate?  If it's needed, would be great to add a comment to ex
It's duplicate. Removed it now.


http://gerrit.cloudera.org:8080/#/c/19698/4/src/kudu/tools/kudu-tool-test.cc@2736
PS4, Line 2736: ayout());
> Aren't these the same as per https://kudu.apache.org/docs/command_line_tool
Removed the duplicate values.


http://gerrit.cloudera.org:8080/#/c/19698/4/src/kudu/tools/kudu-tool-test.cc@2743
PS4, Line 2743:                         /*file_cache*/nullptr,
              :                         kTestTablet,
              :                         schema_with_id,
> Does it make sense to add a scenario to make sure these entries (without ex
Done


http://gerrit.cloudera.org:8080/#/c/19698/4/src/kudu/tools/kudu-tool-test.cc@2750
PS4, Line 2750:
> Remove the logical dups?
Done


http://gerrit.cloudera.org:8080/#/c/19698/4/src/kudu/tools/kudu-tool-test.cc@2757
PS4, Line 2757:     
write->mutable_auto_incrementing_column()->set_auto_incrementing_counter(0x5a);
              :     ASSERT_OK(SchemaToPB(schema, write->mutable_schema()));
              :     AddTestRowToPB(RowOperationsPB::INSERT, schema,
> nit for here and below: for these ASSERT_STR_NOT_MATCHES() it would make a
Done



--
To view, visit http://gerrit.cloudera.org:8080/19698
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e807aaef48683ec7c5317eecdedf8e6e15950e2
Gerrit-Change-Number: 19698
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Comment-Date: Mon, 17 Apr 2023 19:05:22 +0000
Gerrit-HasComments: Yes

Reply via email to