kumarpritam863 commented on code in PR #18739:
URL: https://github.com/apache/iceberg/pull/18739#discussion_r4520079870


##########
flink/v2.3/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DataConverter.java:
##########
@@ -161,8 +175,14 @@ class RowDataConverter implements DataConverter {
           RowType.RowField sourceField = 
sourceType.getFields().get(sourceFieldIndex);
           fieldGetter = RowData.createFieldGetter(sourceField.getType(), 
sourceFieldIndex);
           dataConverter =
-              DataConverter.getNullable(
-                  sourceField.getType(), targetField.getType(), caseSensitive);
+              targetField.getType().isNullable()
+                  ? DataConverter.getNullable(
+                      sourceField.getType(), targetField.getType(), 
caseSensitive)
+                  : DataConverter.getNonNullable(
+                      sourceField.getType(),
+                      targetField.getType(),
+                      caseSensitive,
+                      targetField.getName());

Review Comment:
   Good catch, thanks. identity() never ran a converter, so a record with the 
table's schema skipped the null check entirely and failed in the writer (or, 
with ORC, was written with a null in a required column). 
DataConverter.identity(Schema) now checks the required columns for those 
records too, using column indices computed once per schema. It does not depend 
on the error stream, so with ORC this now fails the job like Parquet and Avro 
already do. I have added tests for these too.



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