Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14306 )
Change subject: add a tool to create table ...................................................................... Patch Set 29: (3 comments) http://gerrit.cloudera.org:8080/#/c/14306/29/src/kudu/tools/create-table-tool-test.cc File src/kudu/tools/create-table-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/14306/29/src/kudu/tools/create-table-tool-test.cc@1237 PS29, Line 1237: std::uniform_int_distribution<int> distribution_bad(0, bad_case_size - 1); This isn't quite what Adar meant by "fuzzing". Rather than selecting randomly from a set of pre-defined schemas, "fuzzing" means randomly generating a schema, and then validating that if it's supposed to be correct (e.g. it was generated as a correct schema), then the tool accepts it, and if it's not supposed to be correct (e.g. in generating the schema, an error was injected, like an incorrect enum), the tool rejects it. Like Adar said, it may be a lot of work to do something like that here, and I think I'd be happy merging this without fuzzing. That said, since randomness here isn't really "fuzzing", I'd prefer removing the new randomness and reverting back to running all these test cases (but we can keep it in this create-table-tool-test) http://gerrit.cloudera.org:8080/#/c/14306/25/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/14306/25/src/kudu/tools/tool_action_table.cc@981 PS25, Line 981: > No, here is the EncodingType define: I see, thanks for clarifying. Could you reword this then as: "If no valid encoding is provided, AUTO_ENCODING will be used by default." and below, "If no valid compression is provided, DEFAULT_COMPRESSION will be used." http://gerrit.cloudera.org:8080/#/c/14306/25/src/kudu/tools/tool_action_table.cc@984 PS25, Line 984: if (column.has_compression()) { > I find here has a warning during building. The warning message is "warning: I see. I don't mind explicit declaration. Although, take a look at consensus/leader_election.cc L164 and see if wrapping these with GCC diagnostic ignored will help. -- To view, visit http://gerrit.cloudera.org:8080/14306 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0bce4733a504f8ef5f024089a16fe3c6f1e493f1 Gerrit-Change-Number: 14306 Gerrit-PatchSet: 29 Gerrit-Owner: YangSong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: YangSong <[email protected]> Gerrit-Comment-Date: Thu, 24 Oct 2019 02:44:10 +0000 Gerrit-HasComments: Yes
