Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12563 )
Change subject: [tools] Add tool to copy table data to another table ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/12563/8/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/12563/8/src/kudu/tools/kudu-tool-test.cc@635 PS8, Line 635: std::vector<RunCopyTableCheckArgs> GenerateArgs() { There are more deduplication opportunities here if you're willing to make it less declarative. Like, you can declare a RunCopyTableCheckArgs, populate it with the all of the args that are the same, modify the one arg that's different, add it to a vector, add more entries to the vector for kTestCopyTablePredicates, then return it. http://gerrit.cloudera.org:8080/#/c/12563/8/src/kudu/tools/kudu-tool-test.cc@741 PS8, Line 741: KuduSchema CreateComplexSchema() { This function actually does a few things that may fail. Could you change it to return a Status and switch the CHECK_OK macros with RETURN_NOT_OK? You can have the KuduSchema returned via OUT parameter (i.e. KuduSchema*). http://gerrit.cloudera.org:8080/#/c/12563/8/src/kudu/tools/kudu-tool-test.cc@3047 PS8, Line 3047: RunCopyTableCheck(arg); Wrap in NO_FATALS(). http://gerrit.cloudera.org:8080/#/c/12563/8/src/kudu/tools/table_scanner.cc File src/kudu/tools/table_scanner.cc: http://gerrit.cloudera.org:8080/#/c/12563/8/src/kudu/tools/table_scanner.cc@20 PS8, Line 20: #include <cstddef> : #include <cstdint> : #include <cstring> Needs to be resorted with the includes on L24-29. -- To view, visit http://gerrit.cloudera.org:8080/12563 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifdec51701ac9ec57739b1a6f7c18786294642a54 Gerrit-Change-Number: 12563 Gerrit-PatchSet: 8 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Fri, 22 Mar 2019 17:57:39 +0000 Gerrit-HasComments: Yes
