rionmonster opened a new pull request, #29558: URL: https://github.com/apache/flink/pull/29558
> **Stacked on #29532 (FLINK-41034).** Please review only the last commit, `[FLINK-41067][table] ...`. The first two commits come from #29532: > - `JoinConditionWithNullFilters#currentKeyCannotMatch`, which the window join reuses. > - The `HandwrittenKeySelector` hotfix, which the new tests need for a NULL in key field 1. > > I'll rebase onto `master` and mark this ready once #29532 is merged. ## What is the purpose of the change The window join and the interval join stored rows whose join key has a NULL in a field that isn't null-safe. Such rows can never match. They all share one key, so they piled up in a single subtask until the window fired or the interval expired. This is the follow-up to FLINK-41034 for these two operators. ## Brief change log - `WindowJoinHelper` (used by `WindowJoinOperator` and `AsyncStateWindowJoinOperator`) doesn't store such a row on an input whose unmatched rows are never emitted. It uses `JoinConditionWithNullFilters#currentKeyCannotMatch` from FLINK-41034. - `TimeIntervalJoin` doesn't cache such a row on a non-preserved side. The NULL check in `IntervalJoinFunction#join` moves into `currentKeyCannotMatch()` so the operator can reuse it. - The preserved side of an outer join, and the left side of an anti window join, still store these rows. Today they are emitted when the window fires or the interval expires, and the docs describe that timing, so it doesn't change. ## Verifying this change This change added tests: - `WindowJoinOperatorTest#testDoesNotStoreRecordsThatCanNeverMatch` checks state size and output for every join type, with and without async state. - `RowTimeIntervalJoinTest#testDoesNotCacheRowsThatCanNeverMatch` and `ProcTimeIntervalJoinTest#testDoesNotCacheRowsThatCanNeverMatch` check the same for INNER, LEFT, RIGHT and FULL. - `WindowJoinOperatorTest#testStoresRecordsWithNullSafeKeys` and `RowTimeIntervalJoinTest#testCachesRowsWithNullSafeKeys` check that a NULL key compared null-safely (as with `IS NOT DISTINCT FROM`) is still stored and joined. They guard against dropping rows that can match, which would lose output. The first three tests fail without the change. The null-safe tests fail if null-safe keys are treated as never matching. The existing window and interval join tests pass: the runtime join tests, `WindowJoinITCase`, `IntervalJoinITCase` and the restore tests. No new restore test is needed, because the state layout doesn't change. NULL-key rows stored by an earlier version are still emitted or cleaned up when their window fires or their interval expires. ## 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. It adds one null check per stored row, and in return it skips the state write for rows that can never match. - 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 Code (Claude Opus 5.5) -- 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]
