xuyangzhong commented on code in PR #23828:
URL: https://github.com/apache/flink/pull/23828#discussion_r1410290690
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/physical/batch/PushLocalSortAggWithCalcIntoScanRule.java:
##########
@@ -74,9 +74,8 @@ public boolean matches(RelOptRuleCall call) {
BatchPhysicalCalc calc = call.rel(2);
BatchPhysicalTableSourceScan tableSourceScan = call.rel(3);
- return isInputRefOnly(calc)
- && isProjectionNotPushedDown(tableSourceScan)
- && canPushDown(call, localAggregate, tableSourceScan);
+ return isProjectionNotPushedDown(tableSourceScan)
Review Comment:
ditto
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/physical/batch/PushLocalSortAggWithSortAndCalcIntoScanRule.java:
##########
@@ -85,7 +85,7 @@ public boolean matches(RelOptRuleCall call) {
return isInputRefOnly(calc)
Review Comment:
Should these codes also be moved into function `canPushDown`?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/physical/batch/PushLocalSortAggWithCalcIntoScanRule.java:
##########
@@ -85,7 +84,11 @@ public void onMatch(RelOptRuleCall call) {
BatchPhysicalCalc calc = call.rel(2);
BatchPhysicalTableSourceScan oldScan = call.rel(3);
- int[] calcRefFields = getRefFiledIndex(calc);
+ // For count(*) and count(n) we need to ignore the calc.
+ int[] calcRefFields = null;
Review Comment:
ditto
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/physical/batch/PushLocalHashAggWithCalcIntoScanRule.java:
##########
@@ -85,7 +82,11 @@ public void onMatch(RelOptRuleCall call) {
BatchPhysicalCalc calc = call.rel(2);
BatchPhysicalTableSourceScan oldScan = call.rel(3);
- int[] calcRefFields = getRefFiledIndex(calc);
+ // For count(*) and count(n) we need to ignore the calc.
+ int[] calcRefFields = null;
Review Comment:
Can these codes also be moved into `pushLocalAggregateIntoScan` because of
duplication?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/rules/physical/batch/PushLocalSortAggWithSortAndCalcIntoScanRule.java:
##########
@@ -94,7 +94,10 @@ public void onMatch(RelOptRuleCall call) {
BatchPhysicalCalc calc = call.rel(3);
BatchPhysicalTableSourceScan oldScan = call.rel(4);
- int[] calcRefFields = getRefFiledIndex(calc);
+ int[] calcRefFields = null;
+ if (isInputRefOnly(calc) && isProjectionNotPushedDown(oldScan)) {
Review Comment:
ditto
--
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]
