wgtmac commented on code in PR #416:
URL: https://github.com/apache/iceberg-cpp/pull/416#discussion_r2650246525


##########
src/iceberg/table_metadata.h:
##########
@@ -210,8 +219,13 @@ class ICEBERG_EXPORT TableMetadataBuilder : public 
ErrorCollector {
   /// \brief Create a builder from existing table metadata
   ///
   /// \param base The base table metadata to build from
-  /// \return A new TableMetadataBuilder instance initialized with base 
metadata
-  static std::unique_ptr<TableMetadataBuilder> BuildFrom(const TableMetadata* 
base);
+  /// \param is_create Whether the builder is for creating a new table. It 
will call

Review Comment:
   Then can we do something like this?
   
   ```cpp
   Transaction::Transaction(std::shared_ptr<Table> table, Kind kind, bool 
auto_commit,
                            std::unique_ptr<TableMetadataBuilder> 
metadata_builder)
       : table_(std::move(table)),
         kind_(kind),
         auto_commit_(auto_commit),
         metadata_builder_(std::move(metadata_builder)) {}
   
   Result<std::shared_ptr<Transaction>> 
Transaction::Make(std::shared_ptr<Table> table,
                                                          Kind kind, bool 
auto_commit) {
     if (!table || !table->catalog()) [[unlikely]] {
       return InvalidArgument("Table and catalog cannot be null");
     }
   
     const auto* metadata = table->metadata().get();
     std::unique_ptr<TableMetadataBuilder> metadata_builder;
     switch (kind) {
       case Kind::kCreate: {
         metadata_builder = TableMetadataBuilder::BuildFromEmpty();
         ICEBERG_RETURN_UNEXPECTED(
             TableMetadataUtil::ApplyCreateChanges(metadata, 
*metadata_builder));
       } break;
       case Kind::kUpdate: {
         metadata_builder = TableMetadataBuilder::BuildFrom(metadata);
       } break;
       default:
         return NotSupported("Unsupported transaction kind: {}", 
static_cast<int>(kind));
     }
   
     return std::shared_ptr<Transaction>(
         new Transaction(std::move(table), kind, auto_commit, 
std::move(metadata_builder)));
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to