You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "xiedeyantu (via GitHub)" <gi...@apache.org> on 2025/02/23 08:22:44 UTC
[PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
xiedeyantu opened a new pull request, #4209:
URL: https://github.com/apache/calcite/pull/4209
(no comment)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1970838781
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,46 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRuleOneField() {
Review Comment:
> since plans are hard to read, I would encourage you to also add some quidem tests, these are much simpler to read and write
@mihaibudiu Could you give me a description of quidem tests? I don't quite understand how to write them now.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2693108517
> @NobiGo I have added a test case for SQL to Rel and Rel to SQL. Can this verify the conclusion? As for the execution result of this SQL, it depends on various execution engines.
I need more time to review this PR.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1970841776
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,46 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRuleOneField() {
Review Comment:
Look at the files with iq suffix
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1967589996
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,12 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRule() {
Review Comment:
Thanks for your reply, I will add jira link and more tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1970819950
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,46 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRuleOneField() {
Review Comment:
since plans are hard to read, I would encourage you to also add some quidem tests, these are much simpler to read and write
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1970846825
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,46 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRuleOneField() {
Review Comment:
> Look at the files with iq suffix
@mihaibudiu Thanks, I have seen the file with this suffix. Do I need to write a SQL test to output the final result through the optimization rules? Is there a similar example for reference?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2685253844
@asolimando @suibianwanwank @NobiGo I have fixed, please review it.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1967528892
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,12 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRule() {
Review Comment:
1. Please add the Jira link
2. Add more test cases about
- INTERSECT ALL
- INTERSECT on multiple fields
- Multiple INTERSECT
3. We may not adopt this rule as the default one for now, but it can be set as the default rule to test whether the rule function is normal.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1971322604
##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -5440,6 +5440,143 @@ LogicalIntersect(all=[true])
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
LogicalFilter(condition=[=($7, 30)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testIntersectToExistsRuleMultiFields">
+ <Resource name="sql">
+ <![CDATA[SELECT a.ename, a.job FROM emp AS a
+INTERSECT
+SELECT b.ename, b.job FROM emp AS b]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalIntersect(all=[false])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalAggregate(group=[{0, 1}])
Review Comment:
@suibianwanwank This is indeed a good question. Many database implementations are as you said, so do we need to modify the logic?
`SELECT a.CustID FROM tbl1 AS a
INTERSECT
SELECT b.CustID FROM tbl2 AS b `
The Exists SQL change to
`SELECT DISTINCT a.CustID FROM tbl1 AS a
WHERE EXISTS (SELECT b.CustID FROM tbl2 b WHERE (a.CustID=b.CustID) or (a.CustID is NULL and b.CustID is NULL))`
@NobiGo Could you help me determine whether this logic is correct?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2683809789
> Actually I think I am wrong, there is no way right now to use a specific optimization in a quidem file. That ticket is still open. So you can for now forget about my request.
It doesn't matter, I can just learn this testing method, which may be covered later.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1971676801
##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -5440,6 +5440,143 @@ LogicalIntersect(all=[true])
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
LogicalFilter(condition=[=($7, 30)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testIntersectToExistsRuleMultiFields">
+ <Resource name="sql">
+ <![CDATA[SELECT a.ename, a.job FROM emp AS a
+INTERSECT
+SELECT b.ename, b.job FROM emp AS b]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalIntersect(all=[false])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalAggregate(group=[{0, 1}])
Review Comment:
> IS NOT DISTINCT FROM
@asolimando Great! I will chang my rule to fit your suggestion.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1981503517
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.plan.hep.HepRelVertex;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ RelDataType rowType = intersect.getRowType();
+ List<RelNode> inputs = intersect.getInputs();
+ RelNode current = inputs.get(0);
+
+ // get all column indices of intersect
+ ImmutableBitSet fieldIndices = ImmutableBitSet.of(rowType.getFieldList()
+ .stream().map(RelDataTypeField::getIndex)
+ .collect(Collectors.toList()));
+
+ // iterate over the inputs and apply exists subquery
+ for (int i = 1; i < inputs.size(); i++) {
+ RelNode nextInput = removeHepRelVertex(inputs.get(i));
+
+ // create correlation
+ CorrelationId correlationId = intersect.getCluster().createCorrel();
+ RexNode correl =
+ rexBuilder.makeCorrel(rowType, correlationId);
+
+ // create condition in exists filter, and use correlation
+ List<RexNode> conditions = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode outerField = rexBuilder.makeInputRef(rowType, fieldIndex);
+ RexNode innerField = rexBuilder.makeFieldAccess(correl, fieldIndex);
+ conditions.add(
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+ outerField, innerField));
+ }
+ RexNode condition = RexUtil.composeConjunction(rexBuilder, conditions);
+
+ // build exists subquery
+ RelNode existsSubQuery = builder.push(nextInput)
+ .filter(condition)
+ .project(builder.fields(fieldIndices))
+ .build();
+
+ // apply exists subquery to the current relation
+ current = builder.push(current)
+ .filter(ImmutableSet.of(correlationId),
+ RexSubQuery.exists(existsSubQuery))
+ .build();
+ }
+
+ builder.push(current);
+ List<RexNode> projects = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode rexNode = builder.fields().get(fieldIndex);
+ RelDataType originalType =
+ rowType.getFieldList().get(projects.size()).getType();
+ RexNode expr;
+ if (!originalType.equals(rexNode.getType())) {
+ expr = rexBuilder.makeCast(originalType, rexNode, true, false);
+ } else {
+ expr = rexNode;
+ }
+ projects.add(expr);
+ }
+
+ RelNode result = builder.project(projects)
+ .distinct()
+ .build();
+
+ call.transformTo(result);
+ }
+
+ /**
+ * Will not eliminate HepRelVertex in subqueries
+ * during the optimization process.
+ * We need to eliminate it here. */
+ private RelNode removeHepRelVertex(RelNode input) {
Review Comment:
> You may need to stipped().
Yes, thanks.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2699498348
@xiedeyantu This PR cannot be added with test cases in JdbcTest due to hidden issues. I have been dealing with them and we can merge this PR in the next version.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "suibianwanwank (via GitHub)" <gi...@apache.org>.
suibianwanwank commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1981312662
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.plan.hep.HepRelVertex;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ RelDataType rowType = intersect.getRowType();
+ List<RelNode> inputs = intersect.getInputs();
+ RelNode current = inputs.get(0);
+
+ // get all column indices of intersect
+ ImmutableBitSet fieldIndices = ImmutableBitSet.of(rowType.getFieldList()
+ .stream().map(RelDataTypeField::getIndex)
+ .collect(Collectors.toList()));
+
+ // iterate over the inputs and apply exists subquery
+ for (int i = 1; i < inputs.size(); i++) {
+ RelNode nextInput = removeHepRelVertex(inputs.get(i));
+
+ // create correlation
+ CorrelationId correlationId = intersect.getCluster().createCorrel();
+ RexNode correl =
+ rexBuilder.makeCorrel(rowType, correlationId);
+
+ // create condition in exists filter, and use correlation
+ List<RexNode> conditions = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode outerField = rexBuilder.makeInputRef(rowType, fieldIndex);
+ RexNode innerField = rexBuilder.makeFieldAccess(correl, fieldIndex);
+ conditions.add(
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+ outerField, innerField));
+ }
+ RexNode condition = RexUtil.composeConjunction(rexBuilder, conditions);
+
+ // build exists subquery
+ RelNode existsSubQuery = builder.push(nextInput)
+ .filter(condition)
+ .project(builder.fields(fieldIndices))
+ .build();
+
+ // apply exists subquery to the current relation
+ current = builder.push(current)
+ .filter(ImmutableSet.of(correlationId),
+ RexSubQuery.exists(existsSubQuery))
+ .build();
+ }
+
+ builder.push(current);
+ List<RexNode> projects = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode rexNode = builder.fields().get(fieldIndex);
+ RelDataType originalType =
+ rowType.getFieldList().get(projects.size()).getType();
+ RexNode expr;
+ if (!originalType.equals(rexNode.getType())) {
+ expr = rexBuilder.makeCast(originalType, rexNode, true, false);
+ } else {
+ expr = rexNode;
+ }
+ projects.add(expr);
+ }
+
+ RelNode result = builder.project(projects)
+ .distinct()
+ .build();
+
+ call.transformTo(result);
+ }
+
+ /**
+ * Will not eliminate HepRelVertex in subqueries
+ * during the optimization process.
+ * We need to eliminate it here. */
+ private RelNode removeHepRelVertex(RelNode input) {
Review Comment:
You may need to stipped().
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "suibianwanwank (via GitHub)" <gi...@apache.org>.
suibianwanwank commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1971809477
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.RelOptUtil;
+import org.apache.calcite.plan.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ // get all column indices of intersect
+ List<Integer> fieldIndices = intersect.getRowType().getFieldList()
Review Comment:
Nit: Here you can use an ImmutableBitSet to store.
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,46 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRuleOneField() {
+ String sql = "SELECT a.ename FROM emp AS a\n"
+ + "INTERSECT\n"
+ + "SELECT b.name FROM dept AS b";
+ sql(sql).withRule(CoreRules.INTERSECT_TO_EXISTS)
+ .check();
+ }
+
+ @Test void testIntersectToExistsRulePrimaryKey() {
Review Comment:
The test should have a jira link added to it, so that people who see this test in the future can more easily understand the purpose of this test.
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.RelOptUtil;
+import org.apache.calcite.plan.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ // get all column indices of intersect
+ List<Integer> fieldIndices = intersect.getRowType().getFieldList()
+ .stream().map(RelDataTypeField::getIndex)
+ .collect(Collectors.toList());
+
+ List<RelNode> inputs = intersect.getInputs();
+ RelNode current = inputs.get(0);
+
+ // iterate over the inputs and apply exists subquery
+ for (int i = 1; i < inputs.size(); i++) {
+ RelNode nextInput = inputs.get(i);
+
+ // create correlation
+ CorrelationId correlationId = intersect.getCluster().createCorrel();
+ RexNode correl =
+ rexBuilder.makeCorrel(nextInput.getRowType(), correlationId);
+
+ // create condition in exists filter, and use correlation
+ List<RexNode> conditions = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode outerField = rexBuilder.makeInputRef(current, fieldIndex);
+ RexNode innerField =
+ rexBuilder.makeFieldAccess(correl, fieldIndex);
+ conditions.add(
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+ outerField, innerField));
+ }
+ RexNode condition = RexUtil.composeConjunction(rexBuilder, conditions);
+
+ // build exists subquery
+ RelNode existsSubQuery = builder.push(nextInput)
+ .filter(condition)
+ .project(builder.fields(fieldIndices))
+ .build();
+
+ // apply exists subquery to the current relation
+ current = builder.push(current)
+ .filter(ImmutableSet.of(correlationId),
+ RexSubQuery.exists(existsSubQuery))
+ .build();
+ }
+ System.out.println(RelOptUtil.toString(current));
Review Comment:
The output during debugging should be deleted, as should the following.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2687379085
> The issue raised by @suibianwanwank is a very good test case. We should test the accuracy of this logic in the JdbcTest.
@NobiGo Sorry, I tried many ways and couldn't find a good jdbc test method. I found that when executing the test, the plan should not include subquery, which will cause this error
```
public RexLocalRef registerInput(RexNode expr) {
final RexShuttle shuttle = new RegisterInputShuttle(true);
final RexNode ref = expr.accept(shuttle);
return (RexLocalRef) ref; <------ ref is Subquery, can not cast to RexLocalRef
}
```
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2692274634
@NobiGo I have added a test case for SQL to Rel and Rel to SQL. Can this verify the conclusion? As for the execution result of this SQL, it depends on various execution engines.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1981485392
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -244,6 +245,33 @@ private static String toSql(RelNode root, SqlDialect dialect,
sql(query).withMysql().ok(expected);
}
+ /**
+ * Test case of
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-6836">[CALCITE-6836]
+ * Add Rule to convert INTERSECT to EXISTS</a>. */
+ @Test void testIntersectToExistsRule() {
+ String query = "SELECT \"product_name\"\n"
Review Comment:
> In fact, I don't think it's necessary to test in RelToSqlConverter because the semantics of relational algebra is already expressed in RelNode. What is tested here is the correctness of RelNode -> sqlnode -> sqlText.
@suibianwanwank Thanks, I agree with you, but currently Calcite may not have the ability to execute this plan, and I have no other good options.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "suibianwanwank (via GitHub)" <gi...@apache.org>.
suibianwanwank commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1981311531
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.plan.hep.HepRelVertex;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ RelDataType rowType = intersect.getRowType();
+ List<RelNode> inputs = intersect.getInputs();
+ RelNode current = inputs.get(0);
+
+ // get all column indices of intersect
+ ImmutableBitSet fieldIndices = ImmutableBitSet.of(rowType.getFieldList()
+ .stream().map(RelDataTypeField::getIndex)
+ .collect(Collectors.toList()));
+
+ // iterate over the inputs and apply exists subquery
+ for (int i = 1; i < inputs.size(); i++) {
+ RelNode nextInput = removeHepRelVertex(inputs.get(i));
+
+ // create correlation
+ CorrelationId correlationId = intersect.getCluster().createCorrel();
+ RexNode correl =
+ rexBuilder.makeCorrel(rowType, correlationId);
+
+ // create condition in exists filter, and use correlation
+ List<RexNode> conditions = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode outerField = rexBuilder.makeInputRef(rowType, fieldIndex);
+ RexNode innerField = rexBuilder.makeFieldAccess(correl, fieldIndex);
+ conditions.add(
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+ outerField, innerField));
+ }
+ RexNode condition = RexUtil.composeConjunction(rexBuilder, conditions);
+
+ // build exists subquery
+ RelNode existsSubQuery = builder.push(nextInput)
+ .filter(condition)
+ .project(builder.fields(fieldIndices))
+ .build();
+
+ // apply exists subquery to the current relation
+ current = builder.push(current)
+ .filter(ImmutableSet.of(correlationId),
+ RexSubQuery.exists(existsSubQuery))
+ .build();
+ }
+
+ builder.push(current);
+ List<RexNode> projects = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode rexNode = builder.fields().get(fieldIndex);
+ RelDataType originalType =
+ rowType.getFieldList().get(projects.size()).getType();
+ RexNode expr;
+ if (!originalType.equals(rexNode.getType())) {
Review Comment:
What situation is the type adjustment for here?
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -244,6 +245,33 @@ private static String toSql(RelNode root, SqlDialect dialect,
sql(query).withMysql().ok(expected);
}
+ /**
+ * Test case of
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-6836">[CALCITE-6836]
+ * Add Rule to convert INTERSECT to EXISTS</a>. */
+ @Test void testIntersectToExistsRule() {
+ String query = "SELECT \"product_name\"\n"
Review Comment:
In fact, I don't think it's necessary to test in RelToSqlConverter because the semantics of relational algebra is already expressed in RelNode. What is tested here is the correctness of RelNode -> sqlnode -> sqlText.
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9716,4 +9716,50 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
fixture().withRelBuilderConfig(a -> a.withBloat(-1))
.relFn(relFn).withPlanner(planner).check();
}
+
+ /**
+ * Test case of
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-6836">[CALCITE-6836]
+ * Add Rule to convert INTERSECT to EXISTS</a>. */
+ @Test void testIntersectToExistsRuleOneField() {
+ String sql = "SELECT a.ename FROM emp AS a\n"
+ + "INTERSECT\n"
+ + "SELECT b.name FROM dept AS b";
+ sql(sql).withRule(CoreRules.INTERSECT_TO_EXISTS)
+ .check();
+ }
+
+ @Test void testIntersectToExistsRulePrimaryKey() {
+ String sql = "SELECT a.empno FROM emp AS a\n"
+ + "INTERSECT\n"
+ + "SELECT b.empno FROM emp AS b";
+ sql(sql).withRule(CoreRules.INTERSECT_TO_EXISTS)
+ .check();
+ }
+
Review Comment:
For all remaining tests, also add the jira case
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.plan.hep.HepRelVertex;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ RelDataType rowType = intersect.getRowType();
+ List<RelNode> inputs = intersect.getInputs();
+ RelNode current = inputs.get(0);
+
+ // get all column indices of intersect
+ ImmutableBitSet fieldIndices = ImmutableBitSet.of(rowType.getFieldList()
+ .stream().map(RelDataTypeField::getIndex)
+ .collect(Collectors.toList()));
+
+ // iterate over the inputs and apply exists subquery
+ for (int i = 1; i < inputs.size(); i++) {
+ RelNode nextInput = removeHepRelVertex(inputs.get(i));
+
+ // create correlation
+ CorrelationId correlationId = intersect.getCluster().createCorrel();
+ RexNode correl =
+ rexBuilder.makeCorrel(rowType, correlationId);
+
+ // create condition in exists filter, and use correlation
+ List<RexNode> conditions = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode outerField = rexBuilder.makeInputRef(rowType, fieldIndex);
+ RexNode innerField = rexBuilder.makeFieldAccess(correl, fieldIndex);
+ conditions.add(
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+ outerField, innerField));
+ }
+ RexNode condition = RexUtil.composeConjunction(rexBuilder, conditions);
+
+ // build exists subquery
+ RelNode existsSubQuery = builder.push(nextInput)
+ .filter(condition)
+ .project(builder.fields(fieldIndices))
+ .build();
+
+ // apply exists subquery to the current relation
+ current = builder.push(current)
+ .filter(ImmutableSet.of(correlationId),
+ RexSubQuery.exists(existsSubQuery))
+ .build();
+ }
+
+ builder.push(current);
+ List<RexNode> projects = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode rexNode = builder.fields().get(fieldIndex);
+ RelDataType originalType =
+ rowType.getFieldList().get(projects.size()).getType();
+ RexNode expr;
+ if (!originalType.equals(rexNode.getType())) {
+ expr = rexBuilder.makeCast(originalType, rexNode, true, false);
+ } else {
+ expr = rexNode;
+ }
+ projects.add(expr);
+ }
+
+ RelNode result = builder.project(projects)
+ .distinct()
+ .build();
+
+ call.transformTo(result);
+ }
+
+ /**
+ * Will not eliminate HepRelVertex in subqueries
+ * during the optimization process.
+ * We need to eliminate it here. */
+ private RelNode removeHepRelVertex(RelNode input) {
Review Comment:
You can learn about stripped().
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo merged PR #4209:
URL: https://github.com/apache/calcite/pull/4209
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2688337810
> ```
> @Test void testIntersect() {
> final String sql = ""
> + "select \"empid\", \"name\" from \"hr\".\"emps\" where \"deptno\"=10\n"
> + "intersect\n"
> + "select \"empid\", \"name\" from \"hr\".\"emps\" where \"empid\">=150";
> CalciteAssert.hr()
> .query(sql)
> .withHook(Hook.PLANNER, (Consumer<RelOptPlanner>)
> planner -> {
> planner.removeRule(CoreRules.INTERSECT_TO_DISTINCT);
> planner.removeRule(EnumerableRules.ENUMERABLE_INTERSECT_RULE);
> planner.addRule(CoreRules.INTERSECT_TO_EXISTS);
> })
> .explainContains("")
> .returnsUnordered("empid=150; name=Sebastian");
> }
> ```
>
> You can try this unit test in JdbcTest. Now it throws exception.
@NobiGo I have debugged the code and modified some of the logic, but there are still errors. as
```
cannot translate expression EXISTS({
LogicalFilter(condition=[AND(IS NOT DISTINCT FROM($0, $cor0.empid), IS NOT DISTINCT FROM($1, $cor0.name))])
LogicalFilter(subset=[rel#282:RelSubset#5.NONE.[]], condition=[>=($0, 150)])
LogicalProject(subset=[rel#280:RelSubset#4.NONE.[]], empid=[$0], name=[$2])
LogicalTableScan(subset=[rel#272:RelSubset#0.NONE.[]], table=[[hr, emps]])
})
```
the stack is
```
0 = {StackTraceElement@7965} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitSubQuery(RexToLixTranslator.java:1701)"
1 = {StackTraceElement@7966} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitSubQuery(RexToLixTranslator.java:108)"
2 = {StackTraceElement@7967} "org.apache.calcite.rex.RexSubQuery.accept(RexSubQuery.java:162)"
3 = {StackTraceElement@7968} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.implementCallOperand(RexToLixTranslator.java:1439)"
4 = {StackTraceElement@7969} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitCall(RexToLixTranslator.java:1426)"
5 = {StackTraceElement@7970} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitCall(RexToLixTranslator.java:108)"
6 = {StackTraceElement@7971} "org.apache.calcite.rex.RexCall.accept(RexCall.java:208)"
7 = {StackTraceElement@7972} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitLocalRef(RexToLixTranslator.java:1306)"
8 = {StackTraceElement@7973} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.visitLocalRef(RexToLixTranslator.java:108)"
9 = {StackTraceElement@7974} "org.apache.calcite.rex.RexLocalRef.accept(RexLocalRef.java:78)"
10 = {StackTraceElement@7975} "org.apache.calcite.adapter.enumerable.RexToLixTranslator.translate(RexToLixTranslator.java:260)"
```
it seems not support exectue subquery, as I mentioned before. Are there any other solutions？
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1981487187
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.plan.hep.HepRelVertex;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ RelDataType rowType = intersect.getRowType();
+ List<RelNode> inputs = intersect.getInputs();
+ RelNode current = inputs.get(0);
+
+ // get all column indices of intersect
+ ImmutableBitSet fieldIndices = ImmutableBitSet.of(rowType.getFieldList()
+ .stream().map(RelDataTypeField::getIndex)
+ .collect(Collectors.toList()));
+
+ // iterate over the inputs and apply exists subquery
+ for (int i = 1; i < inputs.size(); i++) {
+ RelNode nextInput = removeHepRelVertex(inputs.get(i));
+
+ // create correlation
+ CorrelationId correlationId = intersect.getCluster().createCorrel();
+ RexNode correl =
+ rexBuilder.makeCorrel(rowType, correlationId);
+
+ // create condition in exists filter, and use correlation
+ List<RexNode> conditions = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode outerField = rexBuilder.makeInputRef(rowType, fieldIndex);
+ RexNode innerField = rexBuilder.makeFieldAccess(correl, fieldIndex);
+ conditions.add(
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+ outerField, innerField));
+ }
+ RexNode condition = RexUtil.composeConjunction(rexBuilder, conditions);
+
+ // build exists subquery
+ RelNode existsSubQuery = builder.push(nextInput)
+ .filter(condition)
+ .project(builder.fields(fieldIndices))
+ .build();
+
+ // apply exists subquery to the current relation
+ current = builder.push(current)
+ .filter(ImmutableSet.of(correlationId),
+ RexSubQuery.exists(existsSubQuery))
+ .build();
+ }
+
+ builder.push(current);
+ List<RexNode> projects = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode rexNode = builder.fields().get(fieldIndex);
+ RelDataType originalType =
+ rowType.getFieldList().get(projects.size()).getType();
+ RexNode expr;
+ if (!originalType.equals(rexNode.getType())) {
Review Comment:
> What situation is the type adjustment for here?
Here we can get java type and sql type, when we using setop.
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToExistsRule.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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.RelOptUtil.Exists;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.plan.hep.HepRelVertex;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.logical.LogicalIntersect;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import com.google.common.collect.ImmutableSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Planner rule that translates a {@link Intersect}
+ * (<code>all</code> = <code>false</code>)
+ * into a {@link Exists}.
+ *
+ * @see CoreRules#INTERSECT_TO_EXISTS
+ */
+@Value.Enclosing
+public class IntersectToExistsRule
+ extends RelRule<IntersectToExistsRule.Config>
+ implements TransformationRule {
+
+ /** Creates an IntersectToExistRule. */
+ protected IntersectToExistsRule(Config config) {
+ super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public IntersectToExistsRule(Class<? extends Intersect> intersectClass,
+ RelBuilderFactory relBuilderFactory) {
+ this(Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+ .as(Config.class)
+ .withOperandFor(intersectClass));
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public void onMatch(RelOptRuleCall call) {
+ final Intersect intersect = call.rel(0);
+ if (intersect.all) {
+ return; // nothing we can do
+ }
+
+ final RelBuilder builder = call.builder();
+ final RexBuilder rexBuilder = builder.getRexBuilder();
+
+ RelDataType rowType = intersect.getRowType();
+ List<RelNode> inputs = intersect.getInputs();
+ RelNode current = inputs.get(0);
+
+ // get all column indices of intersect
+ ImmutableBitSet fieldIndices = ImmutableBitSet.of(rowType.getFieldList()
+ .stream().map(RelDataTypeField::getIndex)
+ .collect(Collectors.toList()));
+
+ // iterate over the inputs and apply exists subquery
+ for (int i = 1; i < inputs.size(); i++) {
+ RelNode nextInput = removeHepRelVertex(inputs.get(i));
+
+ // create correlation
+ CorrelationId correlationId = intersect.getCluster().createCorrel();
+ RexNode correl =
+ rexBuilder.makeCorrel(rowType, correlationId);
+
+ // create condition in exists filter, and use correlation
+ List<RexNode> conditions = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode outerField = rexBuilder.makeInputRef(rowType, fieldIndex);
+ RexNode innerField = rexBuilder.makeFieldAccess(correl, fieldIndex);
+ conditions.add(
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
+ outerField, innerField));
+ }
+ RexNode condition = RexUtil.composeConjunction(rexBuilder, conditions);
+
+ // build exists subquery
+ RelNode existsSubQuery = builder.push(nextInput)
+ .filter(condition)
+ .project(builder.fields(fieldIndices))
+ .build();
+
+ // apply exists subquery to the current relation
+ current = builder.push(current)
+ .filter(ImmutableSet.of(correlationId),
+ RexSubQuery.exists(existsSubQuery))
+ .build();
+ }
+
+ builder.push(current);
+ List<RexNode> projects = new ArrayList<>();
+ for (int fieldIndex : fieldIndices) {
+ RexNode rexNode = builder.fields().get(fieldIndex);
+ RelDataType originalType =
+ rowType.getFieldList().get(projects.size()).getType();
+ RexNode expr;
+ if (!originalType.equals(rexNode.getType())) {
Review Comment:
> What situation is the type adjustment for here?
Here we can get java type and sql type, when we using setop.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2701327599
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4209) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4209&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4209&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4209&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [91.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4209&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4209&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4209)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2681653583
> According to the description provided by Jira, we need to add a Distinct at the top level of the Project, and perhaps we can also add test cases to JdbcTest to ensure that our transformation implementation is correct. The SQL example:
>
> ```
> select * from (values (1, 2), (1, 2), (3, 4), (5, 7)) as t(a, b)
> intersect
> select * from (values (1, 2), (3, 4), (5, 6)) as t(a, b) ;
> ```
>
> You will know why we need the Distinct.
@NobiGo I know the cause of the problem. I considered distinct in the rule, but the field I chose was too special. It was the primary key of the table, so agg was automatically removed. I modified the test case again, making all the current test cases use non-primary keys, and then adding a test case using the primary key field.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2677662859
Hi, @NobiGo please review thanks !
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "asolimando (via GitHub)" <gi...@apache.org>.
asolimando commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1971650453
##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -5440,6 +5440,143 @@ LogicalIntersect(all=[true])
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
LogicalFilter(condition=[=($7, 30)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testIntersectToExistsRuleMultiFields">
+ <Resource name="sql">
+ <![CDATA[SELECT a.ename, a.job FROM emp AS a
+INTERSECT
+SELECT b.ename, b.job FROM emp AS b]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalIntersect(all=[false])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalAggregate(group=[{0, 1}])
Review Comment:
Haven't read the paper either, but the specific rewrite you propose LGTM, even in the case @suibianwanwank proposed.
You can even opt for a more concise (equivalent) rewrite:
```
SELECT DISTINCT a.CustID FROM tbl1 AS a WHERE EXISTS (SELECT b.CustID FROM tbl2 b WHERE a.CustID IS NOT DISTINCT FROM b.CustID)
```
I have played a bit with it here to double check my understanding: https://www.db-fiddle.com/#&togetherjs=NSeyJDgqqy
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "suibianwanwank (via GitHub)" <gi...@apache.org>.
suibianwanwank commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1971663879
##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -5440,6 +5440,143 @@ LogicalIntersect(all=[true])
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
LogicalFilter(condition=[=($7, 30)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testIntersectToExistsRuleMultiFields">
+ <Resource name="sql">
+ <![CDATA[SELECT a.ename, a.job FROM emp AS a
+INTERSECT
+SELECT b.ename, b.job FROM emp AS b]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalIntersect(all=[false])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalAggregate(group=[{0, 1}])
Review Comment:
That looks great!
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2688432898
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4209) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4209&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4209&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4209&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [91.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4209&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4209&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4209)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2699637974
> @xiedeyantu This PR cannot be added with test cases in JdbcTest due to hidden issues. I have been dealing with them and we can merge this PR in the next version.
@NobiGo Got it, thanks!
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2681139792
> According to the description provided by Jira, we need to add a Distinct at the top level of the Project, and perhaps we can also add test cases to JdbcTest to ensure that our transformation implementation is correct. The SQL example:
>
> ```
> select * from (values (1, 2), (1, 2), (3, 4), (5, 7)) as t(a, b)
> intersect
> select * from (values (1, 2), (3, 4), (5, 6)) as t(a, b) ;
> ```
>
> You will know why we need the Distinct.
@NobiGo Thanks, I got it, I will update it.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2683805892
Actually I think I am wrong, there is no way right now to use a specific optimization in a quidem file. That ticket is still open. So you can for now forget about my request.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1972649324
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -9585,4 +9585,46 @@ private void checkJoinAssociateRuleWithTopAlwaysTrueCondition(boolean allowAlway
.withRule(CoreRules.MULTI_JOIN_OPTIMIZE)
.check();
}
+
+ @Test void testIntersectToExistsRuleOneField() {
Review Comment:
Just as @mihaibudiu said, currently this quidem file cannot specify specific optimizations. Before Quidem's test, we wrote such test cases in JdbcTest. Please feel free to check it.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "suibianwanwank (via GitHub)" <gi...@apache.org>.
suibianwanwank commented on code in PR #4209:
URL: https://github.com/apache/calcite/pull/4209#discussion_r1971217293
##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -5440,6 +5440,143 @@ LogicalIntersect(all=[true])
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
LogicalFilter(condition=[=($7, 30)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testIntersectToExistsRuleMultiFields">
+ <Resource name="sql">
+ <![CDATA[SELECT a.ename, a.job FROM emp AS a
+INTERSECT
+SELECT b.ename, b.job FROM emp AS b]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalIntersect(all=[false])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(ENAME=[$1], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalAggregate(group=[{0, 1}])
Review Comment:
I have not read this paper for the time being. Is it necessary to consider the case of null values here. For example, if the emp table has only one null row. in intersect it should return null, in exsits it will return 0 rows of data.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6836] Add Rule to convert INTERSECT to EXISTS [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4209:
URL: https://github.com/apache/calcite/pull/4209#issuecomment-2697858563
@NobiGo @suibianwanwank @mihaibudiu Hi all, Do we still need this PR？
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org