ykisana opened a new pull request, #29424:
URL: https://github.com/apache/flink/pull/29424
## What is the purpose of the change
`UnsliceAssigners.WindowedUnsliceAssigner` reported a NULL
`window_start`/`window_end` as `"RowTime field should not be null."`. That is
the same message `WatermarkAssignerOperator` and
`SliceAssigners.AbstractSliceAssigner` use for a NULL user rowtime, which the
user can fix in their data. The windowed assigners only see rows produced by an
upstream window operator, where these columns are `TIMESTAMP(3) NOT NULL`, so a
NULL there is an internal bug. The shared message points users at their data
and makes the two cases indistinguishable.
The slicing counterparts, `SliceAssigners.WindowedSliceAssigner`
(`window_end`) and the sliced assigners used by two-phase window aggregation
(`slice_end`), had no null check at all. With a `BinaryRowData`, `getTimestamp`
on a NULL compact timestamp reads the zeroed slot, so the row is silently
assigned to a slice ending at epoch 0 instead of failing.
## Brief change log
- `WindowedUnsliceAssigner#createWindow` throws an `IllegalStateException`
saying the window start or end of a windowed input row is null and that this is
a bug.
- `WindowedSliceAssigner#assignSliceEnd` and
`AbstractSlicedSliceAssigner#assignSliceEnd` check for a NULL
`window_end`/`slice_end` and throw the same kind of exception.
- `WatermarkAssignerOperator` and `AbstractSliceAssigner` are unchanged;
their message is correct for a NULL user rowtime.
## Verifying this change
This change added tests and can be verified as follows:
- `WindowedSliceAssignerTest#testNullWindowEnd`: NULL `window_end` fails
with the new message.
- New `SlicedSliceAssignerTest`: slice assignment for the shared and
unshared sliced assigners, and NULL `slice_end` failure for both.
- New `WindowedUnsliceAssignerTest`: window assignment, plus NULL
`window_start` and NULL `window_end` failures.
- Added `nullRow()` and an `assignSliceEnd(SliceAssigner, RowData)`
overload to `SliceAssignerTestBase`.
- Verified red-green: without the fix the slicing tests fail because no
exception is thrown (the NULL is read as 0), and the unslicing tests fail on
the exception type.
## 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. One
null-bit check per record in `WindowedSliceAssigner` and the sliced assigners,
the same check `AbstractSliceAssigner` already does per record.
- 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
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Opus 5.5
Fix written by and all code manually reviewed by a human. Tests, PR
description and comments by Claude.
--
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]
