Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17917 )
Change subject: KUDU-3326 : Add soft delete table supports ...................................................................... Patch Set 5: (2 comments) Just a quick preliminary review: pin-pointed a couple of items related to the design and the C++ client library ABI compatiblity. http://gerrit.cloudera.org:8080/#/c/17917/5/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/17917/5/src/kudu/client/client.h@690 PS5, Line 690: bool force_on_trashed_table = false, : uint32_t reserve_seconds = 0); I guess this change will break ABI compatibility: applications build against prior version of the Kudu C++ client library will fail to work with the newer library. We tend to keep ABI compatibility, see https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B for details. In particular, this change is covered by the following item: * extending a function with another parameter, even if this parameter has a default argument http://gerrit.cloudera.org:8080/#/c/17917/5/src/kudu/client/client.h@714 PS5, Line 714: /// Recall a deleted but still reserved table. : /// : /// @param [in] table_name : /// Name of the table to recall. : /// @return Operation status. : : Status RecallTable(const std::string& table_name); What if there were many tables with the same name created and then deleted? BTW, in Kudu there is a way to address a table using table UUID -- that way you can uniquely address a table, even after renamings, etc. Also, under what name the table is restored -- what if a table with 'table_name' already exists by the time this method is called? -- To view, visit http://gerrit.cloudera.org:8080/17917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d1dddfbca55a5c4bcac4028157325ad618ea665 Gerrit-Change-Number: 17917 Gerrit-PatchSet: 5 Gerrit-Owner: KeDeng <[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-Comment-Date: Wed, 13 Oct 2021 18:03:02 +0000 Gerrit-HasComments: Yes
