godfreyhe commented on code in PR #20596:
URL: https://github.com/apache/flink/pull/20596#discussion_r948921617
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/rules/physical/batch/DynamicPartitionPruningRuleTest.java:
##########
@@ -410,21 +419,35 @@ public void testComplexDimSideWithAggInDimSide() {
// --------------------------dpp factor test
---------------------------------------------
@Test
- public void testDPPFactorToReorderTable() {
+ public void testDPPFactorToReorderTableWithoutStats() {
// While there are several joins, and fact table not adjacent to dim
table directly. dynamic
// partition pruning factor will try best to reorder join relations to
make fact table
// adjacent to dim table.
- String ddl1 =
- "CREATE TABLE test_database.sales (\n"
+ String ddl2 =
Review Comment:
nit: ddl2 -> ddl ?
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/rules/physical/batch/DynamicPartitionPruningRuleTest.java:
##########
@@ -479,10 +543,20 @@ public void testDPPFactorWithDimSideJoinKeyChanged() {
String query =
"Select * from fact_part join item on fact_part.id = item.id"
- + " join sales on fact_part.id = sales.id"
+ " join (select dim_date_sk + 1 as dim_date_sk, price
from dim) dim1"
+ " on fact_part.fact_date_sk = dim1.dim_date_sk"
+ " where dim1.price < 500 and dim1.price > 300";
util.verifyRelPlan(query);
}
+
Review Comment:
please add a test which join keys are not partition key but are directly
reference the face source fields
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/utils/DynamicPartitionPruningUtils.java:
##########
@@ -171,7 +171,20 @@ private static void visitFactSide(
joinKeys.stream()
.map(i -> scan.getRowType().getFieldNames().get(i))
.collect(Collectors.toList());
- factSideFactors.isSuitableFactScanSource =
!candidateFields.isEmpty();
+ if (candidateFields.isEmpty()) {
+ factSideFactors.isSuitableFactScanSource = false;
+ return;
+ }
+
+ List<String> suitableFields = new ArrayList<>();
+
+ for (String candidateField : candidateFields) {
Review Comment:
please add some comments to explain why we need this change
--
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]
