wombatu-kun commented on code in PR #17024:
URL: https://github.com/apache/iceberg/pull/17024#discussion_r3510173697
##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/EvolveSchemaVisitor.java:
##########
@@ -211,8 +212,15 @@ private void updateColumn(Types.NestedField existingField,
Types.NestedField tar
String existingColumnName =
this.existingSchema.findColumnName(existingField.fieldId());
boolean needsOptionalUpdate = targetField.isOptional() &&
existingField.isRequired();
+ boolean narrowingHandledByConversion =
+ existingField.type().isPrimitiveType()
+ && targetField.type().isPrimitiveType()
+ && TypeUtil.isPromotionAllowed(
Review Comment:
`TypeUtil.isPromotionAllowed` does not cover date -> timestamp_without_zone,
but `CompareSchemasVisitor.primitive` treats it as `DATA_CONVERSION_NEEDED` and
`DataConverter.get` converts it. So a record that both adds a column (forcing
SCHEMA_UPDATE_NEEDED) and carries a date for a timestamp table column still
reaches `api.updateColumn(col, date)` here and throws `Cannot change column
type: ...: timestamp -> date` - the same failure this fix removes for int->long
and float->double. Mirror the conversion set `CompareSchemasVisitor` recognizes
(add date -> timestamp), ideally by extracting a single shared predicate both
visitors call so the two cannot drift, and add a test for the add-column +
date->timestamp case.
--
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]
