fapaul opened a new pull request, #27730:
URL: https://github.com/apache/flink/pull/27730
## What is the purpose of the change
`CharLengthConstraint.enforce()` and `BinaryLengthConstraint.enforce()`
crash with a `NullPointerException` when a nullable
`CHAR`/`VARCHAR`/`BINARY`/`VARBINARY` column
contains a null value. The root cause is that both methods access the
field value (e.g. `rowData.getString(idx).numChars()` /
`rowData.getBinary(idx).length`) without
first checking `rowData.isNullAt(idx)`. The `NotNullConstraint` only
guards `NOT NULL` columns, so nullable columns with length constraints are
unprotected.
## Brief change log
- Added a `rowData.isNullAt(fieldIdx)` guard in
`CharLengthConstraint.enforce()` to skip null values before calling
`getString()`
- Added a `rowData.isNullAt(fieldIdx)` guard in
`BinaryLengthConstraint.enforce()` to skip null values before calling
`getBinary()`
- Added two semantic test programs
(`CHAR_LENGTH_TRIM_PAD_WITH_NULLABLE_COLUMNS`,
`BINARY_LENGTH_TRIM_PAD_WITH_NULLABLE_COLUMNS`) in
`ConstraintEnforcerTestPrograms`
to verify null values in nullable columns pass through correctly with
`TRIM_PAD` enforcement
## Verifying this change
This change added tests and can be verified as follows:
- Added `CHAR_LENGTH_TRIM_PAD_WITH_NULLABLE_COLUMNS` semantic test
program that feeds rows with null values into nullable
`CHAR(8)`/`CHAR(6)`/`VARCHAR(6)` columns
with `TRIM_PAD` enforcement enabled, verifying that null fields pass
through unchanged while non-null fields are correctly trimmed/padded
- Added `BINARY_LENGTH_TRIM_PAD_WITH_NULLABLE_COLUMNS` semantic test
program that feeds rows with null values into nullable
`BINARY(8)`/`BINARY(6)`/`VARBINARY(6)`
columns with `TRIM_PAD` enforcement enabled, verifying the same behavior
for binary types
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): **no**
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: **no**
- The serializers: **no**
- The runtime per-record code paths (performance sensitive): **yes**
(adds a null check in the sink constraint enforcement hot path, but it is a
single `isNullAt`
call which is negligible)
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: **no**
- The S3 file system connector: **no**
## Documentation
- Does this pull request introduce a new feature? **no**
- If yes, how is the feature documented? **not applicable**
--
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]
