cshuo commented on a change in pull request #16979:
URL: https://github.com/apache/flink/pull/16979#discussion_r696579500
##########
File path:
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/optimize/StreamCommonSubGraphBasedOptimizer.scala
##########
@@ -85,12 +83,13 @@ class StreamCommonSubGraphBasedOptimizer(planner:
StreamPlanner)
return sinkBlocks
}
- // infer updateAsRetraction property and miniBatchInterval property for
all input blocks
+ // infer modifyKind property for each blocks independently
+ sinkBlocks.foreach(b => optimizeBlock(b, isSinkBlock = true))
+ // infer and propagate updateKind and miniBatchInterval property for each
blocks
sinkBlocks.foreach { b =>
- inferTraits(b, b.isUpdateBeforeRequired, b.getMiniBatchInterval,
isSinkBlock = true)
+ propagateUpdateKindAndMiniBatchInterval(
+ b, b.isUpdateBeforeRequired, b.getMiniBatchInterval, isSinkBlock =
true)
}
- // propagate updateAsRetraction property and miniBatchInterval property to
all input blocks
- sinkBlocks.foreach(propagateTraits(_, isSinkBlock = true))
// clear the intermediate result
sinkBlocks.foreach(resetIntermediateResult)
// optimize recursively RelNodeBlock
Review comment:
For solution here, it cannot be remove. You're right that the final
`optimizeBlock ` is updating the intermediate results, but the updated
intermediate result is physical node plan optimized with correct `ModifyKind`
and `UpdateKind` trait, which are inferred and propagated by Line 86 ~ 91.
The plan is optimized 3 times currently. Actually, we can move changeLog and
minibatch interval inference out of optimizing phase, in that way the plan only
needs one optimizing. This refactor involves many changes, so I think we may
put it in later version.
--
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]
