[FLINK-36248][table] Introduce new Join Operator with Async State API #25320
Conversation
|
Note: some ITCases and harness tests are failed because there are some minor issues with the current ForStDB state backend. I'll try to rebase the latest master until all issues are fixed. |
xuyangzhong
force-pushed
the
FLINK-36248
branch
from
378f28c to
8cbd0bb
Compare
There was a problem hiding this comment.
Hi, @xuyangzhong. Your PR looks pretty good. I've just left some minor comments about test.
| @@ -99,6 +106,33 @@ class JoinITCase(miniBatch: MiniBatchMode, state: StateBackendMode) | |||
| // Tests for inner join. | |||
| override def after(): Unit = {} | |||
|
|
|||
| @TestTemplate | |||
| def test(): Unit = { | |||
There was a problem hiding this comment.
Coud it be renamed to more effectively reflect the purpose of the test?
There was a problem hiding this comment.
Hi, @xishuaidelin this is a piece of useless code that is used to debug, and I have deleted them in new commit.
...le-planner/src/test/scala/org/apache/flink/table/planner/runtime/stream/sql/JoinITCase.scala
Outdated
Show resolved
Hide resolved
xuyangzhong
force-pushed
the
FLINK-36248
branch
from
567fc8d to
6874b0d
Compare
|
LGTM |
There was a problem hiding this comment.
@xuyangzhong Thank you for working on this! Looks pretty good for the incoming 2.0 preview version. Only one minor comment before merging.
| @@ -239,6 +239,7 @@ object StreamingWithStateTestBase { | |||
| override def toString: String = backend.toString | |||
| } | |||
|
|
|||
| // TODO test FORSTDB_BACKEND after 2.0-preview | |||
There was a problem hiding this comment.
We can create a jira to track this and avoid using the TODO annotation because there left too many staled TODOs
There was a problem hiding this comment.
I created FLINK-36354
xuyangzhong
force-pushed
the
FLINK-36248
branch
from
6874b0d to
443bbe0
Compare
What is the purpose of the change
Introduce new join operator with async state api.
Brief change log
Verifying this change
ITCases and harness tests are added to verity this pr.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation