[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431610131
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
Now it generates a correct plan (sort enforced for top projection).
Also changed this test case to `select a, b*-1, c` pattern for better
testing.
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431610131
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
Now it generates a correct plan (sort enforced for top projection).
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on pull request #1985: URL: https://github.com/apache/calcite/pull/1985#issuecomment-635136029 Rebased on master. 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: us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431609986
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +100,74 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+for (RelFieldCollation rc : collation.getFieldCollations()) {
+ if (!isCollationOnTrivialExpr(map, rc)) {
+return null;
+ }
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
Review comment:
Thanks. Updated.
Later we could add util functions for such reverse operations.
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on pull request #1985: URL: https://github.com/apache/calcite/pull/1985#issuecomment-635132405 Can you rebase on latest master? 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: us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431605210
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
Will make a change (which is easy), just add a break after encountering
first non-trivial expr.
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431604532
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
I see. So basically the correctness of ordering is just partial sequence
ordering that counts from beginning.
Say sort on [a, b, c, d], then it only guarantees [a], [a, b], [a, b, c] and
[a, b, c, d].
And the rational is any non-trivial expr will break the ordering guarantees,
thus produce a wrong plan.
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431602181
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +100,74 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+for (RelFieldCollation rc : collation.getFieldCollations()) {
+ if (!isCollationOnTrivialExpr(map, rc)) {
+return null;
+ }
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
Review comment:
Let's not consider `cast` in this PR:
```java
int maxField = Math.max(inputFieldCount, projects.size());
Mappings.TargetMapping mapping = Mappings
.create(MappingType.FUNCTION, maxField, maxField);
for (int i = 0; i < projects.size(); i++) {
RexNode exp = projects.get(i);
if (exp instanceof RexInputRef) {
mapping.set(((RexInputRef) exp).getIndex(), i);
}
}
```
If you check the body of `permutationIgnoreCast`, they are just reversed
mapping.
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:
us...@infra.apache.org
[calcite] branch master updated: [CALCITE-3972] Allow RelBuilder to create RelNode with convention (Xiening Dai)
This is an automated email from the ASF dual-hosted git repository.
hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new ada6cc4 [CALCITE-3972] Allow RelBuilder to create RelNode with
convention (Xiening Dai)
ada6cc4 is described below
commit ada6cc4309dd79e09abcbd67570b786d80340018
Author: Xiening Dai
AuthorDate: Tue Mar 24 16:29:57 2020 -0700
[CALCITE-3972] Allow RelBuilder to create RelNode with convention (Xiening
Dai)
1. Provide Convention.transformRelBuilder() to transform an existing
RelBuilder
into one with specific convention.
2. RelBuilder provides withRelFactories() method to allow caller swap the
underlying RelFactories and create a new builder.
We can avoid ~1/3 of total rule firings in a N way join case with this
change.
Close #1884
---
.../adapter/enumerable/EnumerableConvention.java | 11 +++
.../adapter/enumerable/EnumerableRelFactories.java | 102 +
.../apache/calcite/plan/AbstractRelOptPlanner.java | 10 ++
.../java/org/apache/calcite/plan/Convention.java | 8 ++
.../calcite/rel/rules/JoinPushThroughJoinRule.java | 2 +-
.../java/org/apache/calcite/tools/RelBuilder.java | 18 +++-
.../org/apache/calcite/test/RelBuilderTest.java| 39
site/_docs/algebra.md | 15 +++
8 files changed, 203 insertions(+), 2 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableConvention.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableConvention.java
index 5b583da..48ae551 100644
---
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableConvention.java
+++
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableConvention.java
@@ -16,6 +16,7 @@
*/
package org.apache.calcite.adapter.enumerable;
+import org.apache.calcite.plan.Contexts;
import org.apache.calcite.plan.Convention;
import org.apache.calcite.plan.ConventionTraitDef;
import org.apache.calcite.plan.RelOptPlanner;
@@ -25,6 +26,7 @@ import org.apache.calcite.plan.RelTraitSet;
import org.apache.calcite.rel.RelCollation;
import org.apache.calcite.rel.RelCollationTraitDef;
import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.RelFactories;
/**
* Family of calling conventions that return results as an
@@ -84,4 +86,13 @@ public enum EnumerableConvention implements Convention {
RelTraitSet toTraits) {
return true;
}
+
+ public RelFactories.Struct getRelFactories() {
+return RelFactories.Struct.fromContext(
+Contexts.of(
+EnumerableRelFactories.ENUMERABLE_TABLE_SCAN_FACTORY,
+EnumerableRelFactories.ENUMERABLE_PROJECT_FACTORY,
+EnumerableRelFactories.ENUMERABLE_FILTER_FACTORY,
+EnumerableRelFactories.ENUMERABLE_SORT_FACTORY));
+ }
}
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelFactories.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelFactories.java
new file mode 100644
index 000..ab9cdd0
--- /dev/null
+++
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelFactories.java
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.adapter.enumerable;
+
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.rel.RelCollation;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.hint.RelHint;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Contains factory interface and default implementation for creating various
+ * rel nodes.
+ */
+public class EnumerableRelFactories {
+
+ public static final org.apache.calcite.rel.core.RelFactories.TableScanFactory
+ ENUMERABLE_TABLE_SCAN_FACTORY = new TableScanFactoryImpl();
+
+ public
[GitHub] [calcite] hsyuan closed pull request #1884: [CALCITE-3972] Allow RelBuilder to create RelNode with convention and use it for trait convert
hsyuan closed pull request #1884: URL: https://github.com/apache/calcite/pull/1884 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: us...@infra.apache.org
[GitHub] [calcite] wenhuitang commented on a change in pull request #1990: [CALCITE-4026] When the cassandra primary key is of type bigint,the query will report an error
wenhuitang commented on a change in pull request #1990:
URL: https://github.com/apache/calcite/pull/1990#discussion_r431575566
##
File path:
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java
##
@@ -174,14 +174,13 @@ private String translateMatch(RexNode condition) {
}
}
-/** Convert the value of a literal to a string.
+/** Returns the value of the literal.
*
* @param literal Literal to translate
- * @return String representation of the literal
+ * @return The value of the literal in the form of the actual type.
*/
-private static String literalValue(RexLiteral literal) {
- Object value = literal.getValue2();
- return String.valueOf(value);
+private static Object literalValue(RexLiteral literal) {
+ return literal.getValue2();
}
Review comment:
> For `Integral` and `string` it is correct, but i'm not sure if it is
for other data types literal, such as `TIMESTAMP(3)`, can you make sure that ?
Thanks a lot for reviewing. This PR has been updated.
I have tested for type like Date, Timestamp and Time.
1.When the type of primary key 'f_time' is Time, it will throw exception
when the filter condition is "f_time = Time '13:30:54.234'", because the type
of 'f_time' is Bigint.
It seems another issue.
2.As for Timestamp(3), CassandraSchema create a Sql type Timestamp with
precision 0. So It is possible to lose precision when get timestamp data from
Cassandra.
As for above two issues, Whether we should solve them at this PR or creat
new jira issues for them?
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431561103
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +100,74 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+for (RelFieldCollation rc : collation.getFieldCollations()) {
+ if (!isCollationOnTrivialExpr(map, rc)) {
+return null;
+ }
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
Review comment:
You can't use the same mapping. This is used to map parent to child.
What you want is map child position to parent position.
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431559976
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
For 2, say the input is sorted by a,b,c.
There is a project on top of that, select a, b*-1 as d, c.
With your approach, it will derive collation on [a, c], which is wrong. Its
derived collation should be just [a]. Any non-trivial expr will break the
collation.
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985: URL: https://github.com/apache/calcite/pull/1985#discussion_r431486913 ## File path: core/src/test/resources/org/apache/calcite/test/TopDownOptTest.xml ## @@ -318,6 +318,31 @@ EnumerableProject(ENAME=[$0], JOB=[$1], EXPR$2=[$2], ENAME0=[$3], JOB0=[$4], SAL EnumerableSort(sort0=[$4], sort1=[$0], dir0=[ASC], dir1=[ASC]) EnumerableProject(ENAME=[$0], JOB=[$1], SAL=[$2], COMM=[$3], JOB0=[CAST($1):BIGINT NOT NULL]) EnumerableTableScan(table=[[CATALOG, SALES, BONUS]]) +]]> + + + + + + + + + + +
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985: URL: https://github.com/apache/calcite/pull/1985#discussion_r431484608 ## File path: core/src/test/resources/org/apache/calcite/test/TopDownOptTest.xml ## @@ -318,6 +318,31 @@ EnumerableProject(ENAME=[$0], JOB=[$1], EXPR$2=[$2], ENAME0=[$3], JOB0=[$4], SAL EnumerableSort(sort0=[$4], sort1=[$0], dir0=[ASC], dir1=[ASC]) EnumerableProject(ENAME=[$0], JOB=[$1], SAL=[$2], COMM=[$3], JOB0=[CAST($1):BIGINT NOT NULL]) EnumerableTableScan(table=[[CATALOG, SALES, BONUS]]) +]]> + + + + + + + + + + +
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985: URL: https://github.com/apache/calcite/pull/1985#discussion_r431484608 ## File path: core/src/test/resources/org/apache/calcite/test/TopDownOptTest.xml ## @@ -318,6 +318,31 @@ EnumerableProject(ENAME=[$0], JOB=[$1], EXPR$2=[$2], ENAME0=[$3], JOB0=[$4], SAL EnumerableSort(sort0=[$4], sort1=[$0], dir0=[ASC], dir1=[ASC]) EnumerableProject(ENAME=[$0], JOB=[$1], SAL=[$2], COMM=[$3], JOB0=[CAST($1):BIGINT NOT NULL]) EnumerableTableScan(table=[[CATALOG, SALES, BONUS]]) +]]> + + + + + + + + + + +
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985: URL: https://github.com/apache/calcite/pull/1985#discussion_r431484135 ## File path: core/src/test/resources/org/apache/calcite/test/TopDownOptTest.xml ## @@ -318,6 +318,31 @@ EnumerableProject(ENAME=[$0], JOB=[$1], EXPR$2=[$2], ENAME0=[$3], JOB0=[$4], SAL EnumerableSort(sort0=[$4], sort1=[$0], dir0=[ASC], dir1=[ASC]) EnumerableProject(ENAME=[$0], JOB=[$1], SAL=[$2], COMM=[$3], JOB0=[CAST($1):BIGINT NOT NULL]) EnumerableTableScan(table=[[CATALOG, SALES, BONUS]]) +]]> + + + + + + + + + + +
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985: URL: https://github.com/apache/calcite/pull/1985#discussion_r431484135 ## File path: core/src/test/resources/org/apache/calcite/test/TopDownOptTest.xml ## @@ -318,6 +318,31 @@ EnumerableProject(ENAME=[$0], JOB=[$1], EXPR$2=[$2], ENAME0=[$3], JOB0=[$4], SAL EnumerableSort(sort0=[$4], sort1=[$0], dir0=[ASC], dir1=[ASC]) EnumerableProject(ENAME=[$0], JOB=[$1], SAL=[$2], COMM=[$3], JOB0=[CAST($1):BIGINT NOT NULL]) EnumerableTableScan(table=[[CATALOG, SALES, BONUS]]) +]]> + + + + + + + + + + +
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985: URL: https://github.com/apache/calcite/pull/1985#discussion_r431483152 ## File path: core/src/test/resources/org/apache/calcite/test/TopDownOptTest.xml ## @@ -318,6 +318,31 @@ EnumerableProject(ENAME=[$0], JOB=[$1], EXPR$2=[$2], ENAME0=[$3], JOB0=[$4], SAL EnumerableSort(sort0=[$4], sort1=[$0], dir0=[ASC], dir1=[ASC]) EnumerableProject(ENAME=[$0], JOB=[$1], SAL=[$2], COMM=[$3], JOB0=[CAST($1):BIGINT NOT NULL]) EnumerableTableScan(table=[[CATALOG, SALES, BONUS]]) +]]> + + + + + + + + + + +
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431482634
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -128,36 +132,42 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
final Mappings.TargetMapping map =
RelOptUtil.permutationIgnoreCast(
getProjects(), getInput().getRowType());
-if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
- return null;
+
+List collationFieldsToDerive = new ArrayList<>();
+for (RelFieldCollation rc : collation.getFieldCollations()) {
+ if (isCollationOnTrivialExpr(map, rc)) {
+collationFieldsToDerive.add(rc);
+ }
Review comment:
This is a good test case. Added (see `testSortProjectDerive5`)
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431473791
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -128,36 +132,42 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
final Mappings.TargetMapping map =
RelOptUtil.permutationIgnoreCast(
getProjects(), getInput().getRowType());
-if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
- return null;
+
+List collationFieldsToDerive = new ArrayList<>();
+for (RelFieldCollation rc : collation.getFieldCollations()) {
+ if (isCollationOnTrivialExpr(map, rc)) {
+collationFieldsToDerive.add(rc);
+ }
Review comment:
Let's see this query's plan:
```sql
select a, b*-1 as b, c from (select * from foo order by a, b, c limit 10)
order by a, c;
```
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431469982
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
Now I change the implementation to:
1. traits pass through: return null when at least one collation is defined
on non-trivial expr.
2. traits derivation: only return null when all collations are defined on
non-trivial exprs. Otherwise derive those collations that are defined on
trivial expr.
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431469217
##
File path: core/src/test/java/org/apache/calcite/test/TopDownOptTest.java
##
@@ -64,93 +69,243 @@
* Run the test one last time; this time it should pass.
*
*/
-class TopDownOptTest extends RelOptTestBase {
-
- protected DiffRepository getDiffRepos() {
-return DiffRepository.lookup(TopDownOptTest.class);
- }
-
- Sql sql(String sql) {
-VolcanoPlanner planner = new VolcanoPlanner();
-// Always use top-down optimization
-planner.setTopDownOpt(true);
-planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
-planner.addRelTraitDef(RelCollationTraitDef.INSTANCE);
-
-RelOptUtil.registerDefaultRules(planner, false, false);
-
-// Keep deterministic join order
-planner.removeRule(JoinCommuteRule.INSTANCE);
-planner.removeRule(JoinPushThroughJoinRule.LEFT);
-planner.removeRule(JoinPushThroughJoinRule.RIGHT);
-
-// Always use merge join
-planner.removeRule(EnumerableRules.ENUMERABLE_JOIN_RULE);
-planner.removeRule(EnumerableRules.ENUMERABLE_CALC_RULE);
-
-// Always use sorted agg
-planner.removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE);
-planner.addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE);
-
-Tester tester = createTester().withDecorrelation(true)
-.withClusterFactory(cluster -> RelOptCluster.create(planner,
cluster.getRexBuilder()));
-
-return new Sql(tester, sql, null, planner,
-ImmutableMap.of(), ImmutableList.of());
- }
-
+class TopDownOptTest {
Review comment:
Added `extends RelOptTestBase`
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431465794
##
File path: core/src/test/java/org/apache/calcite/test/TopDownOptTest.java
##
@@ -64,93 +69,243 @@
* Run the test one last time; this time it should pass.
*
*/
-class TopDownOptTest extends RelOptTestBase {
-
- protected DiffRepository getDiffRepos() {
-return DiffRepository.lookup(TopDownOptTest.class);
- }
-
- Sql sql(String sql) {
-VolcanoPlanner planner = new VolcanoPlanner();
-// Always use top-down optimization
-planner.setTopDownOpt(true);
-planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
-planner.addRelTraitDef(RelCollationTraitDef.INSTANCE);
-
-RelOptUtil.registerDefaultRules(planner, false, false);
-
-// Keep deterministic join order
-planner.removeRule(JoinCommuteRule.INSTANCE);
-planner.removeRule(JoinPushThroughJoinRule.LEFT);
-planner.removeRule(JoinPushThroughJoinRule.RIGHT);
-
-// Always use merge join
-planner.removeRule(EnumerableRules.ENUMERABLE_JOIN_RULE);
-planner.removeRule(EnumerableRules.ENUMERABLE_CALC_RULE);
-
-// Always use sorted agg
-planner.removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE);
-planner.addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE);
-
-Tester tester = createTester().withDecorrelation(true)
-.withClusterFactory(cluster -> RelOptCluster.create(planner,
cluster.getRexBuilder()));
-
-return new Sql(tester, sql, null, planner,
-ImmutableMap.of(), ImmutableList.of());
- }
-
+class TopDownOptTest {
Review comment:
Checkout this issue: CALCITE-4027
I think we still need `extends RelOptTestBase` if we want to leverage it.
Otherwise we have to manually copy from `TopDownOptTest_actual.xml`.
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431465794
##
File path: core/src/test/java/org/apache/calcite/test/TopDownOptTest.java
##
@@ -64,93 +69,243 @@
* Run the test one last time; this time it should pass.
*
*/
-class TopDownOptTest extends RelOptTestBase {
-
- protected DiffRepository getDiffRepos() {
-return DiffRepository.lookup(TopDownOptTest.class);
- }
-
- Sql sql(String sql) {
-VolcanoPlanner planner = new VolcanoPlanner();
-// Always use top-down optimization
-planner.setTopDownOpt(true);
-planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
-planner.addRelTraitDef(RelCollationTraitDef.INSTANCE);
-
-RelOptUtil.registerDefaultRules(planner, false, false);
-
-// Keep deterministic join order
-planner.removeRule(JoinCommuteRule.INSTANCE);
-planner.removeRule(JoinPushThroughJoinRule.LEFT);
-planner.removeRule(JoinPushThroughJoinRule.RIGHT);
-
-// Always use merge join
-planner.removeRule(EnumerableRules.ENUMERABLE_JOIN_RULE);
-planner.removeRule(EnumerableRules.ENUMERABLE_CALC_RULE);
-
-// Always use sorted agg
-planner.removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE);
-planner.addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE);
-
-Tester tester = createTester().withDecorrelation(true)
-.withClusterFactory(cluster -> RelOptCluster.create(planner,
cluster.getRexBuilder()));
-
-return new Sql(tester, sql, null, planner,
-ImmutableMap.of(), ImmutableList.of());
- }
-
+class TopDownOptTest {
Review comment:
Checkout this issue: CALCITE-4027
I think we still needs `extends RelOptTestBase` if we want to leverage it.
Otherwise we have to manually copy from `TopDownOptTest_actual.xml`.
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431442043
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
lot of thanks for your explanation. Now I understand it more:
Trait derivation can pass collations up when such collations are defined on
trivial exprs.
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:
us...@infra.apache.org
[calcite] branch master updated (2b1254b -> b166b9a)
This is an automated email from the ASF dual-hosted git repository. hyuan pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 2b1254b [CALCITE-4009] Remove traitset remapping in ProjectJoinTransposeRule add b166b9a [CALCITE-4004] Show RelOptRuleOperand description in debugger to facilitate debugging No new revisions were added by this update. Summary of changes: .../org/apache/calcite/plan/RelOptRuleOperand.java | 64 ++ 1 file changed, 64 insertions(+)
[calcite] branch master updated (2b1254b -> b166b9a)
This is an automated email from the ASF dual-hosted git repository. hyuan pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 2b1254b [CALCITE-4009] Remove traitset remapping in ProjectJoinTransposeRule add b166b9a [CALCITE-4004] Show RelOptRuleOperand description in debugger to facilitate debugging No new revisions were added by this update. Summary of changes: .../org/apache/calcite/plan/RelOptRuleOperand.java | 64 ++ 1 file changed, 64 insertions(+)
[GitHub] [calcite] hsyuan closed pull request #1978: [CALCITE-4004] Show RelOptRuleOperand description in IDE debugger to facilitate debugging
hsyuan closed pull request #1978: URL: https://github.com/apache/calcite/pull/1978 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: us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431314094
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +100,52 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+// Determine mapping between project input and output fields. If sort
+// relies on non-trivial expressions, we can't push.
+for (RelFieldCollation fc : collation.getFieldCollations()) {
+ if (map.getTargetOpt(fc.getFieldIndex()) < 0) {
+return null;
+ }
+ final RexNode node = getProjects().get(fc.getFieldIndex());
+ if (node.isA(SqlKind.CAST)) {
+// Check whether it is a monotonic preserving cast, otherwise we
cannot push
+final RexCall cast = (RexCall) node;
+RelFieldCollation newFc = Objects.requireNonNull(RexUtil.apply(map,
fc));
+final RexCallBinding binding =
+RexCallBinding.create(getCluster().getTypeFactory(), cast,
+ImmutableList.of(RelCollations.of(newFc)));
+if (cast.getOperator().getMonotonicity(binding) ==
SqlMonotonicity.NOT_MONOTONIC) {
+ return null;
+}
+ }
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+
+return Pair.of(getTraitSet().replace(collation),
ImmutableList.of(childTraits));
Review comment:
That's a great point. I also realized new tests's names cannot explain
their purpose already. I was thinking a better way.
Will add more details comments.
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431184692
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
For the last 2 negative test cases, I think you already have similar
ones in your test.
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431120503
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
The test is difficult to come up with for derivation. :)
```sql
-- b is int, no sort for join's left input
select * from
(select a, cast(b as varchar) as d, c+1 as c from
(select * from foo order by a desc, b desc, c limit 10) t) foo
join bar
on foo.a = bar.a;
```
```sql
-- b is int, no sort for join's left input
select * from
(select a, cast(b as bigint) as d, c+1 as c from
(select * from foo order by a desc, b desc, c limit 10) t) foo
join bar
on foo.a = bar.a and foo.d = bar.d;
```
```sql
-- b is int, need sort for join's left input
select * from
(select a, cast(b as varchar) as d, c+1 as c from
(select * from foo order by a desc, b desc, c limit 10) t) foo
join bar
on foo.a = bar.a and foo.d = bar.d;
```
```sql
-- b is int, need sort for join's left input
select * from
(select a, b*-2 as d, c from
(select * from foo order by a desc, b desc, c limit 10) t) foo
join bar
on foo.a = bar.a and foo.d = bar.d;
```
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431120503
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
```sql
-- no sort for order by
select a, b*2 as d, c from (select distinct a, b, c from foo) order by a;
```
```sql
-- b is int, no sort for order by
select a, cast(b as bigint) as d, c from (select distinct a, b, c from foo)
order by a, d;
```
```sql
-- b is int, need sort for order by
select a, cast(b as varchar) as d, c from (select distinct a, b, c from foo)
order by a, d;
```
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:
us...@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431072268
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableFilter.java
##
@@ -68,4 +74,25 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalc is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
Review comment:
We can replace `collation` with `required.getCollation()`.
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
Review comment:
Rename to `IsCollationOnTrivialExpr()`, and reverse the return value.
```
if (IsCollationOnTrivialExpr()) {
return Pair.of();
} else {
return null;
}
```
Not succinct, but clear.
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
+ return null;
+}
+
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+if (checkIfSortIsDefinedOnNonTrivalExpr(map, collation)) {
+ return null;
Review comment:
Here is something `derive` works differently with `passThrough`.
When the parent request a collation on [a,b,c], if b is a expr, say `a*k`,
we will find that only `a` is mappable. But we can't pass down [a], because
even the project can deliver collation on [a], it still can't satisfy [a,b,c],
so Sort will be enforced. That will be a waste.
But if the child of Project delivers collation on [a,b,c], and the overall
project expressions are (a, b, c*k, d, e), we can derive collation on [a,b] for
the Project operator. This might be useful for parent operator. (Might still be
wasteful, we'd better consider the parent request when derivation, but I guess
that is something can be framework's duty, in the future).
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableFilter.java
##
@@ -68,4 +74,25 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalc is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+
+// Filter, not like Project, does not change on which column that
collation defines.
+return Pair.of(required, ImmutableList.of(required));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null) {
Review comment:
same here
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,65 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollati
[GitHub] [calcite] hsyuan commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
hsyuan commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r431071501
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +100,52 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+// Determine mapping between project input and output fields. If sort
+// relies on non-trivial expressions, we can't push.
+for (RelFieldCollation fc : collation.getFieldCollations()) {
+ if (map.getTargetOpt(fc.getFieldIndex()) < 0) {
+return null;
+ }
+ final RexNode node = getProjects().get(fc.getFieldIndex());
+ if (node.isA(SqlKind.CAST)) {
+// Check whether it is a monotonic preserving cast, otherwise we
cannot push
+final RexCall cast = (RexCall) node;
+RelFieldCollation newFc = Objects.requireNonNull(RexUtil.apply(map,
fc));
+final RexCallBinding binding =
+RexCallBinding.create(getCluster().getTypeFactory(), cast,
+ImmutableList.of(RelCollations.of(newFc)));
+if (cast.getOperator().getMonotonicity(binding) ==
SqlMonotonicity.NOT_MONOTONIC) {
+ return null;
+}
+ }
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+
+return Pair.of(getTraitSet().replace(collation),
ImmutableList.of(childTraits));
Review comment:
Correct, this is expected.
Can you add some comments to state the purpose of these tests. Sorry for not
giving a good example in my code.
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:
us...@infra.apache.org
[GitHub] [calcite] danny0405 commented on a change in pull request #1990: [CALCITE-4026] When the cassandra primary key is of type bigint,the query will report an error
danny0405 commented on a change in pull request #1990:
URL: https://github.com/apache/calcite/pull/1990#discussion_r431061963
##
File path:
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java
##
@@ -174,14 +174,13 @@ private String translateMatch(RexNode condition) {
}
}
-/** Convert the value of a literal to a string.
+/** Returns the value of the literal.
*
* @param literal Literal to translate
- * @return String representation of the literal
+ * @return The value of the literal in the form of the actual type.
*/
-private static String literalValue(RexLiteral literal) {
- Object value = literal.getValue2();
- return String.valueOf(value);
+private static Object literalValue(RexLiteral literal) {
+ return literal.getValue2();
}
Review comment:
For `Integral` and `string` it is correct, but i'm not sure if it is for
other data types literal, such as `TIMESTAMP(3)`, can you make sure that ?
##
File path:
cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterDataTypesTest.java
##
@@ -79,6 +79,18 @@ static void load(Session session) {
+ ", f_varint INTEGER]");
}
+ @Test void testSimpleQuery() {
+CalciteAssert.that()
Review comment:
`testSimpleQuery()` -> `testFilterWithNonStringLiteral` ?
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:
us...@infra.apache.org
[GitHub] [calcite] wenhuitang opened a new pull request #1990: [CALCITE-4026] When the cassandra primary key is of type bigint,he query will report an error
wenhuitang opened a new pull request #1990: URL: https://github.com/apache/calcite/pull/1990 PR for issue https://issues.apache.org/jira/browse/CALCITE-4026 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: us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r430922984
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +100,52 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+// Determine mapping between project input and output fields. If sort
+// relies on non-trivial expressions, we can't push.
+for (RelFieldCollation fc : collation.getFieldCollations()) {
+ if (map.getTargetOpt(fc.getFieldIndex()) < 0) {
+return null;
+ }
+ final RexNode node = getProjects().get(fc.getFieldIndex());
+ if (node.isA(SqlKind.CAST)) {
+// Check whether it is a monotonic preserving cast, otherwise we
cannot push
+final RexCall cast = (RexCall) node;
+RelFieldCollation newFc = Objects.requireNonNull(RexUtil.apply(map,
fc));
+final RexCallBinding binding =
+RexCallBinding.create(getCluster().getTypeFactory(), cast,
+ImmutableList.of(RelCollations.of(newFc)));
+if (cast.getOperator().getMonotonicity(binding) ==
SqlMonotonicity.NOT_MONOTONIC) {
+ return null;
+}
+ }
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+
+return Pair.of(getTraitSet().replace(collation),
ImmutableList.of(childTraits));
Review comment:
And regarding the distinct query, I added it as the query in test case
`testSortProjectDerive2`, where it generates a plan:
```
EnumerableSortedAggregate(group=[{0, 1, 2}])
EnumerableSort(sort0=[$0], sort1=[$1], sort2=[$2], dir0=[ASC], dir1=[ASC],
dir2=[ASC])
EnumerableProject(ENAME=[$0], EXPR$1=[*($2, -2)], MGR=[$1])
EnumerableLimit(fetch=[100])
EnumerableProject(ENAME=[$1], MGR=[$3], SAL=[$5])
EnumerableSort(sort0=[$1], sort1=[$3], sort2=[$5], dir0=[ASC],
dir1=[ASC], dir2=[ASC])
EnumerableTableScan(table=[[CATALOG, SALES, EMP]])
```
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r430913096
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +100,52 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+// Determine mapping between project input and output fields. If sort
+// relies on non-trivial expressions, we can't push.
+for (RelFieldCollation fc : collation.getFieldCollations()) {
+ if (map.getTargetOpt(fc.getFieldIndex()) < 0) {
+return null;
+ }
+ final RexNode node = getProjects().get(fc.getFieldIndex());
+ if (node.isA(SqlKind.CAST)) {
+// Check whether it is a monotonic preserving cast, otherwise we
cannot push
+final RexCall cast = (RexCall) node;
+RelFieldCollation newFc = Objects.requireNonNull(RexUtil.apply(map,
fc));
+final RexCallBinding binding =
+RexCallBinding.create(getCluster().getTypeFactory(), cast,
+ImmutableList.of(RelCollations.of(newFc)));
+if (cast.getOperator().getMonotonicity(binding) ==
SqlMonotonicity.NOT_MONOTONIC) {
+ return null;
+}
+ }
+}
+
+final RelCollation newCollation = collation.apply(map);
+return Pair.of(required, ImmutableList.of(required.replace(newCollation)));
+ }
+
+ @Override public Pair> deriveTraits(
+ final RelTraitSet childTraits, final int childId) {
+RelCollation collation = childTraits.getCollation();
+if (collation == null || collation.getFieldCollations().size() == 0) {
+ return null;
+}
+
+return Pair.of(getTraitSet().replace(collation),
ImmutableList.of(childTraits));
Review comment:
Have added the logic to deal with SqlMonotonicity for traits derivation,
and add one positive test case
`testSortProjectDeriveWhenCastLeadingToMonotonic` and one negative test case
`testSortProjectDeriveWhenCastLeadingToNonMonotonic`
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:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #1985: [CALCITE-4011] Support trait propagation for EnumerableProject and EnumerableFilter
amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r430908505
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##
@@ -86,4 +99,48 @@ public Result implement(EnumerableRelImplementor
implementor, Prefer pref) {
// EnumerableCalcRel is always better
throw new UnsupportedOperationException();
}
+
+ @Override public Pair> passThroughTraits(
+ RelTraitSet required) {
+RelCollation collation = required.getCollation();
+if (collation == null) {
+ return null;
+}
+final Mappings.TargetMapping map =
+RelOptUtil.permutationIgnoreCast(
+getProjects(), getInput().getRowType());
+
+// Determine mapping between project input and output fields. If sort
+// relies on non-trivial expressions, we can't push.
+for (RelFieldCollation fc : collation.getFieldCollations()) {
+ if (map.getTargetOpt(fc.getFieldIndex()) < 0) {
+return null;
Review comment:
Good point. Add one test case for trait push down
`testSortProjectOnRexCall` and one for trait derivation
`testSortProjectDeriveOnRexCall`
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:
us...@infra.apache.org
