Skip to content

Commit a287caf

Browse files
committed
Allow pushing down multiple times
Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent c62d738 commit a287caf

5 files changed

Lines changed: 6 additions & 32 deletions

File tree

opensearch/src/main/java/org/opensearch/sql/opensearch/planner/physical/EnumerableIndexScanRule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ public RelNode convert(RelNode rel) {
4343
final CalciteLogicalIndexScan scan = (CalciteLogicalIndexScan) rel;
4444
return new CalciteEnumerableIndexScan(
4545
scan.getCluster(),
46-
rel.getTraitSet().getCollation(),
46+
// Retains RelDistribution and RelCollation but replaces Convention
47+
scan.getTraitSet().plus(EnumerableConvention.INSTANCE),
4748
scan.getHints(),
4849
scan.getTable(),
4950
scan.getOsIndex(),

opensearch/src/main/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchIndexScanRule.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,4 @@ static boolean test(CalciteLogicalIndexScan scan) {
1818
static boolean sortByFieldsOnly(LogicalSort sort) {
1919
return !sort.getCollation().getFieldCollations().isEmpty() && sort.fetch == null;
2020
}
21-
22-
static boolean isSortPushed(CalciteLogicalIndexScan scan) {
23-
return scan.getPushDownContext().isSortPushed();
24-
}
2521
}

opensearch/src/main/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchSortIndexScanRule.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
package org.opensearch.sql.opensearch.planner.physical;
77

8-
import java.util.function.Predicate;
98
import org.apache.calcite.plan.RelOptRuleCall;
109
import org.apache.calcite.plan.RelRule;
1110
import org.apache.calcite.rel.logical.LogicalSort;
@@ -44,9 +43,7 @@ public interface Config extends RelRule.Config {
4443
.oneInput(
4544
b1 ->
4645
b1.operand(CalciteLogicalIndexScan.class)
47-
.predicate(
48-
Predicate.not(OpenSearchIndexScanRule::isSortPushed)
49-
.and(OpenSearchIndexScanRule::test))
46+
.predicate(OpenSearchIndexScanRule::test)
5047
.noInputs()));
5148

5249
@Override

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteEnumerableIndexScan.java

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package org.opensearch.sql.opensearch.storage.scan;
77

88
import java.util.List;
9-
import org.apache.calcite.adapter.enumerable.EnumerableConvention;
109
import org.apache.calcite.adapter.enumerable.EnumerableRel;
1110
import org.apache.calcite.adapter.enumerable.EnumerableRelImplementor;
1211
import org.apache.calcite.adapter.enumerable.PhysType;
@@ -21,7 +20,7 @@
2120
import org.apache.calcite.plan.RelOptPlanner;
2221
import org.apache.calcite.plan.RelOptRule;
2322
import org.apache.calcite.plan.RelOptTable;
24-
import org.apache.calcite.plan.RelTrait;
23+
import org.apache.calcite.plan.RelTraitSet;
2524
import org.apache.calcite.rel.hint.RelHint;
2625
import org.apache.calcite.rel.rules.CoreRules;
2726
import org.apache.calcite.rel.type.RelDataType;
@@ -45,21 +44,13 @@ public class CalciteEnumerableIndexScan extends CalciteIndexScan implements Enum
4544
*/
4645
public CalciteEnumerableIndexScan(
4746
RelOptCluster cluster,
48-
RelTrait relTrait,
47+
RelTraitSet traitSet,
4948
List<RelHint> hints,
5049
RelOptTable table,
5150
OpenSearchIndex osIndex,
5251
RelDataType schema,
5352
PushDownContext pushDownContext) {
54-
55-
super(
56-
cluster,
57-
cluster.traitSetOf(EnumerableConvention.INSTANCE, relTrait),
58-
hints,
59-
table,
60-
osIndex,
61-
schema,
62-
pushDownContext);
53+
super(cluster, traitSet, hints, table, osIndex, schema, pushDownContext);
6354
}
6455

6556
@Override

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteIndexScan.java

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ public RelWriter explainTerms(RelWriter pw) {
6565
// TODO: should we consider equivalent among PushDownContexts with different push down sequence?
6666
public static class PushDownContext extends ArrayDeque<PushDownAction> {
6767
private boolean isAggregatePushed = false;
68-
private boolean isSortPushed = false;
6968

7069
@Override
7170
public PushDownContext clone() {
@@ -76,13 +75,9 @@ public PushDownContext clone() {
7675
public boolean add(PushDownAction pushDownAction) {
7776
// Defense check. It should never do push down to this context after aggregate push-down.
7877
assert !isAggregatePushed : "Aggregate has already been pushed!";
79-
assert !isSortPushed : "Sort has already been pushed!";
8078
if (pushDownAction.type == PushDownType.AGGREGATION) {
8179
isAggregatePushed = true;
8280
}
83-
if (pushDownAction.type == PushDownType.SORT) {
84-
isSortPushed = true;
85-
}
8681
return super.add(pushDownAction);
8782
}
8883

@@ -91,12 +86,6 @@ public boolean isAggregatePushed() {
9186
isAggregatePushed = !isEmpty() && super.peekLast().type == PushDownType.AGGREGATION;
9287
return isAggregatePushed;
9388
}
94-
95-
public boolean isSortPushed() {
96-
if (isSortPushed) return true;
97-
isSortPushed = !isEmpty() && super.peekLast().type == PushDownType.SORT;
98-
return isSortPushed;
99-
}
10089
}
10190

10291
protected enum PushDownType {

0 commit comments

Comments
 (0)