zabetak commented on a change in pull request #2623:
URL: https://github.com/apache/calcite/pull/2623#discussion_r813790518
##########
File path: core/src/main/java/org/apache/calcite/rel/core/Project.java
##########
@@ -297,6 +321,10 @@ private static int countTrivial(List<RexNode> refs) {
return pw;
}
+ @Override public Set<CorrelationId> getVariablesSet() {
Review comment:
I think the javadoc of the `RelNode` interface is obsolete with respect
to what operators are setting variables. Can we fix that as well as part of
this change. Simply removing the mention that only correlate sets variables may
suffice.
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -905,11 +905,11 @@ public static RelNode createCastRel(
if (rename) {
// Use names and types from castRowType.
return projectFactory.createProject(input, hints, castExps,
- castRowType.getFieldNames());
+ castRowType.getFieldNames(), ImmutableSet.of());
Review comment:
I think we should recover the correlated variables when the `input` is a
project instead of passing an empty set,
##########
File path:
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
##########
@@ -43,13 +47,15 @@
public class ElasticsearchProject extends Project implements ElasticsearchRel {
ElasticsearchProject(RelOptCluster cluster, RelTraitSet traitSet, RelNode
input,
List<? extends RexNode> projects, RelDataType rowType) {
- super(cluster, traitSet, ImmutableList.of(), input, projects, rowType);
+ super(cluster, traitSet, ImmutableList.of(), input, projects, rowType,
ImmutableSet.of());
assert getConvention() == ElasticsearchRel.CONVENTION;
assert getConvention() == input.getConvention();
}
@Override public Project copy(RelTraitSet relTraitSet, RelNode input,
List<RexNode> projects,
- RelDataType relDataType) {
+ RelDataType relDataType, Set<CorrelationId> variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: subqueries is -> subqueries are
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
##########
@@ -187,7 +187,7 @@ protected FilterProjectTransposeRule(
RelNode newProject =
config.isCopyProject()
? project.copy(project.getTraitSet(), newFilterRel,
Review comment:
The behavior below seems a bit problematic. When we copy we propagate
the variables and when we use the builder we are losing them. Seeing this makes
me a bit skeptical about all the rules where we are matching a project and then
use the builder to recreate the output. Have you investigated all calls to
project?
##########
File path: core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
##########
@@ -6565,11 +6568,13 @@ protected MyFilterRule(Config config) {
RelNode input,
List<? extends RexNode> projects,
RelDataType rowType) {
- super(cluster, traitSet, ImmutableList.of(), input, projects, rowType);
+ super(cluster, traitSet, ImmutableList.of(), input, projects, rowType,
ImmutableSet.of());
}
public MyProject copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path:
geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeProject.java
##########
@@ -45,13 +49,15 @@
GeodeProject(RelOptCluster cluster, RelTraitSet traitSet,
RelNode input, List<? extends RexNode> projects, RelDataType rowType) {
- super(cluster, traitSet, ImmutableList.of(), input, projects, rowType);
+ super(cluster, traitSet, ImmutableList.of(), input, projects, rowType,
ImmutableSet.of());
assert getConvention() == GeodeRel.CONVENTION;
assert getConvention() == input.getConvention();
}
@Override public Project copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path: core/src/main/java/org/apache/calcite/tools/RelBuilder.java
##########
@@ -1794,7 +1794,74 @@ public RelBuilder project(Iterable<? extends RexNode>
nodes,
*/
public RelBuilder project(Iterable<? extends RexNode> nodes,
Iterable<? extends @Nullable String> fieldNames, boolean force) {
- return project_(nodes, fieldNames, ImmutableList.of(), force);
+ return project_(ImmutableSet.of(), nodes, fieldNames, ImmutableList.of(),
force);
+ }
+
+ /** Creates a {@link Project} of a set of correlation variables
+ * and the given expressions. */
+ public RelBuilder projectCorrelated(Iterable<CorrelationId> variablesSet,
+ RexNode... nodes) {
+ return projectCorrelated(variablesSet, ImmutableList.copyOf(nodes));
+ }
+
+ /** Creates a {@link Project} of a set of correlation variables
+ * and the given list of expressions.
+ *
+ * <p>Infers names as would {@link #projectCorrelated(Iterable, Iterable,
Iterable)}
+ * if all suggested names were null.
+ *
+ * @param variablesSet Set of correlation variables
+ * @param nodes Expressions
+ */
+ public RelBuilder projectCorrelated(Iterable<CorrelationId> variablesSet,
+ Iterable<? extends RexNode> nodes) {
+ return projectCorrelated(variablesSet, nodes, ImmutableList.of());
+ }
+
+ /** Creates a {@link Project} of a set of correlation variables
+ * and the given list of expressions and field names.
+ *
+ * @param variablesSet Set of correlation variables
+ * @param nodes Expressions
+ * @param fieldNames field names for expressions
+ */
+ public RelBuilder projectCorrelated(
+ Iterable<CorrelationId> variablesSet,
+ Iterable<? extends RexNode> nodes,
+ Iterable<? extends @Nullable String> fieldNames) {
+ return projectCorrelated(variablesSet, nodes, fieldNames, false);
+ }
+
+ /** Creates a {@link Project} of a set of correlation variables
+ * and the given list of expressions, using the given names.
+ *
+ * <p>Names are deduced as follows:
+ * <ul>
+ * <li>If the length of {@code fieldNames} is greater than the index of
+ * the current entry in {@code nodes}, and the entry in
+ * {@code fieldNames} is not null, uses it; otherwise
+ * <li>If an expression projects an input field,
+ * or is a cast an input field,
+ * uses the input field name; otherwise
+ * <li>If an expression is a call to
+ * {@link SqlStdOperatorTable#AS}
+ * (see {@link #alias}), removes the call but uses the intended alias.
+ * </ul>
+ *
+ * <p>After the field names have been inferred, makes the
+ * field names unique by appending numeric suffixes.
+ *
+ * @param variablesSet Set of correlation variables
+ * @param nodes Expressions
+ * @param fieldNames Suggested field names
+ * @param force create project even if it is identity
+ */
+ public RelBuilder projectCorrelated(
+ Iterable<CorrelationId> variablesSet,
+ Iterable<? extends RexNode> nodes,
+ Iterable<? extends @Nullable String> fieldNames,
+ boolean force) {
+ return project_(variablesSet, nodes, fieldNames, ImmutableList.of(),
force);
Review comment:
Where are these new APIs used? I didn't see any usages.
##########
File path:
core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
##########
@@ -664,6 +664,46 @@ LogicalProject(EMPNO=[$0], JOB=[$2])
]]>
</Resource>
</TestCase>
+ <TestCase name="testCorrelatedScalarSubQueryInSelectList">
+ <Resource name="planNotExpanded">
+ <![CDATA[
+LogicalProject(correlation=[[$cor0]], DEPTNO=[$7], EXPR$1=[$SCALAR_QUERY({
+LogicalAggregate(group=[{}], EXPR$0=[COUNT()])
+ LogicalProject($f0=[0])
+ LogicalFilter(condition=[>($cor0.DEPTNO, 0)])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+})], EXPR$2=[$SCALAR_QUERY({
+LogicalAggregate(group=[{}], EXPR$0=[COUNT()])
+ LogicalProject($f0=[0])
+ LogicalFilter(condition=[AND(>($cor0.DEPTNO, 0), <($cor0.DEPTNO, 10))])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+})])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planExpanded">
+ <![CDATA[
+LogicalProject(DEPTNO=[$7], EXPR$1=[$9], EXPR$2=[$10])
+ LogicalCorrelate(correlation=[$cor1], joinType=[left], requiredColumns=[{7}])
+ LogicalCorrelate(correlation=[$cor0], joinType=[left],
requiredColumns=[{7}])
Review comment:
Are the changes in `SubqueryRemoveRule` done so that you get a different
correlate variable here?
If yes can you explain why it is a problem to have the same correlated
variable. I am asking this cause we have other plans in the repo where multiple
correlates have the same correlation id. Check plans for:
* RelOptRulesTest#testDecorrelateTwoExists
* RelOptRulesTest#testDecorrelateTwoExists#testDecorrelateTwoIns
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java
##########
@@ -129,16 +132,33 @@ protected RexNode apply(RexSubQuery e, Set<CorrelationId>
variablesSet,
*/
private static RexNode rewriteScalarQuery(RexSubQuery e, Set<CorrelationId>
variablesSet,
RelBuilder builder, int inputCount, int offset) {
- builder.push(e.rel);
- final RelMetadataQuery mq = e.rel.getCluster().getMetadataQuery();
+ // in case the relation has several scalar queries
+ // referred to the same correlated variable, every
+ // such a query will be re-written to a particular
+ // correlate with the common set of variables,
+ // resulting a different relations will be trying to
+ // set the same correlated variable. To overcome this
+ // let's rewrite an every correlated variable with a
+ // new one created specially for current scalar query
+ RelNode r = e.rel;
+ RelOptCluster cluster = r.getCluster();
+ ImmutableSet.Builder<CorrelationId> newVariablesSet =
ImmutableSet.builder();
+ for (CorrelationId corrId : variablesSet) {
+ CorrelationId newId = cluster.createCorrel();
+ newVariablesSet.add(newId);
+ r = DeduplicateCorrelateVariables.go(cluster.getRexBuilder(), newId,
+ ImmutableSet.of(corrId), r);
+ }
+ builder.push(r);
+ final RelMetadataQuery mq = cluster.getMetadataQuery();
final Boolean unique = mq.areColumnsUnique(builder.peek(),
ImmutableBitSet.of());
Review comment:
Thanks for adding the explanation comment. Unfortunately, I don't fully
understand why the deduplication logic fits here. Can you maybe given an
example in terms of plans?
Moreover this code is called in three cases: subquery in join, subquery in
project, subquery in filter. Is the problem you observed for project
reproducible for filter or join? If not why?
Furthermore, if the deduplication is needed for `rewriteScalar` why it is
not needed for `rewriteExists`, `rewriteIn` etc?
##########
File path: core/src/main/java/org/apache/calcite/rel/core/Project.java
##########
@@ -70,6 +71,8 @@
protected final ImmutableList<RelHint> hints;
+ protected final ImmutableSet<CorrelationId> variablesSet;
Review comment:
nit. Adding some javadoc here would be nice. I think it's easier to find
rather than in the constructor.
##########
File path: core/src/main/java/org/apache/calcite/rel/core/Project.java
##########
@@ -120,14 +134,15 @@ protected Project(RelInput input) {
ImmutableList.of(),
input.getInput(),
requireNonNull(input.getExpressionList("exprs"), "exprs"),
- input.getRowType("exprs", "fields"));
+ input.getRowType("exprs", "fields"),
+ ImmutableSet.of());
Review comment:
If we serialize variables we should be able to deserialize them. It
seems that now at least JSON serialization/deserialization will be broken when
project operators have variables. If it is out of the scope of this change then
I would strongly recommend creating a follow-up JIRA and putting appropriate
references here using the `Bug` class.
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -395,10 +396,11 @@ protected DruidProjectRule(DruidProjectRuleConfig config)
{
}
builder.add(name, e.getType());
}
- final RelNode newProject = project.copy(project.getTraitSet(), input,
below, builder.build());
+ final RelNode newProject = project.copy(project.getTraitSet(), input,
below, builder.build(),
+ ImmutableSet.of());
Review comment:
Why we pass empty instead of `project.getVariablesSet` since we are
copying?
##########
File path:
innodb/src/main/java/org/apache/calcite/adapter/innodb/InnodbProject.java
##########
@@ -42,13 +46,15 @@
public class InnodbProject extends Project implements InnodbRel {
InnodbProject(RelOptCluster cluster, RelTraitSet traitSet,
RelNode input, List<? extends RexNode> projects, RelDataType rowType) {
- super(cluster, traitSet, ImmutableList.of(), input, projects, rowType);
+ super(cluster, traitSet, ImmutableList.of(), input, projects, rowType,
ImmutableSet.of());
assert getConvention() == InnodbRel.CONVENTION;
assert getConvention() == input.getConvention();
}
@Override public Project copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ProjectWindowTransposeRule.java
##########
@@ -99,7 +100,7 @@ public ProjectWindowTransposeRule(RelBuilderFactory
relBuilderFactory) {
final LogicalProject projectBelowWindow =
new LogicalProject(cluster, window.getTraitSet(), ImmutableList.of(),
- window.getInput(), exps, builder.build());
+ window.getInput(), exps, builder.build(), ImmutableSet.of());
Review comment:
Is it safe to transpose project with windows when the former has
correlated variables? Also it seems that here we are going to lose the
variables set in the original projection. Is this intentional?
##########
File path: core/src/main/java/org/apache/calcite/rel/logical/LogicalProject.java
##########
@@ -58,29 +61,34 @@
* @param input Input relational expression
* @param projects List of expressions for the input columns
* @param rowType Output row type
+ * @param variablesSet Correlation variables set by this relational
expression
+ * to be used by nested expressions
*/
public LogicalProject(
RelOptCluster cluster,
RelTraitSet traitSet,
List<RelHint> hints,
RelNode input,
List<? extends RexNode> projects,
- RelDataType rowType) {
- super(cluster, traitSet, hints, input, projects, rowType);
+ RelDataType rowType,
+ Set<CorrelationId> variablesSet) {
+ super(cluster, traitSet, hints, input, projects, rowType, variablesSet);
assert traitSet.containsIfApplicable(Convention.NONE);
}
Review comment:
Should we deprecate the old constructor instead of removing it directly?
##########
File path: druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java
##########
@@ -395,10 +396,11 @@ protected DruidProjectRule(DruidProjectRuleConfig config)
{
}
builder.add(name, e.getType());
}
- final RelNode newProject = project.copy(project.getTraitSet(), input,
below, builder.build());
+ final RelNode newProject = project.copy(project.getTraitSet(), input,
below, builder.build(),
+ ImmutableSet.of());
final DruidQuery newQuery = DruidQuery.extendQuery(query, newProject);
final RelNode newProject2 = project.copy(project.getTraitSet(),
newQuery, above,
- project.getRowType());
+ project.getRowType(), project.getVariablesSet());
call.transformTo(newProject2);
Review comment:
Should the `DruidProjectRule` match and transform an expression since it
doesn't know how to handle correlation variables?
##########
File path:
core/src/test/java/org/apache/calcite/plan/volcano/TraitPropagationTest.java
##########
@@ -386,7 +390,9 @@ public static PhysProj create(final RelNode input,
}
public PhysProj copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> exps, RelDataType rowType) {
+ List<RexNode> exps, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableRelFactories.java
##########
@@ -68,7 +70,10 @@
implements org.apache.calcite.rel.core.RelFactories.ProjectFactory {
@Override public RelNode createProject(RelNode input, List<RelHint> hints,
List<? extends RexNode> childExprs,
- @Nullable List<? extends @Nullable String> fieldNames) {
+ @Nullable List<? extends @Nullable String> fieldNames,
+ Set<CorrelationId> variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path:
mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoProject.java
##########
@@ -56,7 +60,9 @@ public MongoProject(RelOptCluster cluster, RelTraitSet
traitSet,
}
@Override public Project copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableProject.java
##########
@@ -80,7 +84,9 @@ public static EnumerableProject create(final RelNode input,
}
@Override public EnumerableProject copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path: pig/src/main/java/org/apache/calcite/adapter/pig/PigProject.java
##########
@@ -35,12 +39,14 @@
/** Creates a PigProject. */
public PigProject(RelOptCluster cluster, RelTraitSet traitSet, RelNode input,
List<? extends RexNode> projects, RelDataType rowType) {
- super(cluster, traitSet, ImmutableList.of(), input, projects, rowType);
+ super(cluster, traitSet, ImmutableList.of(), input, projects, rowType,
ImmutableSet.of());
assert getConvention() == PigRel.CONVENTION;
}
@Override public Project copy(RelTraitSet traitSet, RelNode input,
List<RexNode> projects,
- RelDataType rowType) {
+ RelDataType rowType, Set<CorrelationId> variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path: core/src/main/java/org/apache/calcite/interpreter/Bindables.java
##########
@@ -403,12 +405,14 @@ protected BindableProjectRule(Config config) {
public static class BindableProject extends Project implements BindableRel {
public BindableProject(RelOptCluster cluster, RelTraitSet traitSet,
RelNode input, List<? extends RexNode> projects, RelDataType rowType) {
- super(cluster, traitSet, ImmutableList.of(), input, projects, rowType);
+ super(cluster, traitSet, ImmutableList.of(), input, projects, rowType,
ImmutableSet.of());
assert getConvention() instanceof BindableConvention;
}
@Override public BindableProject copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path:
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraProject.java
##########
@@ -42,13 +46,15 @@
public class CassandraProject extends Project implements CassandraRel {
public CassandraProject(RelOptCluster cluster, RelTraitSet traitSet,
RelNode input, List<? extends RexNode> projects, RelDataType rowType) {
- super(cluster, traitSet, ImmutableList.of(), input, projects, rowType);
+ super(cluster, traitSet, ImmutableList.of(), input, projects, rowType,
ImmutableSet.of());
assert getConvention() == CassandraRel.CONVENTION;
assert getConvention() == input.getConvention();
}
@Override public Project copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path: site/_docs/algebra.md
##########
@@ -326,6 +326,7 @@ return the `RelBuilder`.
| `values(fieldNames, value...)`<br/>`values(rowType, tupleList)` | Creates a
[Values]({{ site.apiRoot }}/org/apache/calcite/rel/core/Values.html).
| `filter([variablesSet, ] exprList)`<br/>`filter([variablesSet, ] expr...)` |
Creates a [Filter]({{ site.apiRoot }}/org/apache/calcite/rel/core/Filter.html)
over the AND of the given predicates; if `variablesSet` is specified, the
predicates may reference those variables.
| `project(expr...)`<br/>`project(exprList [, fieldNames])` | Creates a
[Project]({{ site.apiRoot }}/org/apache/calcite/rel/core/Project.html). To
override the default name, wrap expressions using `alias`, or specify the
`fieldNames` argument.
+| `projectCorrelated(variablesSet,
expr...)`<br/>`projectCorrelated(variablesSet, exprList [, fieldNames])` |
Variant of `project` that creates correlated project.
Review comment:
If we don't add new APIs this must be removed.
##########
File path: core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
##########
@@ -544,7 +547,9 @@ public JdbcProject(RelOptCluster cluster, RelTraitSet
traitSet,
}
@Override public JdbcProject copy(RelTraitSet traitSet, RelNode input,
- List<RexNode> projects, RelDataType rowType) {
+ List<RexNode> projects, RelDataType rowType, Set<CorrelationId>
variablesSet) {
+ Preconditions.checkArgument(variablesSet.isEmpty(),
+ "Correlated scalar subqueries is not supported");
Review comment:
nit: is -> are
##########
File path:
core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
##########
@@ -534,10 +535,21 @@ public void rewriteRel(LogicalProject rel) {
RelNode newInput = getNewForOldRel(rel.getInput());
List<RexNode> newProjects = Pair.left(flattenedExpList);
List<String> newNames = Pair.right(flattenedExpList);
- final RelNode newRel = relBuilder.push(newInput)
+ RelNode renamedProject = relBuilder.push(newInput)
.projectNamed(newProjects, newNames, true)
Review comment:
I suppose that if we could pass correlations in `projectNamed` method
the snippet below wouldn't be necessary.
##########
File path: core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
##########
@@ -2654,16 +2654,6 @@ protected void convertCollectionTable(
validator().getValidatedNodeType(call),
columnMappings);
- final SqlValidatorScope selectScope =
- ((DelegatingScope) bb.scope()).getParent();
- final Blackboard seekBb = createBlackboard(selectScope, null, false);
-
- final CorrelationUse p = getCorrelationUse(seekBb, callRel);
- if (p != null) {
- assert p.r instanceof LogicalTableFunctionScan;
- callRel = (LogicalTableFunctionScan) p.r;
- }
-
Review comment:
Can you explain why this code snippet need to be removed?
##########
File path:
core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
##########
@@ -7074,17 +7114,17 @@ from dept]]>
</Resource>
<Resource name="plan">
Review comment:
If we don't have a similar query in `sub-query.iq` please include this
as well so that test that it actually runs.
##########
File path:
cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraRules.java
##########
@@ -258,7 +258,8 @@ private static boolean isEqualityOnKey(RexNode node,
List<String> fieldNames,
public static class CassandraProjectRule extends CassandraConverterRule {
/** Default configuration. */
private static final Config DEFAULT_CONFIG = Config.INSTANCE
- .withConversion(LogicalProject.class, Convention.NONE,
+ .withConversion(LogicalProject.class, p -> p.getCorrelVariable() ==
null
+ || p.getVariablesSet().isEmpty(), Convention.NONE,
Review comment:
Do we need to check both `p.getCorrelVariable()` and
`p.getVariablesSet()`? From the API and existing implementations I get the
impression that `p.getVariablesSet()` is a superset of `p.getCorrelVariable()`.
##########
File path: core/src/test/resources/sql/sub-query.iq
##########
@@ -3257,6 +3257,34 @@ EnumerableCalc(expr#0..7=[{inputs}],
expr#8=[null:BOOLEAN], proj#0..8=[{exprs}])
EnumerableTableScan(table=[[scott, EMP]])
!plan
+# [CALCITE-4913] Correlated variables in a select list are not deduplicated
+select e.deptno,
+ (select count(*) from emp where e.deptno > 0) as c1,
+ (select count(*) from emp where e.deptno > 0 and e.deptno < 10) as c2
+ from emp e;
+
++--------+----+----+
+| DEPTNO | C1 | C2 |
++--------+----+----+
+| 10 | 14 | 0 |
+| 10 | 14 | 0 |
+| 10 | 14 | 0 |
+| 20 | 14 | 0 |
+| 20 | 14 | 0 |
+| 20 | 14 | 0 |
+| 20 | 14 | 0 |
+| 20 | 14 | 0 |
+| 30 | 14 | 0 |
+| 30 | 14 | 0 |
+| 30 | 14 | 0 |
+| 30 | 14 | 0 |
+| 30 | 14 | 0 |
+| 30 | 14 | 0 |
++--------+----+----+
+(14 rows)
+
+!ok
Review comment:
Apart from the result please also include the `!plan`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
