fmorillo7694 commented on code in PR #17900:
URL: https://github.com/apache/iceberg/pull/17900#discussion_r4159315569


##########
flink/v2.3/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DataConverter.java:
##########
@@ -124,6 +124,8 @@ static DataConverter get(LogicalType sourceType, 
LogicalType targetType, boolean
         return new ArrayConverter((ArrayType) sourceType, (ArrayType) 
targetType, caseSensitive);
       case MAP:
         return new MapConverter((MapType) sourceType, (MapType) targetType, 
caseSensitive);
+      case VARIANT:
+        return object -> object;

Review Comment:
   Done, `VARIANT` is now part of the identity block.



##########
flink/v2.3/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/CompareSchemasVisitor.java:
##########
@@ -181,6 +181,20 @@ public Result primitive(Type.PrimitiveType primitive, 
Integer tableSchemaId) {
     }
   }
 
+  @Override
+  public Result variant(Types.VariantType variant, Integer tableSchemaId) {
+    if (tableSchemaId == null) {
+      return Result.SCHEMA_UPDATE_NEEDED;
+    }
+
+    Type tableSchemaType = tableSchema.findField(tableSchemaId).type();
+    if (tableSchemaType.isVariantType()) {
+      return Result.SAME;
+    }
+
+    return Result.SCHEMA_UPDATE_NEEDED;

Review Comment:
   Done.



##########
flink/v2.3/flink/src/test/java/org/apache/iceberg/flink/sink/dynamic/TestRowDataConverter.java:
##########
@@ -85,6 +86,19 @@ void testPreservesRowKind() {
     assertThat(convert(deleteRow, SCHEMA, 
SCHEMA2).getRowKind()).isEqualTo(RowKind.DELETE);
   }
 
+  @Test
+  void testVariantIdentity() {

Review Comment:
   Good catch. With identical schemas the converter is never built, so the test 
did not exercise the variant branch. I replaced it with a 
`DATA_CONVERSION_NEEDED` case: `id` goes from int to long while a variant 
column passes through unchanged.



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