mihaibudiu commented on code in PR #4366:
URL: https://github.com/apache/calcite/pull/4366#discussion_r2082784825
##########
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##########
@@ -388,9 +388,17 @@ private CoreRules() {}
public static final IntersectToSemiJoinRule INTERSECT_TO_SEMI_JOIN =
IntersectToSemiJoinRule.Config.DEFAULT.toRule();
+ /** Rule that translates a {@link Union} to {@link Filter}. */
+ public static final SetOpToFilterRule UNION_TO_FILTER =
Review Comment:
A better name would have been `UNION_FILTER_TO_FILTER`, but because we
already have a rule for `MINUS`, we should keep this name, which has the same
structure.
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -10515,6 +10515,115 @@ private void
checkLoptOptimizeJoinRule(LoptOptimizeJoinRule rule) {
.check();
}
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6973";>[CALCITE-6973]
+ * Add rule for convert Minus to Filter</a>. */
+ @Test void testMinusToFilterRule3() {
+ final String sql = "SELECT mgr, comm FROM emp WHERE comm = 5\n"
+ + "EXCEPT\n"
+ + "SELECT e1.mgr, e1.comm\n"
+ + "FROM emp e1\n"
+ + "WHERE EXISTS (\n"
+ + " SELECT 1 FROM emp e2\n"
+ + " WHERE e2.comm = e1.comm)";
+ sql(sql)
+ .withPreRule(CoreRules.PROJECT_FILTER_TRANSPOSE)
+ .withRule(CoreRules.MINUS_TO_FILTER)
+ .check();
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7002";>[CALCITE-7002]
+ * Create an optimization rule to eliminate UNION
+ * from the same source with different filters</a>. */
+ @Test void testUnionToFilterRule() {
+ final String sql = "SELECT mgr, comm FROM emp WHERE mgr = 12\n"
+ + "UNION\n"
+ + "SELECT mgr, comm FROM emp WHERE comm = 5\n";
+ sql(sql)
+ .withPreRule(CoreRules.PROJECT_FILTER_TRANSPOSE)
+ .withRule(CoreRules.UNION_TO_FILTER)
+ .check();
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7002";>[CALCITE-7002]
+ * Create an optimization rule to eliminate UNION
+ * from the same source with different filters</a>. */
+ @Test void testUnionToFilterRule2() {
+ final String sql = "SELECT mgr, comm FROM emp\n"
+ + "UNION\n"
+ + "SELECT mgr, comm FROM emp WHERE comm = 5\n";
+ sql(sql)
+ .withPreRule(CoreRules.PROJECT_FILTER_TRANSPOSE)
+ .withRule(CoreRules.UNION_TO_FILTER)
+ .check();
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7002";>[CALCITE-7002]
+ * Create an optimization rule to eliminate UNION
+ * from the same source with different filters</a>. */
+ @Test void testUnionToFilterRule3() {
+ final String sql = "SELECT mgr, comm FROM emp WHERE comm = 5\n"
+ + "UNION\n"
+ + "SELECT e1.mgr, e1.comm\n"
+ + "FROM emp e1\n"
+ + "WHERE EXISTS (\n"
+ + " SELECT 1 FROM emp e2\n"
+ + " WHERE e2.comm = e1.comm)";
+ sql(sql)
+ .withPreRule(CoreRules.PROJECT_FILTER_TRANSPOSE)
+ .withRule(CoreRules.UNION_TO_FILTER)
+ .check();
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7002";>[CALCITE-7002]
+ * Create an optimization rule to eliminate UNION
+ * from the same source with different filters</a>. */
+ @Test void testIntersectToFilterRule() {
+ final String sql = "SELECT mgr, comm FROM emp WHERE mgr = 12\n"
+ + "INTERSECT\n"
+ + "SELECT mgr, comm FROM emp WHERE comm = 5\n";
+ sql(sql)
+ .withPreRule(CoreRules.PROJECT_FILTER_TRANSPOSE)
+ .withRule(CoreRules.INTERSECT_TO_FILTER)
+ .check();
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7002";>[CALCITE-7002]
+ * Create an optimization rule to eliminate UNION
+ * from the same source with different filters</a>. */
+ @Test void testIntersectToFilterRule2() {
+ final String sql = "SELECT mgr, comm FROM emp\n"
+ + "INTERSECT\n"
+ + "SELECT mgr, comm FROM emp WHERE comm = 5\n";
+ sql(sql)
+ .withPreRule(CoreRules.PROJECT_FILTER_TRANSPOSE)
+ .withRule(CoreRules.INTERSECT_TO_FILTER)
+ .check();
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7002";>[CALCITE-7002]
+ * Create an optimization rule to eliminate UNION
+ * from the same source with different filters</a>. */
+ @Test void testIntersectToFilterRule3() {
+ final String sql = "SELECT mgr, comm FROM emp WHERE comm = 5\n"
+ + "INTERSECT\n"
+ + "SELECT e1.mgr, e1.comm\n"
+ + "FROM emp e1\n"
+ + "WHERE EXISTS (\n"
Review Comment:
these correlated queries are more complicated than really necessary to test
this rule.
why are you using them?
##########
core/src/main/java/org/apache/calcite/rel/rules/MinusToFilterRule.java:
##########
@@ -1,121 +0,0 @@
-/*
- * 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.rel.rules;
-
-import org.apache.calcite.plan.RelOptRuleCall;
-import org.apache.calcite.plan.RelRule;
-import org.apache.calcite.rel.RelNode;
-import org.apache.calcite.rel.core.Filter;
-import org.apache.calcite.rel.core.Minus;
-import org.apache.calcite.rex.RexNode;
-import org.apache.calcite.rex.RexUtil;
-import org.apache.calcite.tools.RelBuilder;
-
-import org.immutables.value.Value;
-
-/**
- * Rule that replaces {@link Minus} operator with {@link Filter}
- * when both inputs are from the same source with only filter conditions
differing.
- * Only inspect a single {@link Filter} layer in the inputs of {@link Minus}.
- * For inputs with nested {@link Filter}s, apply {@link CoreRules#FILTER_MERGE}
- * as a preprocessing step.
- *
- * <p>Example transformation:
- * <blockquote><pre>
- * SELECT mgr, comm FROM emp WHERE mgr = 12
- * EXCEPT
- * SELECT mgr, comm FROM emp WHERE comm = 5
- *
- * to
- *
- * SELECT DISTINCT mgr, comm FROM emp
- * WHERE mgr = 12 AND NOT(comm = 5)
- * </pre></blockquote>
- */
[email protected]
-public class MinusToFilterRule
Review Comment:
I don't think you can delete a public class.
Just mark it `@deprecated` and point to the replacement.
I know, this is annoying.
##########
core/src/main/java/org/apache/calcite/rel/rules/SetOpToFilterRule.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.core.SetOp;
+import org.apache.calcite.rel.core.Union;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.tools.RelBuilder;
+
+import org.immutables.value.Value;
+
+/**
+ * Rule that replaces {@link SetOp} operator with {@link Filter}
+ * when both inputs are from the same source with only filter conditions
differing.
+ * Only inspect a single {@link Filter} layer in the inputs of {@link SetOp}.
+ * For inputs with nested {@link Filter}s, apply {@link CoreRules#FILTER_MERGE}
+ * as a preprocessing step.
+ *
+ * <p>Example:
+ *
+ * <p>UNION
+ * <blockquote><pre>
+ * SELECT mgr, comm FROM emp WHERE mgr = 12
+ * UNION
+ * SELECT mgr, comm FROM emp WHERE comm = 5
+ *
+ * to
Review Comment:
is rewritten to
##########
core/src/main/java/org/apache/calcite/rel/rules/SetOpToFilterRule.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.core.SetOp;
+import org.apache.calcite.rel.core.Union;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.tools.RelBuilder;
+
+import org.immutables.value.Value;
+
+/**
+ * Rule that replaces {@link SetOp} operator with {@link Filter}
+ * when both inputs are from the same source with only filter conditions
differing.
+ * Only inspect a single {@link Filter} layer in the inputs of {@link SetOp}.
+ * For inputs with nested {@link Filter}s, apply {@link CoreRules#FILTER_MERGE}
+ * as a preprocessing step.
+ *
+ * <p>Example:
+ *
+ * <p>UNION
+ * <blockquote><pre>
+ * SELECT mgr, comm FROM emp WHERE mgr = 12
+ * UNION
+ * SELECT mgr, comm FROM emp WHERE comm = 5
+ *
+ * to
+ *
+ * SELECT DISTINCT mgr, comm FROM emp
+ * WHERE mgr = 12 OR comm = 5
+ * </pre></blockquote>
+ *
+ * <p>INTERSECT
+ * <blockquote><pre>
+ * SELECT mgr, comm FROM emp WHERE mgr = 12
+ * INTERSECT
+ * SELECT mgr, comm FROM emp WHERE comm = 5
+ *
+ * to
+ *
+ * SELECT DISTINCT mgr, comm FROM emp
+ * WHERE mgr = 12 AND comm = 5
+ * </pre></blockquote>
+ *
+ * <p>EXCEPT
+ * <blockquote><pre>
+ * SELECT mgr, comm FROM emp WHERE mgr = 12
+ * EXCEPT
+ * SELECT mgr, comm FROM emp WHERE comm = 5
+ *
+ * to
+ *
+ * SELECT DISTINCT mgr, comm FROM emp
+ * WHERE mgr = 12 AND NOT(comm = 5)
+ * </pre></blockquote>
+ */
[email protected]
+public class SetOpToFilterRule
+ extends RelRule<SetOpToFilterRule.Config>
+ implements TransformationRule {
+
+ /** Creates an SetOpToFilterRule. */
+ protected SetOpToFilterRule(Config config) {
+ super(config);
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ config.matchHandler().accept(this, call);
+ }
+
+ private void match(RelOptRuleCall call) {
+ final SetOp setOp = call.rel(0);
+ if (setOp.all || setOp.getInputs().size() != 2) {
+ return;
+ }
+ final RelBuilder builder = call.builder();
+ final RelNode leftInput = call.rel(1);
+ final Filter rightInput = call.rel(2);
+
+ if (!RexUtil.isDeterministic(rightInput.getCondition())) {
+ return;
+ }
+
+ RelNode leftBase;
+ RexNode leftCond = null;
+ if (leftInput instanceof Filter) {
+ Filter leftFilter = (Filter) leftInput;
+ leftBase = leftFilter.getInput().stripped();
+ leftCond = leftFilter.getCondition();
+ } else {
+ leftBase = leftInput.stripped();
+ }
+
+ final RelNode rightBase = rightInput.getInput().stripped();
+ if (!leftBase.equals(rightBase)) {
+ return;
+ }
+
+ RexNode finalCond = null;
+ // Right input is Filter, right cond should be not null
+ if (setOp instanceof Union) {
+ finalCond = leftCond != null
+ ? builder.or(leftCond, rightInput.getCondition())
+ : builder.literal(true);
+ } else if (setOp instanceof Intersect) {
+ finalCond = leftCond != null
+ ? builder.and(leftCond, rightInput.getCondition())
+ : rightInput.getCondition();
+ } else if (setOp instanceof Minus) {
+ finalCond = leftCond != null
+ ? builder.and(leftCond, builder.not(rightInput.getCondition()))
+ : builder.not(rightInput.getCondition());
+ } else {
+ // Should not enter here
Review Comment:
throw some exception that says "unreachable".
maybe there are other examples you can copy.
##########
core/src/main/java/org/apache/calcite/rel/rules/SetOpToFilterRule.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.core.SetOp;
+import org.apache.calcite.rel.core.Union;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.tools.RelBuilder;
+
+import org.immutables.value.Value;
+
+/**
+ * Rule that replaces {@link SetOp} operator with {@link Filter}
+ * when both inputs are from the same source with only filter conditions
differing.
+ * Only inspect a single {@link Filter} layer in the inputs of {@link SetOp}.
Review Comment:
I don't know what "inspect" means here, but I think this phrase is not
necessary.
##########
core/src/main/java/org/apache/calcite/rel/rules/SetOpToFilterRule.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.core.SetOp;
+import org.apache.calcite.rel.core.Union;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.tools.RelBuilder;
+
+import org.immutables.value.Value;
+
+/**
+ * Rule that replaces {@link SetOp} operator with {@link Filter}
+ * when both inputs are from the same source with only filter conditions
differing.
+ * Only inspect a single {@link Filter} layer in the inputs of {@link SetOp}.
+ * For inputs with nested {@link Filter}s, apply {@link CoreRules#FILTER_MERGE}
Review Comment:
who should apply?
If this rule applies, say "this rule applies"
if the user is supposed to apply, say "for nested filters, the rule ...
should be used prior to invoking this one".
##########
core/src/main/java/org/apache/calcite/rel/rules/SetOpToFilterRule.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.core.SetOp;
+import org.apache.calcite.rel.core.Union;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.tools.RelBuilder;
+
+import org.immutables.value.Value;
+
+/**
+ * Rule that replaces {@link SetOp} operator with {@link Filter}
+ * when both inputs are from the same source with only filter conditions
differing.
+ * Only inspect a single {@link Filter} layer in the inputs of {@link SetOp}.
+ * For inputs with nested {@link Filter}s, apply {@link CoreRules#FILTER_MERGE}
+ * as a preprocessing step.
+ *
+ * <p>Example:
+ *
+ * <p>UNION
+ * <blockquote><pre>
+ * SELECT mgr, comm FROM emp WHERE mgr = 12
+ * UNION
+ * SELECT mgr, comm FROM emp WHERE comm = 5
+ *
+ * to
+ *
+ * SELECT DISTINCT mgr, comm FROM emp
+ * WHERE mgr = 12 OR comm = 5
+ * </pre></blockquote>
+ *
+ * <p>INTERSECT
+ * <blockquote><pre>
+ * SELECT mgr, comm FROM emp WHERE mgr = 12
+ * INTERSECT
+ * SELECT mgr, comm FROM emp WHERE comm = 5
+ *
+ * to
+ *
+ * SELECT DISTINCT mgr, comm FROM emp
+ * WHERE mgr = 12 AND comm = 5
+ * </pre></blockquote>
+ *
+ * <p>EXCEPT
+ * <blockquote><pre>
+ * SELECT mgr, comm FROM emp WHERE mgr = 12
+ * EXCEPT
+ * SELECT mgr, comm FROM emp WHERE comm = 5
+ *
+ * to
+ *
+ * SELECT DISTINCT mgr, comm FROM emp
+ * WHERE mgr = 12 AND NOT(comm = 5)
+ * </pre></blockquote>
+ */
[email protected]
+public class SetOpToFilterRule
+ extends RelRule<SetOpToFilterRule.Config>
+ implements TransformationRule {
+
+ /** Creates an SetOpToFilterRule. */
+ protected SetOpToFilterRule(Config config) {
+ super(config);
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ config.matchHandler().accept(this, call);
+ }
+
+ private void match(RelOptRuleCall call) {
+ final SetOp setOp = call.rel(0);
+ if (setOp.all || setOp.getInputs().size() != 2) {
Review Comment:
I think this would work with more than 2 as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
