gustavodemorais commented on code in PR #29067:
URL: https://github.com/apache/flink/pull/29067#discussion_r3923080785
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/ConstraintEnforcerTestPrograms.java:
##########
@@ -937,6 +938,205 @@ public class ConstraintEnforcerTestPrograms {
.runSql("INSERT INTO sink_t SELECT * FROM source_t")
.build();
+ //
------------------------------------------------------------------------------------------
+ // Delete-by-key: a by-key delete legitimately carries null in its non-key
columns ("regardless
+ // of nullability constraints"). The constraint enforcer only checks
not-null key columns for
+ // by-key delete messages and ignores all value columns.
Review Comment:
Currently, it's a bit of a mix in the value columns/there are no guarantees.
Suggestionm
```suggestion
// Delete-by-key: a by-key delete might carry null in its non-key
columns, regardless
// of nullability constraints. The constraint enforcer should only check
key columns for
// by-key delete messages and ignores all value columns.
```
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/operators/sink/constraint/ConstraintEnforcerExecutor.java:
##########
@@ -94,11 +122,20 @@ private static Constraint[] createConstraints(
.mapToObj(idx -> fieldNames[idx])
.toArray(String[]::new);
+ final Set<Integer> primaryKeySet =
Review Comment:
Rest of the class uses BitSet for the same pattern (see buildCouldPad).
Suggestion: swap it instead of Set, cheaper and consistent with the file
--
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]
