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 2019/12/25 05:39:40 UTC
[GitHub] [calcite] xy2953396112 opened a new pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
xy2953396112 opened a new pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688
JIRA: https://issues.apache.org/jira/browse/CALCITE-3626
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] xy2953396112 commented on a change in pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#discussion_r362360499
##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1406,6 +1410,48 @@ private JoinOnCalcsToJoinUnifyRule() {
}
}
+ /**
+ * A {@link SubstitutionVisitor.UnifyRule} that matches a {@link MutableSort}
+ * which has {@link MutableCalc} as child to a {@link MutableSort}.
+ * We try to pull up the {@link MutableCalc} to top of {@link MutableSort},
+ * then match the {@link MutableSort} in query to {@link MutableSort} in target.
+ */
+ private static class SortOnCalcToSortUnifyRule extends AbstractUnifyRule {
+
+ public static final SortOnCalcToSortUnifyRule INSTANCE =
+ new SortOnCalcToSortUnifyRule();
+
+ private SortOnCalcToSortUnifyRule() {
+ super(operand(MutableSort.class, operand(MutableCalc.class, query(0))),
+ operand(MutableSort.class, target(0)), 1);
+ }
+
+ @Override protected UnifyResult apply(UnifyRuleCall call) {
+ final MutableSort query = (MutableSort) call.query;
+ final MutableCalc qInput = (MutableCalc) query.getInput();
+ final Pair<RexNode, List<RexNode>> qInputExplained = explainCalc(qInput);
+ final List<RexNode> qInputProjs = qInputExplained.right;
+
+ final MutableSort target = (MutableSort) call.target;
+ if (!Objects.equals(query.fetch, target.fetch)
+ || !Objects.equals(query.offset, target.offset)) {
+ return null;
+ }
+ final Mappings.TargetMapping mapping =
+ Project.getMapping(fieldCnt(qInput.getInput()), qInputProjs);
+ if (mapping == null) {
+ return null;
+ }
+ RelCollation collation = RelCollations.permute(query.collation, mapping.inverse());
Review comment:
> final ?
ok
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] xy2953396112 commented on a change in pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#discussion_r362360439
##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1406,6 +1410,48 @@ private JoinOnCalcsToJoinUnifyRule() {
}
}
+ /**
+ * A {@link SubstitutionVisitor.UnifyRule} that matches a {@link MutableSort}
+ * which has {@link MutableCalc} as child to a {@link MutableSort}.
+ * We try to pull up the {@link MutableCalc} to top of {@link MutableSort},
+ * then match the {@link MutableSort} in query to {@link MutableSort} in target.
+ */
+ private static class SortOnCalcToSortUnifyRule extends AbstractUnifyRule {
+
+ public static final SortOnCalcToSortUnifyRule INSTANCE =
+ new SortOnCalcToSortUnifyRule();
+
+ private SortOnCalcToSortUnifyRule() {
+ super(operand(MutableSort.class, operand(MutableCalc.class, query(0))),
+ operand(MutableSort.class, target(0)), 1);
+ }
+
+ @Override protected UnifyResult apply(UnifyRuleCall call) {
+ final MutableSort query = (MutableSort) call.query;
+ final MutableCalc qInput = (MutableCalc) query.getInput();
+ final Pair<RexNode, List<RexNode>> qInputExplained = explainCalc(qInput);
+ final List<RexNode> qInputProjs = qInputExplained.right;
+
+ final MutableSort target = (MutableSort) call.target;
+ if (!Objects.equals(query.fetch, target.fetch)
+ || !Objects.equals(query.offset, target.offset)) {
Review comment:
> I would rather to match only when fetch and offset are null.
When query and target have the same fetch and offset, it should be matched. you mean this rule should have same collations to keep the data semantics ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] jinxing64 commented on issue #1688: [CALCITE-3626] Add
SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
jinxing64 commented on issue #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#issuecomment-569057257
Before this PR, shall we have an implementation of `SortToSortUnifyRule` ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] jinxing64 commented on a change in pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
jinxing64 commented on a change in pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#discussion_r361453523
##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1406,6 +1410,48 @@ private JoinOnCalcsToJoinUnifyRule() {
}
}
+ /**
+ * A {@link SubstitutionVisitor.UnifyRule} that matches a {@link MutableSort}
+ * which has {@link MutableCalc} as child to a {@link MutableSort}.
+ * We try to pull up the {@link MutableCalc} to top of {@link MutableSort},
+ * then match the {@link MutableSort} in query to {@link MutableSort} in target.
+ */
+ private static class SortOnCalcToSortUnifyRule extends AbstractUnifyRule {
+
+ public static final SortOnCalcToSortUnifyRule INSTANCE =
+ new SortOnCalcToSortUnifyRule();
+
+ private SortOnCalcToSortUnifyRule() {
+ super(operand(MutableSort.class, operand(MutableCalc.class, query(0))),
+ operand(MutableSort.class, target(0)), 1);
+ }
+
+ @Override protected UnifyResult apply(UnifyRuleCall call) {
+ final MutableSort query = (MutableSort) call.query;
+ final MutableCalc qInput = (MutableCalc) query.getInput();
+ final Pair<RexNode, List<RexNode>> qInputExplained = explainCalc(qInput);
+ final List<RexNode> qInputProjs = qInputExplained.right;
+
+ final MutableSort target = (MutableSort) call.target;
+ if (!Objects.equals(query.fetch, target.fetch)
+ || !Objects.equals(query.offset, target.offset)) {
Review comment:
When collations are not exactly the same, the data semantics will be wrong if offset is specified, though the offsets are the same.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] chunweilei commented on a change in pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#discussion_r361264735
##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1406,6 +1410,48 @@ private JoinOnCalcsToJoinUnifyRule() {
}
}
+ /**
+ * A {@link SubstitutionVisitor.UnifyRule} that matches a {@link MutableSort}
+ * which has {@link MutableCalc} as child to a {@link MutableSort}.
+ * We try to pull up the {@link MutableCalc} to top of {@link MutableSort},
+ * then match the {@link MutableSort} in query to {@link MutableSort} in target.
+ */
+ private static class SortOnCalcToSortUnifyRule extends AbstractUnifyRule {
+
+ public static final SortOnCalcToSortUnifyRule INSTANCE =
+ new SortOnCalcToSortUnifyRule();
+
+ private SortOnCalcToSortUnifyRule() {
+ super(operand(MutableSort.class, operand(MutableCalc.class, query(0))),
+ operand(MutableSort.class, target(0)), 1);
+ }
+
+ @Override protected UnifyResult apply(UnifyRuleCall call) {
+ final MutableSort query = (MutableSort) call.query;
+ final MutableCalc qInput = (MutableCalc) query.getInput();
+ final Pair<RexNode, List<RexNode>> qInputExplained = explainCalc(qInput);
+ final List<RexNode> qInputProjs = qInputExplained.right;
+
+ final MutableSort target = (MutableSort) call.target;
+ if (!Objects.equals(query.fetch, target.fetch)
+ || !Objects.equals(query.offset, target.offset)) {
+ return null;
Review comment:
How about `!(Objects.equals(query.fetch, target.fetch) &&Objects.equals(query.offset, target.offset) )`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] jinxing64 commented on a change in pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
jinxing64 commented on a change in pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#discussion_r361453620
##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1406,6 +1410,48 @@ private JoinOnCalcsToJoinUnifyRule() {
}
}
+ /**
+ * A {@link SubstitutionVisitor.UnifyRule} that matches a {@link MutableSort}
+ * which has {@link MutableCalc} as child to a {@link MutableSort}.
+ * We try to pull up the {@link MutableCalc} to top of {@link MutableSort},
+ * then match the {@link MutableSort} in query to {@link MutableSort} in target.
+ */
+ private static class SortOnCalcToSortUnifyRule extends AbstractUnifyRule {
+
+ public static final SortOnCalcToSortUnifyRule INSTANCE =
+ new SortOnCalcToSortUnifyRule();
+
+ private SortOnCalcToSortUnifyRule() {
+ super(operand(MutableSort.class, operand(MutableCalc.class, query(0))),
+ operand(MutableSort.class, target(0)), 1);
+ }
+
+ @Override protected UnifyResult apply(UnifyRuleCall call) {
+ final MutableSort query = (MutableSort) call.query;
+ final MutableCalc qInput = (MutableCalc) query.getInput();
+ final Pair<RexNode, List<RexNode>> qInputExplained = explainCalc(qInput);
+ final List<RexNode> qInputProjs = qInputExplained.right;
+
+ final MutableSort target = (MutableSort) call.target;
+ if (!Objects.equals(query.fetch, target.fetch)
+ || !Objects.equals(query.offset, target.offset)) {
+ return null;
+ }
+ final Mappings.TargetMapping mapping =
+ Project.getMapping(fieldCnt(qInput.getInput()), qInputProjs);
+ if (mapping == null) {
+ return null;
+ }
+ RelCollation collation = RelCollations.permute(query.collation, mapping.inverse());
Review comment:
final ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] jinxing64 commented on a change in pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
jinxing64 commented on a change in pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#discussion_r361453570
##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1406,6 +1410,48 @@ private JoinOnCalcsToJoinUnifyRule() {
}
}
+ /**
+ * A {@link SubstitutionVisitor.UnifyRule} that matches a {@link MutableSort}
+ * which has {@link MutableCalc} as child to a {@link MutableSort}.
+ * We try to pull up the {@link MutableCalc} to top of {@link MutableSort},
+ * then match the {@link MutableSort} in query to {@link MutableSort} in target.
+ */
+ private static class SortOnCalcToSortUnifyRule extends AbstractUnifyRule {
+
+ public static final SortOnCalcToSortUnifyRule INSTANCE =
+ new SortOnCalcToSortUnifyRule();
+
+ private SortOnCalcToSortUnifyRule() {
+ super(operand(MutableSort.class, operand(MutableCalc.class, query(0))),
+ operand(MutableSort.class, target(0)), 1);
+ }
+
+ @Override protected UnifyResult apply(UnifyRuleCall call) {
+ final MutableSort query = (MutableSort) call.query;
+ final MutableCalc qInput = (MutableCalc) query.getInput();
+ final Pair<RexNode, List<RexNode>> qInputExplained = explainCalc(qInput);
+ final List<RexNode> qInputProjs = qInputExplained.right;
+
+ final MutableSort target = (MutableSort) call.target;
+ if (!Objects.equals(query.fetch, target.fetch)
+ || !Objects.equals(query.offset, target.offset)) {
Review comment:
I would rather to match only when fetch and offset are null.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [calcite] xy2953396112 commented on a change in pull request #1688:
[CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #1688: [CALCITE-3626] Add SortOnCalcToSortUnifyRule in SubstitutionVisitor
URL: https://github.com/apache/calcite/pull/1688#discussion_r361354388
##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1406,6 +1410,48 @@ private JoinOnCalcsToJoinUnifyRule() {
}
}
+ /**
+ * A {@link SubstitutionVisitor.UnifyRule} that matches a {@link MutableSort}
+ * which has {@link MutableCalc} as child to a {@link MutableSort}.
+ * We try to pull up the {@link MutableCalc} to top of {@link MutableSort},
+ * then match the {@link MutableSort} in query to {@link MutableSort} in target.
+ */
+ private static class SortOnCalcToSortUnifyRule extends AbstractUnifyRule {
+
+ public static final SortOnCalcToSortUnifyRule INSTANCE =
+ new SortOnCalcToSortUnifyRule();
+
+ private SortOnCalcToSortUnifyRule() {
+ super(operand(MutableSort.class, operand(MutableCalc.class, query(0))),
+ operand(MutableSort.class, target(0)), 1);
+ }
+
+ @Override protected UnifyResult apply(UnifyRuleCall call) {
+ final MutableSort query = (MutableSort) call.query;
+ final MutableCalc qInput = (MutableCalc) query.getInput();
+ final Pair<RexNode, List<RexNode>> qInputExplained = explainCalc(qInput);
+ final List<RexNode> qInputProjs = qInputExplained.right;
+
+ final MutableSort target = (MutableSort) call.target;
+ if (!Objects.equals(query.fetch, target.fetch)
+ || !Objects.equals(query.offset, target.offset)) {
+ return null;
Review comment:
i think `||` will be better, if `!(Objects.equals(query.fetch, target.fetch)` is true, it will return null, not need to consider second condition.
----------------------------------------------------------------
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
With regards,
Apache Git Services