MartijnVisser commented on PR #26514: URL: https://github.com/apache/flink/pull/26514#issuecomment-5688369600
@morazow I picked this up since the bot closed it rather than anyone rejecting it. You're a co-author on the commit, the reproduction and the test case are yours. Happy to hand it back if you'd rather carry it yourself. New PR: https://github.com/apache/flink/pull/29200 @xuyangzhong on your two comments: > What about reusing this item `table` with [...] it can be aligned with the digest in `CommonPhysicalTableSourceScan` Went this way, but kept the dotted table name. The lookup join now appends `TableSourceTable#getSpecDigests` to the `table` item, so it covers every spec instead of filter and partition only, and matches what the scan digests. `getQualifiedName.mkString(", ")` would have changed all 138 lookup join plan lines in the goldens instead of 56. > Could you please move this test to `LookupJoinITCase`? It ended up as a `TableTestProgram` in `LookupJoinSemanticTests` instead, since the planner `AGENTS.md` prefers semantic tests over ITCase. Both branch orderings are covered, which matters because with the non-matching filter first the query silently returns nothing. One thing the new PR does not fix: the scan also digests its table hints, so two lookup joins differing only in an `OPTIONS` hint still get merged. Same class of bug, separate discriminator, I'll file it separately. -- 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]
