kinolaev commented on PR #17196:
URL: https://github.com/apache/iceberg/pull/17196#issuecomment-4994374394

   > The moment we touch TypeToSchema we are potentially producing records that 
would break the rest of the library.
   
   I see your point. I see your point. However, I suspect there are many places 
where schemas produced with `legacyTimestampMapping=false` will require 
additional work on the data side. For example, if this PR makes it to the main 
branch, I'd like to propose the corresponding changes to the Flink 
`AvroGenericRecordToRowDataMapper` and `RowDataToAvroGenericRecordConverter` 
classes (#17200).
   
   Since the new behavior of `TypeToSchema` is flagged, support on the data 
side can be added gradually by people who are interested in following Avro 
semantics when converting data to/from Iceberg. However, any data conversions 
will require updated schemas first.
   
   > Or restrict the changes here just to the read side.
   
   If this is an option, I'd like to go this route. Personally, I am only 
interested in the read side. I simply found that adding symmetric support to 
the write side was quite easy and might be useful for others (given that there 
are several similar threads). That is the only reason why I included the 
changes to `TypeToSchema` to this PR and to 
`RowDataToAvroGenericRecordConverter` to PR #17200.
   
   Otherwise, I will need some help with the data side. @AnatolyPopov , is 
there a chance you could rebase your PR #15437 on my branch and add support for 
the changes in `DataWriter`?
   
   > DataReader.java:163-164
   PlannedDataReader.java:171-172
   DataWriter.java:140-141
   
   @RussellSpitzer , are there other places where you'd like to have Avro 
native timestamp support?


-- 
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]

Reply via email to