MartijnVisser opened a new pull request, #29200:
URL: https://github.com/apache/flink/pull/29200
## What is the purpose of the change
A lookup join put only the temporal table's `ObjectIdentifier` in its
digest, while a `TableSourceScan` digests `RelOptTable#getQualifiedName`, which
`TableSourceTable` extends with the digest of every pushed-down
`SourceAbilitySpec`. Once a dim-side filter is pushed into the source there is
nothing left to tell two lookup joins apart, so the planner merges them. A
`UNION ALL` over the same table with two different filters then emits one
branch's rows under the other branch's filter, or nothing at all when the
non-matching branch comes first.
This appends the ability spec digests to the `table` item. It does not close
the gap with the scan entirely: the scan also digests its table hints, so two
lookup joins differing only in an `OPTIONS` hint still merge. That is a
separate discriminator and I'll file a follow-up.
Diagnosed by @morazow in #26514, which the stale bot closed.
## Brief change log
- `CommonPhysicalLookupJoin.explainTerms` appends
`TableSourceTable#getSpecDigests` to the `table` item, which covers stream and
batch through the shared base class
- Paired plan tests: two different filters must not merge, the same filter
twice must still be reused
- `LookupJoinSemanticTests` covers both branch orderings, since the wrong
order silently returns nothing
- Recorded plans and the two `determinism.md` pages updated for the richer
`table` item
## Verifying this change
This change added tests and can be verified as follows:
- `LookupJoinTest.testJoinFilterableTemporalTableWithUnion` expects two
`LookupJoin` nodes; before the fix the plan collapses to one plus
`Reused(reference_id=[1])`
- `LookupJoinTest.testJoinFilterableTemporalTableWithUnionSameFilter`
expects that reuse to survive, so a fix that merely made every digest unique
would fail
- `LookupJoinSemanticTests` returns 8 rows instead of 4 before the fix
with the matching filter first, and an empty result with it second
- `./mvnw -pl flink-table/flink-table-planner test` passes apart from
FLINK-40568, an open flake unrelated to lookup joins
Two notes for reviewers. The `table` item now lists the pushed-down
abilities exactly as `TableSourceScan` already does, which is why plans
unrelated to this bug change: projection push-down through a snapshot is
routine, and a `FilterPushDownSpec` is attached whenever a dim-side predicate
is convertible even when the source accepts none of it, rendering as
`filter=[]`. And a plan compiled before this fix has the merge baked in, so
affected users have to recompile.
## 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): no
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no, but an affected
query gains a node, so such a job cannot restore in place
- 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)
--
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]
