Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19890 )
Change subject: [tools] KUDU-1945: Kudu table copy and perf loadgen ...................................................................... Patch Set 7: (4 comments) Overall LGTM, just a few nits to address. http://gerrit.cloudera.org:8080/#/c/19890/6/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/19890/6/src/kudu/tools/table_scanner.cc@770 PS6, Line 770: CHECK(dst_table->schema() == src_table->schema()); > Right, makes sense. I'll replace it with CHECK(). I'd suggest to avoid crashing here because of CHECK() if the schemas of two tables are different, but instead just return Status::InvalidArgument() http://gerrit.cloudera.org:8080/#/c/19890/7/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/19890/7/src/kudu/tools/table_scanner.cc@880 PS7, Line 880: -1 nit: consider using Schema::kColumnNotFound constant instead http://gerrit.cloudera.org:8080/#/c/19890/7/src/kudu/tools/table_scanner.cc@889 PS7, Line 889: nit: wrong indent http://gerrit.cloudera.org:8080/#/c/19890/7/src/kudu/tools/table_scanner.cc@891 PS7, Line 891: ditto -- To view, visit http://gerrit.cloudera.org:8080/19890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754a7e84c16d1f3b2d52be937e1eb50b3d00d759 Gerrit-Change-Number: 19890 Gerrit-PatchSet: 7 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: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 24 May 2023 01:36:10 +0000 Gerrit-HasComments: Yes
