kinolaev commented on code in PR #17196:
URL: https://github.com/apache/iceberg/pull/17196#discussion_r3605429116
##########
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:
I like the idea of non-legacy behavior of the `AvroSchemaUtil` methods by
default.
The problem is that `AvroSchemaUtil` is in the `org.apache.iceberg.avro`
package while (Planned)DataReader and DataWriter are in the
`org.apache.iceberg.data.avro` package. Because of this, we can't just make the
`AvroSchemaUtil` methods with `legacyTimestampMapping` parameter
package-private. Or have I misunderstood you?
We can set `legacyTimestampMapping` to `false` by default in
`AvroSchemaUtil`. But then we should check usages in the other Iceberg
libraries - `flink`, `kafka-connect` and `parquet`.
In Flink's we can most probably just flip the corresponding
`legacyTimestampMapping` parameter to `false` by default in
`AvroToRowDataConverters.createRowConverter` and
`RowDataToAvroConverters.createConverter` to match the new default behavior of
`AvroSchemaUtil`.
I don't know if we could break anything in `kafka-connect` or `parquet` that
way. In general, I think your intuition is right.
Here is the draft - 3fe6c4dad40ad5794051245748f842d890803071. Is that close
to what you were imagining?
--
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]
