zhjwpku commented on code in PR #1080:
URL: https://github.com/apache/iceberg-cpp/pull/1080#discussion_r4464927339


##########
src/iceberg/table_metadata.h:
##########
@@ -132,11 +132,21 @@ struct ICEBERG_EXPORT TableMetadata {
   /// A list of encrypted table key entries
   std::vector<EncryptedKey> encryption_keys;
 
+  /// \brief Create table metadata from user-supplied properties.
+  /// \note The reserved format-version property selects the table format 
version,
+  /// defaulting to kDefaultTableFormatVersion, and is not persisted. Other 
reserved
+  /// properties are rejected. The input properties are not modified.
   static Result<std::unique_ptr<TableMetadata>> Make(
       const iceberg::Schema& schema, const iceberg::PartitionSpec& spec,
       const iceberg::SortOrder& sort_order, const std::string& location,
-      const std::unordered_map<std::string, std::string>& properties,
-      int format_version = kDefaultTableFormatVersion);
+      const std::unordered_map<std::string, std::string>& properties);
+
+  /// \brief Create table metadata with an explicit supported format version.
+  /// \note All reserved properties, including format-version, are rejected.
+  static Result<std::unique_ptr<TableMetadata>> Make(
+      const iceberg::Schema& schema, const iceberg::PartitionSpec& spec,
+      const iceberg::SortOrder& sort_order, const std::string& location,
+      const std::unordered_map<std::string, std::string>& properties, int 
format_version);

Review Comment:
   Java impl uses a public overload to read format-version from 
properties(defaulting to V2), filters out reserved properties, and delegates to 
a package-private overload taking an explicit int formatVersion, see [1].
   
   We can follow Java here. But the overload with format_version was already 
public before this PR, so I'm not sure if we should make it private. WDYT?
   
   [1] 
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadata.java#L69-L119



-- 
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