[FLINK-39015][table] Fix key extractor for multi join. Change GenericRowData to BinaryRowData#27508
[FLINK-39015][table] Fix key extractor for multi join. Change GenericRowData to BinaryRowData#27508ldadima wants to merge 1 commit intoapache:masterfrom
Conversation
…RowData to BinaryRowData
|
Hey @snuyanzin Could you review, please? |
gustavodemorais
left a comment
There was a problem hiding this comment.
Looks great! Thanks for picking this up @ldadima.
Could you just move the test as I mentioned in the comment? Thanks
| } | ||
|
|
||
| @TestTemplate | ||
| def testInnerMultiJoinWithEqualPk(): Unit = { |
There was a problem hiding this comment.
I think it either makes sense to
- Have the whole suite running with this the MULTI_JOIN flag as a param
- Add this to MultiJoinSemanticTests
Since 1. is high LOE, for this PR, I'd just go with 2. for now.
There was a problem hiding this comment.
Thanks for your review @gustavodemorais
1 Checked. All passed

2 I don't understand why we need to add SemanticTest. The problem only occurs after restoring from a checkpoint and only for HeapStateBackend. Also in SemanticTest there is MULTI_JOIN_TWO_WAY_INNER_JOIN_WITH_WHERE_IN, which checks similar sql (uses the same MultiJoinStateViews.JoinKeyContainsUniqueKey). I suggest to modify MultiJoinITCase (here) for failingDataSource and add this case
There was a problem hiding this comment.
Hey @ldadima, if it only happens after restoring from a checkpoint, then it'd make sense to move it to `MultiJoinRestoreTest?
What is the purpose of the change
To fix FLINK-39015
Verifying this change
Run JoinITCase#testInnerMultiJoinWithEqualPk
Does this pull request potentially affect one of the following parts:
@Public(Evolving): ( no)Documentation