kinolaev commented on code in PR #17196:
URL: https://github.com/apache/iceberg/pull/17196#discussion_r3633244845
##########
core/src/main/java/org/apache/iceberg/data/avro/DataReader.java:
##########
@@ -49,21 +49,37 @@ public static <D> DataReader<D> create(
public static <D> DataReader<D> create(
org.apache.iceberg.Schema expectedSchema, Schema readSchema,
Map<Integer, ?> idToConstant) {
- return new DataReader<>(expectedSchema, readSchema, idToConstant);
+ return create(expectedSchema, readSchema, idToConstant, true);
+ }
+
+ public static <D> DataReader<D> create(
Review Comment:
@AnatolyPopov and @RussellSpitzer, no more changes planned from my side and
I would appreciate your next review whenever you have a chance!
I dropped `adjust-to-utc-default`. I previously added it because a schema
property was the only way to pass parameters to
`(Planned)DataReader`/`DataWriter`. However, I realized I was just replacing
one Iceberg-specific convention with another, and there is clearly no point in
doing that.
The core rule remains the same: methods without parameters align with the
Avro spec, while internal usages pass parameters to align with the Iceberg spec.
Specifically, Iceberg->Avro schema conversion methods produce
`local-timestamp-*` and `timestamp-*` with `adjust-to-utc=true`, and
Avro->Iceberg schema conversion methods interpret `timestamp-*` without
`adjust-to-utc` as `.withZone()` by default.
`DataReader` and `PlannedDataReader` handle `timestamp-*` without
`adjust-to-utc` as `OffsetDateTime`, and `DataWriter` expects `OffsetDateTime`
by default. In `GenericAppenderFactory`, `GenericFileWriterFactory`,
`RewriteTablePathSparkAction`, `IcebergEncoder` and `IcebergDecoder` it is
guaranteed that the reader/writer receive schemas that don't contain
`timestamp-*` without `adjust-to-utc`, and the `adjustToUtcDefault` constructor
parameter has no effect, that is why the default constructors are used. In
`GenericFormatModels`, where it is not guaranteed, `adjustToUtcDefault` is
pinned to `false` to align with the Iceberg spec.
In all manifest and encryption related schema conversions
`localTimestampEnabled` is explicitly set to `false`.
The `write.avro.local-timestamp.enabled` table property is documented.
--
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]
