kinolaev commented on PR #17196: URL: https://github.com/apache/iceberg/pull/17196#issuecomment-5254238829
@RussellSpitzer, I've reread our discussion. > Instead of a parameter we plumb through all these methods, we should just have a table property similar to AVRO_COMPRESSION and a private method which takes it's resolution Based on this suggestion, I dropped legacy Iceberg->Avro public methods, but kept the `write.avro.local-timestamp.enabled` table property. `AvroSchemaUtil.isTimestamptz` has been inlined into `(Planned)DataReader`, `DataWriter`, and `SchemaToType`, the method was removed. String values `"true"`/`"false"` are no longer supported (they never were spec-compliant). If you want to keep string support, I think it would be better to retain `AvroSchemaUtil.isTimestamptz` (or add `AvroSchemaUtil.getBooleanProp(String name, boolean defaultValue)` instead). Two questions remain: 1. Partition fields `identity(timestamp)`/`identity(timestamp_ns)`/`void(timestamp)`/`void(timestamp_ns)` are now encoded as `local-timestamp-*` in manifests, which breaks old readers - is that acceptable? For now, I've added `local-timestamp-*` to the spec to reflect the change. Fortunately, there are no other `timestamp`/`timestamp_ns` types in the manifest schemas for now. 2. `adjustToUtcDefault`: it is now Avro-compliant (`true`) in `AvroSchemaUtil`, `DataWriter` and readers (configurable via `SupportsLocalTimestamp`), and Iceberg-compliant (`false`) in manifests and for the `read.avro.adjust-to-utc.default` table property. Do you prefer to have it Iceberg-compliant (`false`) everywhere? -- 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]
