You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/06/05 12:46:47 UTC
[GitHub] [calcite] rubenada opened a new pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
rubenada opened a new pull request #2005:
URL: https://github.com/apache/calcite/pull/2005
Implement trait propagation for EnumerableCorrelate
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436186347
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,35 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
Review comment:
Not very familiar with `EnumerableCorrelate`, will it need to deal with RIGHT OUTER/FULL OUTER JOIN style case?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436249058
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,35 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
Review comment:
Correlate operator (base class) can only handle LEFT / INNER / SEMI / ANTI join. We do not need to consider RIGHT/ FULL in this case.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436338497
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,35 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ // EnumerableCorrelate traits passdown shall only pass through collation to left input.
+ // This is because for EnumerableCorrelate always uses left input as the outer loop,
+ // thus only left input can preserve ordering.
+
+ for (RelFieldCollation relFieldCollation : collation.getFieldCollations()) {
+ // If field collation belongs to right input: bail out.
+ if (relFieldCollation.getFieldIndex() >= getLeft().getRowType().getFieldCount()) {
+ return null;
+ }
+ }
+
+ final RelTraitSet passThroughTraitSet = traitSet.replace(collation);
+ return Pair.of(passThroughTraitSet,
+ ImmutableList.of(
+ passThroughTraitSet,
+ passThroughTraitSet.replace(RelCollations.EMPTY)));
+ }
+
+ @Override public DeriveMode getDeriveMode() {
+ return DeriveMode.LEFT_FIRST;
+ }
Review comment:
`deriveTraits` method added, not sure if the implementation is correct though, please take a look.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
hsyuan commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436379014
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,49 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ // EnumerableCorrelate traits passdown shall only pass through collation to left input.
+ // This is because for EnumerableCorrelate always uses left input as the outer loop,
+ // thus only left input can preserve ordering.
+
+ for (RelFieldCollation relFieldCollation : collation.getFieldCollations()) {
+ // If field collation belongs to right input: bail out.
+ if (relFieldCollation.getFieldIndex() >= getLeft().getRowType().getFieldCount()) {
+ return null;
+ }
+ }
+
+ final RelTraitSet passThroughTraitSet = traitSet.replace(collation);
+ return Pair.of(passThroughTraitSet,
+ ImmutableList.of(
+ passThroughTraitSet,
+ passThroughTraitSet.replace(RelCollations.EMPTY)));
+ }
+
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+ // should only derive traits (limited to collation for now) from left input.
+ assert childId == 0;
+
+ final RelCollation collation = childTraits.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ final RelTraitSet traits = traitSet.replace(collation);
+ return Pair.of(traits, ImmutableList.of(traits));
Review comment:
the list should contain right child traitset.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436186347
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,35 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
Review comment:
Not very familiar with `EnumerableCorrelate`, will it need to deal with RIGHT OUTER/FULL OUTER JOIN style case?
If so also should return null for those cases.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia commented on pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#issuecomment-639854014
Indeed it looks good.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436598152
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,49 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ // EnumerableCorrelate traits passdown shall only pass through collation to left input.
+ // This is because for EnumerableCorrelate always uses left input as the outer loop,
+ // thus only left input can preserve ordering.
+
+ for (RelFieldCollation relFieldCollation : collation.getFieldCollations()) {
+ // If field collation belongs to right input: bail out.
+ if (relFieldCollation.getFieldIndex() >= getLeft().getRowType().getFieldCount()) {
+ return null;
+ }
+ }
+
+ final RelTraitSet passThroughTraitSet = traitSet.replace(collation);
+ return Pair.of(passThroughTraitSet,
+ ImmutableList.of(
+ passThroughTraitSet,
+ passThroughTraitSet.replace(RelCollations.EMPTY)));
+ }
+
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+ // should only derive traits (limited to collation for now) from left input.
+ assert childId == 0;
+
+ final RelCollation collation = childTraits.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ final RelTraitSet traits = traitSet.replace(collation);
+ return Pair.of(traits, ImmutableList.of(traits));
Review comment:
modified
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
hsyuan commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436379014
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,49 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ // EnumerableCorrelate traits passdown shall only pass through collation to left input.
+ // This is because for EnumerableCorrelate always uses left input as the outer loop,
+ // thus only left input can preserve ordering.
+
+ for (RelFieldCollation relFieldCollation : collation.getFieldCollations()) {
+ // If field collation belongs to right input: bail out.
+ if (relFieldCollation.getFieldIndex() >= getLeft().getRowType().getFieldCount()) {
+ return null;
+ }
+ }
+
+ final RelTraitSet passThroughTraitSet = traitSet.replace(collation);
+ return Pair.of(passThroughTraitSet,
+ ImmutableList.of(
+ passThroughTraitSet,
+ passThroughTraitSet.replace(RelCollations.EMPTY)));
+ }
+
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+ // should only derive traits (limited to collation for now) from left input.
+ assert childId == 0;
+
+ final RelCollation collation = childTraits.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ final RelTraitSet traits = traitSet.replace(collation);
+ return Pair.of(traits, ImmutableList.of(traits));
Review comment:
the list should contain right child's desired traitset.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
hsyuan commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436191153
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,35 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
Review comment:
Subquery, lateral query can only be inner, left outer correlate.
I don't know how to generate full/right outer correlate.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
hsyuan commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436291037
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,35 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ // EnumerableCorrelate traits passdown shall only pass through collation to left input.
+ // This is because for EnumerableCorrelate always uses left input as the outer loop,
+ // thus only left input can preserve ordering.
+
+ for (RelFieldCollation relFieldCollation : collation.getFieldCollations()) {
+ // If field collation belongs to right input: bail out.
+ if (relFieldCollation.getFieldIndex() >= getLeft().getRowType().getFieldCount()) {
+ return null;
+ }
+ }
+
+ final RelTraitSet passThroughTraitSet = traitSet.replace(collation);
+ return Pair.of(passThroughTraitSet,
+ ImmutableList.of(
+ passThroughTraitSet,
+ passThroughTraitSet.replace(RelCollations.EMPTY)));
+ }
+
+ @Override public DeriveMode getDeriveMode() {
+ return DeriveMode.LEFT_FIRST;
+ }
Review comment:
`deriveTraits` is still missing. If the left child can deliver some collation, the correlate should be able to deliver the same collation.
##########
File path: core/src/test/java/org/apache/calcite/test/TopDownOptTest.java
##########
@@ -132,6 +134,72 @@
.check();
}
+ // Order by left field(s): push down sort to left input.
+ @Test void testCorrelateInnerJoinDeriveLeft() {
+ final String sql = "select * from emp e\n"
+ + "join dept d on e.deptno=d.deptno\n"
+ + "order by e.ename";
+ Query.create(sql)
+ .addRule(JoinToCorrelateRule.INSTANCE)
+ .removeRule(EnumerableRules.ENUMERABLE_JOIN_RULE)
+ .removeRule(EnumerableRules.ENUMERABLE_MERGE_JOIN_RULE)
+ .removeRule(EnumerableRules.ENUMERABLE_SORT_RULE)
+ .check();
+ }
Review comment:
You can also use lateral to generate correlate if you would like to. The current tests are good enough, though.
e.g.
https://github.com/apache/calcite/blob/master/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml#L6581
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hsyuan closed pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
hsyuan closed pull request #2005:
URL: https://github.com/apache/calcite/pull/2005
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436881001
##########
File path: core/src/test/java/org/apache/calcite/test/TopDownOptTest.java
##########
@@ -132,6 +134,72 @@
.check();
}
+ // Order by left field(s): push down sort to left input.
+ @Test void testCorrelateInnerJoinDeriveLeft() {
+ final String sql = "select * from emp e\n"
+ + "join dept d on e.deptno=d.deptno\n"
+ + "order by e.ename";
+ Query.create(sql)
+ .addRule(JoinToCorrelateRule.INSTANCE)
+ .removeRule(EnumerableRules.ENUMERABLE_JOIN_RULE)
+ .removeRule(EnumerableRules.ENUMERABLE_MERGE_JOIN_RULE)
+ .removeRule(EnumerableRules.ENUMERABLE_SORT_RULE)
+ .check();
+ }
+
+ // Order by contains right field: sort cannot be pushed down.
+ @Test void testCorrelateInnerJoinNoDerive() {
+ final String sql = "select * from emp e\n"
+ + "join dept d on e.deptno=d.deptno\n"
+ + "order by e.ename, d.name";
Review comment:
Thanks for adding such test cases!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#issuecomment-640019300
Thanks for the feedback @hsyuan , @amaliujia . I have added some unit tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#discussion_r436306942
##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCorrelate.java
##########
@@ -81,6 +87,35 @@ public static EnumerableCorrelate create(
traitSet, left, right, correlationId, requiredColumns, joinType);
}
+ @Override public Pair<RelTraitSet, List<RelTraitSet>> passThroughTraits(
+ final RelTraitSet required) {
+ final RelCollation collation = required.getCollation();
+ if (collation == null || collation == RelCollations.EMPTY) {
+ return null;
+ }
+
+ // EnumerableCorrelate traits passdown shall only pass through collation to left input.
+ // This is because for EnumerableCorrelate always uses left input as the outer loop,
+ // thus only left input can preserve ordering.
+
+ for (RelFieldCollation relFieldCollation : collation.getFieldCollations()) {
+ // If field collation belongs to right input: bail out.
+ if (relFieldCollation.getFieldIndex() >= getLeft().getRowType().getFieldCount()) {
+ return null;
+ }
+ }
+
+ final RelTraitSet passThroughTraitSet = traitSet.replace(collation);
+ return Pair.of(passThroughTraitSet,
+ ImmutableList.of(
+ passThroughTraitSet,
+ passThroughTraitSet.replace(RelCollations.EMPTY)));
+ }
+
+ @Override public DeriveMode getDeriveMode() {
+ return DeriveMode.LEFT_FIRST;
+ }
Review comment:
Yes. Any collation can be derived from left input. Optimizer can decide later if derived collations are useful.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2005: [CALCITE-4041] Implement trait propagation for EnumerableCorrelate
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2005:
URL: https://github.com/apache/calcite/pull/2005#issuecomment-639459909
@hsyuan when you have some time, please take a look at the preliminary work on this PR (this is the first time that I touch trait propagation methods, so I am not very familiar with this subject).
To be done: unit tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org