RussellSpitzer commented on PR #17196: URL: https://github.com/apache/iceberg/pull/17196#issuecomment-5243973880
I still don't think we want to add all these public methods. All the Convert(Iceberg Thing) -> Avro Thing should not have a legacy flag. There is no ambiguity here and we don't need to emulate the old behavior by producing an incorrect type just because we used to. For the other direction convert(Avro Thing) -> Iceberg Thing. We have ambiguity based on what the old writer of the avro schema produced. Only those are ambiguous and we need the ability to handle them in either direction. ``` convert(Schema) ← no-arg, defaults adjustToUtcDefault=true convert(Schema, boolean) ← explicit adjustToUtcDefault toIceberg(Schema) ← no-arg wrapper, returns iceberg.Schema toIceberg(Schema, boolean) ← explicit adjustToUtcDefault ``` The no-arg options here should default to adjustUtcDefault = False to match AVRO_ADJUST_TO_UTC_DEFAULT_DEFAULT. There is also one other method isTimestamptz(Schema, boolean) which is public and i'm not sure it needs to be. It should probably be inlined or something like to keep it out of our public api. -- 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]
