mihaibudiu commented on code in PR #3640:
URL: https://github.com/apache/calcite/pull/3640#discussion_r1459864781
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -5491,18 +5491,24 @@ ImmutableList<RelNode> retrieveCursors() {
builder.add(convertExpression(node));
}
final ImmutableList<RexNode> list = builder.build();
+ RelNode rel = root.rel;
+ CorrelationUse correlationUse = getCorrelationUse(this, root.rel);
Review Comment:
Could you add a comment explaining what happens here?
##########
core/src/main/java/org/apache/calcite/prepare/Prepare.java:
##########
@@ -293,13 +293,6 @@ public PreparedResult prepareSql(
root = root.withRel(decorrelate(sqlToRelConverter, sqlQuery, root.rel));
}
- if (configHolder.get().isTrimUnusedFields()) {
Review Comment:
This config feature seems to only be used here.
So perhaps it can be removed completely?
My question is whether the bug is in the trimUnusedFields method, or is it
in invoking it here.
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -8217,6 +8217,72 @@ private void checkGetTimestamp(Connection con) throws
SQLException {
.returns("EXPR$0=[1, 1.1]\n");
}
+ @Test void testMultiLevelDecorrelation() throws Exception {
Review Comment:
Should this function have a JavaDoc describing the issue fixed?
##########
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableCorrelateTest.java:
##########
@@ -143,13 +143,13 @@ class EnumerableCorrelateTest {
.query(
"select empid, name from emps e where exists (select 1 from depts
d where d.deptno=e.deptno)")
.explainContains(""
- + "EnumerableCalc(expr#0..3=[{inputs}], empid=[$t0], name=[$t2])\n"
- + " EnumerableCorrelate(correlation=[$cor0], joinType=[inner],
requiredColumns=[{1}])\n"
- + " EnumerableCalc(expr#0..4=[{inputs}], proj#0..2=[{exprs}])\n"
- + " EnumerableTableScan(table=[[s, emps]])\n"
+ + "EnumerableCalc(expr#0..5=[{inputs}], empid=[$t0], name=[$t2])\n
"
Review Comment:
Is this case even changed?
##########
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableCorrelateTest.java:
##########
@@ -143,13 +143,13 @@ class EnumerableCorrelateTest {
.query(
"select empid, name from emps e where exists (select 1 from depts
d where d.deptno=e.deptno)")
.explainContains(""
- + "EnumerableCalc(expr#0..3=[{inputs}], empid=[$t0], name=[$t2])\n"
- + " EnumerableCorrelate(correlation=[$cor0], joinType=[inner],
requiredColumns=[{1}])\n"
- + " EnumerableCalc(expr#0..4=[{inputs}], proj#0..2=[{exprs}])\n"
- + " EnumerableTableScan(table=[[s, emps]])\n"
+ + "EnumerableCalc(expr#0..5=[{inputs}], empid=[$t0], name=[$t2])\n
"
Review Comment:
Please move the spaces at the end of the previous line here.
Same in the subsequent lines.
##########
core/src/test/resources/sql/sub-query.iq:
##########
@@ -2225,7 +2225,7 @@ where exists
# following plan (two scans of EMP table instead of three).
EnumerableCalc(expr#0..2=[{inputs}], ENAME=[$t1])
EnumerableHashJoin(condition=[=($2, $3)], joinType=[semi])
- EnumerableCalc(expr#0..7=[{inputs}], expr#8=[IS NOT NULL($t3)],
expr#9=[CAST($t3):INTEGER], expr#10=[0], expr#11=[CASE($t8, $t9, $t10)],
proj#0..1=[{exprs}], $f3=[$t11])
+ EnumerableCalc(expr#0..7=[{inputs}], expr#8=[IS NOT NULL($t3)],
expr#9=[CAST($t3):INTEGER], expr#10=[0], expr#11=[CASE($t8, $t9, $t10)],
proj#0..1=[{exprs}], $f8=[$t11])
Review Comment:
This seems to be a test for a prior bug.
Are you saying that the result was incorrect and is correct 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
