You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "xuyangzhong (via GitHub)" <gi...@apache.org> on 2024/09/12 11:51:20 UTC
[PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
xuyangzhong opened a new pull request, #25320:
URL: https://github.com/apache/flink/pull/25320
## What is the purpose of the change
*Introduce new join operator with async state api.*
## Brief change log
- *Introduce new join operator with async state api*
- *ITCases and harness tests are added to verity this pr*
## Verifying this change
ITCases and harness tests are added to verity this pr.
## 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
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? Yes
- If yes, how is the feature documented? A Independent doc pr about this feature will be added later.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil commented on code in PR #25320:
URL: https://github.com/apache/flink/pull/25320#discussion_r1772492551
##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/utils/StreamingWithStateTestBase.scala:
##########
@@ -239,6 +239,7 @@ object StreamingWithStateTestBase {
override def toString: String = backend.toString
}
+ // TODO test FORSTDB_BACKEND after 2.0-preview
Review Comment:
I created FLINK-36354
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #25320:
URL: https://github.com/apache/flink/pull/25320#issuecomment-2346097191
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "378f28c125f4192a35e614d09a51489309c6156c",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "378f28c125f4192a35e614d09a51489309c6156c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 378f28c125f4192a35e614d09a51489309c6156c UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "xishuaidelin (via GitHub)" <gi...@apache.org>.
xishuaidelin commented on code in PR #25320:
URL: https://github.com/apache/flink/pull/25320#discussion_r1770576947
##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/stream/sql/JoinITCase.scala:
##########
@@ -99,6 +106,33 @@ class JoinITCase(miniBatch: MiniBatchMode, state: StateBackendMode)
// Tests for inner join.
override def after(): Unit = {}
+ @TestTemplate
+ def test(): Unit = {
+ val tableA = failingDataSource(
+ Seq[(Int, Long, String)]((1, 1L, "Hi"), (2, 2L, "Hello"), (3, 3L, "Hello world")))
+ .toTable(tEnv, 'a1, 'a2, 'a3)
+ val tableB = failingDataSource(
+ Seq[(Int, Long, String)]((1, 1L, "Hi"), (2, 2L, "Hello"), (3, 3L, "Hello world")))
+ .toTable(tEnv, 'b1, 'b2, 'b3)
+ tEnv.createTemporaryView("C", tableA)
+ tEnv.createTemporaryView("D", tableB)
+
+ val sqlQuery = "SELECT * FROM C, D WHERE a1 = b1 and a2 = 1"
+
+ val sink = new TestingRetractSink
+ tEnv.sqlQuery(sqlQuery).toRetractStream[Row].addSink(sink).setParallelism(1)
+ env.execute()
+
+ val expected = mutable.Seq(
Review Comment:
Could you please clarify the rationale behind the expected sequence? IMO, Is it intended to be List(1, 1, 'Hi', 1, 1, 'Hi')?
##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/stream/sql/JoinITCase.scala:
##########
@@ -99,6 +106,33 @@ class JoinITCase(miniBatch: MiniBatchMode, state: StateBackendMode)
// Tests for inner join.
override def after(): Unit = {}
+ @TestTemplate
+ def test(): Unit = {
Review Comment:
Coud it be renamed to more effectively reflect the purpose of the test?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil commented on code in PR #25320:
URL: https://github.com/apache/flink/pull/25320#discussion_r1772487203
##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/utils/StreamingWithStateTestBase.scala:
##########
@@ -239,6 +239,7 @@ object StreamingWithStateTestBase {
override def toString: String = backend.toString
}
+ // TODO test FORSTDB_BACKEND after 2.0-preview
Review Comment:
We can create a jira to track this and avoid using the TODO annotation because there left too many staled TODOs
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "xishuaidelin (via GitHub)" <gi...@apache.org>.
xishuaidelin commented on PR #25320:
URL: https://github.com/apache/flink/pull/25320#issuecomment-2369952265
LGTM
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "xuyangzhong (via GitHub)" <gi...@apache.org>.
xuyangzhong commented on PR #25320:
URL: https://github.com/apache/flink/pull/25320#issuecomment-2346081877
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.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "xuyangzhong (via GitHub)" <gi...@apache.org>.
xuyangzhong commented on code in PR #25320:
URL: https://github.com/apache/flink/pull/25320#discussion_r1770706342
##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/stream/sql/JoinITCase.scala:
##########
@@ -99,6 +106,33 @@ class JoinITCase(miniBatch: MiniBatchMode, state: StateBackendMode)
// Tests for inner join.
override def after(): Unit = {}
+ @TestTemplate
+ def test(): Unit = {
Review Comment:
Hi, @xishuaidelin this is a piece of useless code that is used to debug, and I have deleted them in new commit.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil commented on code in PR #25320:
URL: https://github.com/apache/flink/pull/25320#discussion_r1772492551
##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/utils/StreamingWithStateTestBase.scala:
##########
@@ -239,6 +239,7 @@ object StreamingWithStateTestBase {
override def toString: String = backend.toString
}
+ // TODO test FORSTDB_BACKEND after 2.0-preview
Review Comment:
I created FLINK-36157
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36248][table] Introduce new Join Operator with Async State API [flink]
Posted by "lincoln-lil (via GitHub)" <gi...@apache.org>.
lincoln-lil merged PR #25320:
URL: https://github.com/apache/flink/pull/25320
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org