twalthr commented on code in PR #26436:
URL: https://github.com/apache/flink/pull/26436#discussion_r2036749316


##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/data/DataStructureConvertersTest.java:
##########
@@ -355,7 +355,62 @@ static List<TestSpec> testData() {
                 TestSpec.forDataType(
                                 DataTypes.STRUCTURED(GenericPojo.class, 
FIELD("value", DATE())))
                         .convertedTo(
-                                GenericPojo.class, new 
GenericPojo<>(LocalDate.ofEpochDay(123))));
+                                GenericPojo.class, new 
GenericPojo<>(LocalDate.ofEpochDay(123))),
+
+                // partial delete messages
+                TestSpec.forDataType(
+                                DataTypes.ROW(
+                                        DataTypes.FIELD("f0", 
DataTypes.TINYINT().notNull()),
+                                        DataTypes.FIELD("f1", 
DataTypes.SMALLINT().notNull()),
+                                        DataTypes.FIELD("f2", 
DataTypes.INT().notNull()),
+                                        DataTypes.FIELD("f3", 
DataTypes.BIGINT().notNull()),
+                                        DataTypes.FIELD("f4", 
DataTypes.DOUBLE().notNull()),
+                                        DataTypes.FIELD("f5", 
DataTypes.FLOAT().notNull()),
+                                        DataTypes.FIELD("f6", 
DataTypes.DATE().notNull()),
+                                        DataTypes.FIELD("f7", 
DataTypes.BINARY(12).notNull()),
+                                        DataTypes.FIELD("f8", 
DataTypes.VARBINARY(12).notNull()),
+                                        DataTypes.FIELD("f9", 
DataTypes.CHAR(12).notNull()),
+                                        DataTypes.FIELD("f10", 
DataTypes.VARCHAR(12).notNull()),
+                                        DataTypes.FIELD("f11", 
DataTypes.BOOLEAN().notNull()),
+                                        DataTypes.FIELD("f12", 
DataTypes.TIME().notNull()),
+                                        DataTypes.FIELD("f13", 
DataTypes.TIMESTAMP().notNull()),
+                                        DataTypes.FIELD("f14", 
DataTypes.TIMESTAMP_LTZ().notNull()),
+                                        DataTypes.FIELD("f15", 
DataTypes.DECIMAL(10, 2).notNull()),
+                                        DataTypes.FIELD(
+                                                "f16",
+                                                DataTypes.MAP(DataTypes.INT(), 
DataTypes.STRING())
+                                                        .notNull()),
+                                        DataTypes.FIELD(
+                                                "f17", 
DataTypes.ARRAY(DataTypes.INT()).notNull()),
+                                        DataTypes.FIELD(
+                                                "f18", 
DataTypes.ARRAY(DataTypes.INT()).notNull()),
+                                        DataTypes.FIELD(
+                                                "f19",
+                                                
DataTypes.MULTISET(DataTypes.INT()).notNull())))
+                        .convertedTo(
+                                Row.class,
+                                Row.ofKind(
+                                        RowKind.DELETE,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        null,
+                                        new Integer[] {null, null},

Review Comment:
   Shouldn't this be null instead?



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/data/RowData.java:
##########
@@ -287,9 +287,6 @@ static FieldGetter createFieldGetter(LogicalType fieldType, 
int fieldPos) {
             default:
                 throw new IllegalArgumentException();
         }
-        if (!fieldType.isNullable()) {

Review Comment:
   Can you add this to the JavaDoc of the Field getter interface?
   // Converters and serializers always support nullability. The NOT NULL 
constraint is only considered on SQL
   // semantic level but not data transfer. E.g. partial deletes (i.e. key-only 
upserts) set all non-key fields to
   // null, regardless of logical type.



##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/typeutils/RowDataSerializerTest.java:
##########
@@ -398,4 +408,49 @@ private static RowDataSerializer getRowSerializer() {
                     
InternalSerializers.<RowData>create(NESTED_DATA_TYPE.getLogicalType());
         }
     }
+
+    static final class RowDataSerializerWithNullForNotNullTypeTest extends 
RowDataSerializerTest {

Review Comment:
   Add comment from above also here to give readers more context.



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

Reply via email to