szehon-ho commented on PR #8299: URL: https://github.com/apache/iceberg/pull/8299#issuecomment-1679605105
Looks like there's some open question. First, I assume we want to do this, to reduce the impact for user (keep gzip for old table and use zstd for new tables). If that's the case, the questions: 1. Should we change TableMetadata constructor to set these two compression table properties for new TableMetadata? https://github.com/apache/iceberg/pull/8299#discussion_r1294836281 * Currently changing each catalog risks missing this change in custom catalog. * But TableMetadata is public API and it may be strange to have new instances default with 2 properties. But then again, maybe this is a good direction and we should start returning with other default table property as well (which is a way for us to start persisting the defaults). 2. Should we just have two compression constants (DEFAULT and NEW_TABLE_DEFAULT) and not have to persist GZIP explicitly in SnapshotProducer? https://github.com/apache/iceberg/pull/8299#discussion_r1294839051 * Its less intrusive code in SnapshotProducer, and doesnt change a public constant. But it looks a bit weird. -- 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]
