swuferhong commented on code in PR #23192:
URL: https://github.com/apache/flink/pull/23192#discussion_r1290855481
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/reuse/ScanReuserUtils.java:
##########
@@ -329,6 +329,12 @@ public static String
getDigest(CommonPhysicalTableSourceScan scan, boolean witho
TableSourceTable table = scan.tableSourceTable();
List<String> digest = new ArrayList<>();
digest.addAll(table.getNames());
+
+ // input should be the first item
+ if (!scan.getInputs().isEmpty()) {
+ digest.add("input=[" + scan.getInputs() + "]");
Review Comment:
The length of `scan.getInputs()` will be very long? I think you can
simplify the output of the digest here
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/reuse/ReplaceScanWithCalcShuttle.java:
##########
@@ -57,4 +61,12 @@ public RelNode visit(RelNode rel) {
return super.visit(rel);
}
+
+ private void visitDppSource(RelNode scan) {
+ // If scan is BatchPhysicalDynamicFilteringTableSourceScan,the input
should be recursive
+ // first
Review Comment:
I think there needn't to strictly limit scan is dpp scan in the comments.
But let's talk about sources with input, such as dpp sources
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/reuse/ReusableScanVisitor.java:
##########
@@ -42,6 +42,10 @@ public void visit(RelNode node, int ordinal, RelNode parent)
{
CommonPhysicalTableSourceScan scan =
(CommonPhysicalTableSourceScan) node;
String digest = getDigest(scan, true);
digestToReusableScans.computeIfAbsent(digest, k -> new
ArrayList<>()).add(scan);
+ // BatchPhysicalDynamicFilteringTableSourceScan has one input, so
need to consider it
Review Comment:
ditto
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/optimize/program/DynamicPartitionPruningProgramTest.java:
##########
@@ -658,4 +658,47 @@ public void testDPPWithJoinKeysNotIncludePartitionKeys() {
+ " and dim.price < 500 and dim.price > 300";
util.verifyRelPlan(query);
}
+
+ @Test
+ public void testDppFactSideCannotReuseWithSameCommonSource() {
+ String query =
+ "SELECT * FROM(\n"
+ + " Select fact_part.id, fact_part.price,
fact_part.amount from fact_part join (Select * from dim) t1"
+ + " on fact_part.fact_date_sk = dim_date_sk where
t1.price < 500\n"
+ + " UNION ALL Select fact_part.id, fact_part.price,
fact_part.amount from fact_part)";
+ util.verifyExecPlan(query);
+ }
+
+ @Test
+ public void testDimSideReuseWithUnionAllTwoFactSide() {
+ util.tableEnv()
+ .executeSql(
Review Comment:
Can this test be simplified? I feel that the correlation between testing and
this pr is not strong.
--
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]
