[GitHub] [iceberg] jackye1995 commented on a change in pull request #2827: Core: Add WriterFactory and delete table properties

[GitBox <gi...@apache.org>]

jackye1995 commented on a change in pull request #2827:
URL: https://github.com/apache/iceberg/pull/2827#discussion_r670729077



##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -57,25 +60,46 @@ private TableProperties() {
   public static final String MANIFEST_MERGE_ENABLED = "commit.manifest-merge.enabled";
   public static final boolean MANIFEST_MERGE_ENABLED_DEFAULT = true;
 
-  public static final String DEFAULT_FILE_FORMAT = "write.format.default";
+  private static final String DEFAULT_FILE_FORMAT_CONF = "format.default";
+
+  public static final String DEFAULT_FILE_FORMAT = WRITE_PREFIX + DEFAULT_FILE_FORMAT_CONF;
+  public static final String DELETE_DEFAULT_FILE_FORMAT = DELETE_PREFIX + DEFAULT_FILE_FORMAT_CONF;
   public static final String DEFAULT_FILE_FORMAT_DEFAULT = "parquet";
 

Review comment:
       missing `DELETE_DEFAULT_FILE_FORMAT_DEFAULT`.
   
   I am thinking if we should use avro as the default delete file format instead, because it's a row-based format, and it is likely going to be compacted and removed very soon, so it's not worth the effort for writing deletes in Parquet format. I haven't done any rigorous benchmarking on this, so this is just based on my feeling. Any thoughts?




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org