kinolaev commented on PR #17196: URL: https://github.com/apache/iceberg/pull/17196#issuecomment-5245600055
Thanks @RussellSpitzer! Does this mean you want to get rid of the `write.avro.local-timestamp.enabled` table property and make `Avro.write` encode `timestamp(_ns)` as `local-timestamp-*` unconditionally? There is an edge case covered in `core/src/test/java/org/apache/iceberg/TestTimestampPartitions.java`: when a partition spec contains an `identity(timestamp)` field, according to the spec, it should be encoded in manifests as `timestamp-micros` with `adjust-to-utc=false`. Do you want me to update the spec to allow encoding `timestamp(_ns)` as `local-timestamp-*` in manifests? > convert(Schema) ← no-arg, defaults adjustToUtcDefault=true > The no-arg options here should default to adjustUtcDefault = False to match AVRO_ADJUST_TO_UTC_DEFAULT_DEFAULT. Could you please clarify your position on the default value for `adjustUtcDefault`? I'd prefer to keep the Avro-compliant default (`true`) in `AvroSchemaUtil`, as it is not specific to Iceberg, and can be used with externally produced schemas. I don't think it should match `AVRO_ADJUST_TO_UTC_DEFAULT_DEFAULT`, because the latter is specific to Iceberg, and changing it would be a breaking change. -- 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]
