You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "jnh5y (via GitHub)" <gi...@apache.org> on 2024/11/26 20:24:08 UTC
[PR] [FLINK-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
jnh5y opened a new pull request, #25698:
URL: https://github.com/apache/flink/pull/25698
## What is the purpose of the change
* Adds Compiled Plan annotations to BatchExecLookupJoin.
* Tests the new annotations with the existing TestPrograms.
## Verifying this change
This change adds a BatchRestoreTest to cover the new annotations and show that the batch compiled plan can be restored and executed correctly.
## 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: (yes)
- 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? (no)
- If yes, how is the feature documented? (not applicable)
--
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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #25698:
URL: https://github.com/apache/flink/pull/25698#discussion_r1860654915
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtil.java:
##########
@@ -183,6 +184,7 @@ private ExecNodeMetadataUtil() {
add(BatchExecSortAggregate.class);
add(BatchExecSortLimit.class);
add(BatchExecWindowTableFunction.class);
+ add(BatchExecLookupJoin.class);
Review Comment:
Separately, I added this one above the last line intentionally so that there would not be any merge conflicts between the PR and another open at the same time!
--
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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #25698:
URL: https://github.com/apache/flink/pull/25698#discussion_r1860653880
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtil.java:
##########
@@ -183,6 +184,7 @@ private ExecNodeMetadataUtil() {
add(BatchExecSortAggregate.class);
add(BatchExecSortLimit.class);
add(BatchExecWindowTableFunction.class);
+ add(BatchExecLookupJoin.class);
Review Comment:
I'd like to sort the list at the end when I am merging the last few PRs for the present group of tickets. Would that be ok?
--
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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #25698:
URL: https://github.com/apache/flink/pull/25698#discussion_r1860563626
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtil.java:
##########
@@ -183,6 +184,7 @@ private ExecNodeMetadataUtil() {
add(BatchExecSortAggregate.class);
add(BatchExecSortLimit.class);
add(BatchExecWindowTableFunction.class);
+ add(BatchExecLookupJoin.class);
Review Comment:
nit: seems for stream it's ordered(almost ordered) and for batch it is not...
should we have these lists ordered?
--
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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin merged PR #25698:
URL: https://github.com/apache/flink/pull/25698
--
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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #25698:
URL: https://github.com/apache/flink/pull/25698#discussion_r1860563626
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtil.java:
##########
@@ -183,6 +184,7 @@ private ExecNodeMetadataUtil() {
add(BatchExecSortAggregate.class);
add(BatchExecSortLimit.class);
add(BatchExecWindowTableFunction.class);
+ add(BatchExecLookupJoin.class);
Review Comment:
nit: seems for stream it's ordered and for batch it is not...
--
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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #25698:
URL: https://github.com/apache/flink/pull/25698#issuecomment-2503769898
Thanks for the contribution
it looks good to me
I have one minor comment
--
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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #25698:
URL: https://github.com/apache/flink/pull/25698#issuecomment-2501851519
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "99ad2b80279a4e1e4a1cb4b6f9f2a8f006d9c640",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "99ad2b80279a4e1e4a1cb4b6f9f2a8f006d9c640",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 99ad2b80279a4e1e4a1cb4b6f9f2a8f006d9c640 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-36718] Add CompiledPlan annotations to BatchExecLookupJoin [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #25698:
URL: https://github.com/apache/flink/pull/25698#discussion_r1860690755
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtil.java:
##########
@@ -183,6 +184,7 @@ private ExecNodeMetadataUtil() {
add(BatchExecSortAggregate.class);
add(BatchExecSortLimit.class);
add(BatchExecWindowTableFunction.class);
+ add(BatchExecLookupJoin.class);
Review Comment:
makes sense, thanks for clarification
--
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