RussellSpitzer commented on code in PR #17196:
URL: https://github.com/apache/iceberg/pull/17196#discussion_r3590766925
##########
core/src/main/java/org/apache/iceberg/avro/AvroSchemaUtil.java:
##########
@@ -56,37 +56,75 @@ private AvroSchemaUtil() {}
private static final Schema.Type RECORD = Schema.Type.RECORD;
public static Schema convert(org.apache.iceberg.Schema schema, String
tableName) {
- return convert(schema, ImmutableMap.of(schema.asStruct(), tableName));
+ return convert(schema, tableName, true);
+ }
+
+ public static Schema convert(
Review Comment:
I think this approach is trying to solve essetnially two problems at the
same time but I think they should be fixed in different ways.
On Read, we shouldn't have any optional behavior, if you see the new types
you see the new types. Not an issue. So there we just need to add support for
the new types.
Reads
1. See local timestamp - use withoutZone
2. timestamp and adjust-to-utc
3. timestamp and no prop - Check table Config
On the write side
Instead of a parameter we plumb through all these methods, we should just
have a table property similar to AVRO_COMPRESSION and a private method which
takes it's resolution. Something like
```java
// In TableProperties:
String AVRO_TIMESTAMP_ENCODING = "write.avro.timestamp-encoding";
String AVRO_TIMESTAMP_ENCODING_DEFAULT = "legacy"; // or "local-timestamp"
// In Avro.WriteBuilder.build():
boolean legacy =
!"local-timestamp".equals(config.get(AVRO_TIMESTAMP_ENCODING));
schema = AvroSchemaUtil.convert(icebergSchema, name, legacy); // internal
call, not public
```
--
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]
