You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "bvolpato (via GitHub)" <gi...@apache.org> on 2026/03/09 19:47:23 UTC
[PR] [CALCITE-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
bvolpato opened a new pull request, #4828:
URL: https://github.com/apache/calcite/pull/4828
## Summary
Fixes a RelToSqlConverter failure when semi/anti-join rewrites produce correlated filters whose correlation id is not present in .
The change is intentionally small:
- register missing ids while visiting filter conditions;
- use so existing mappings are not overwritten;
- keep existing alias rendering behavior unchanged.
## Test
- added ;
- verified full remains green ( 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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4055052104
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r2925097992
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11887,6 +11887,39 @@ public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
sql(sql).schema(CalciteAssert.SchemaSpec.JDBC_SCOTT).ok(expected);
}
+ /** Test case for
Review Comment:
+1
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4054805496
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4031321011
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] RelToSqlConverter throws NPE (variable $cor1 not found) for correlated projection after semi-join rewrites [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-5194964039
This is nicer.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4105331678
@xiedeyantu I addressed the test-structure part of your note:
- removed the `try-catch` from `testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules`
- updated the PR description so it matches the retained reproducer and current fix
On the Jira alias suggestion: I considered it, but I kept this PR scoped to the failure mode reproduced here. The retained regression still fails on `main` and passes with this patch, while changing alias generation in `visit(Correlate)` would be a broader behavior change than the NPE fix in this PR.
If you want, I can also split out the alias-cleanup direction as a separate follow-up.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4285933585
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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-7440] RelToSqlConverter throws NPE (variable $cor1 not found) for correlated projection after semi-join rewrites [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-5188727813
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] RelToSqlConverter throws NPE (variable $cor1 not found) for correlated projection after semi-join rewrites [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r3714984983
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -508,9 +508,13 @@ private boolean isCommaJoin(Join join) {
/** Visits a Correlate; called by {@link #dispatch} via reflection. */
public Result visit(Correlate e) {
+ final Result visitedLeft = visitInput(e, 0);
Review Comment:
This seems to make some cosmetic changes to the generated code, like using t0 instead of $cor0.
Which one is the actual important semantic change?
Is this cosmetic change valuable, or can it be omitted (maybe it would make comparing the changes in the new plans easier)?
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -12661,6 +12661,37 @@ public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
sql(sql).schema(CalciteAssert.SchemaSpec.JDBC_SCOTT).ok(expected);
}
+ @Test void testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules() {
Review Comment:
Please follow the existing pattern for documenting this test using JavaDoc, there are lots of examples.
No need to have a separate function for the SQL, just make it a final String.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4086873495
Could we please consider @asolimando 's suggestion in Jira? Personally, I don't think this particular fix is entirely appropriate. Incidentally, regarding the structure of the test case: you might want to refer to other examples for guidance. Please avoid using `try-catch` blocks; if an error occurs, it should be allowed to surface naturally.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4031375838
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [33.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r2930870161
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11887,6 +11887,39 @@ public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
sql(sql).schema(CalciteAssert.SchemaSpec.JDBC_SCOTT).ok(expected);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7440">[CALCITE-7440]
+ * RelToSqlConverter throws NPE when correlation scope is missing after
+ * semi-join rewrites.</a>. */
+ @Test void testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules() {
Review Comment:
Please take a look at my description; you may not have solved my problem.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "Dwrite (via GitHub)" <gi...@apache.org>.
Dwrite commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r2924974296
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11887,6 +11887,39 @@ public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
sql(sql).schema(CalciteAssert.SchemaSpec.JDBC_SCOTT).ok(expected);
}
+ /** Test case for
Review Comment:
The current test case testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules covers the scenario where optimization rules are applied. To ensure the fix is robust for the core RelToSql mapping logic, would it be better to add a plain test case without these rules?
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4105414482
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r2921792133
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11887,6 +11887,39 @@ public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
sql(sql).schema(CalciteAssert.SchemaSpec.JDBC_SCOTT).ok(expected);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7440">[CALCITE-7440]
+ * RelToSqlConverter throws NPE when correlation scope is missing after
+ * semi-join rewrites.</a>. */
+ @Test void testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules() {
Review Comment:
I ran this test on the main branch, and strangely it passed. Could you check if your case can reproduce the problem you mentioned?
```
@Test void testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules() {
final String query = "WITH product_keys AS (\n"
+ " SELECT p.\"product_id\",\n"
+ " (SELECT MAX(p3.\"product_id\")\n"
+ " FROM \"foodmart\".\"product\" p3\n"
+ " WHERE p3.\"product_id\" = p.\"product_id\") AS \"mx\"\n"
+ " FROM \"foodmart\".\"product\" p\n"
+ ")\n"
+ "SELECT DISTINCT pk.\"product_id\"\n"
+ "FROM product_keys pk\n"
+ "LEFT JOIN \"foodmart\".\"product\" p2 USING (\"product_id\")\n"
+ "WHERE pk.\"product_id\" IN (\n"
+ " SELECT p4.\"product_id\"\n"
+ " FROM \"foodmart\".\"product\" p4\n"
+ ")";
final RuleSet rules = RuleSets.ofList();
final String generated = sql(query).withPostgresql().optimize(rules, null).exec();
sql(generated).withPostgresql().exec();
}
```
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4054619531
Updated this PR with both requested changes:\n\n1. Reduced the regression to the minimal reproducer on main:\n- FILTER_SUB_QUERY_TO_MARK_CORRELATE\n- PROJECT_SUB_QUERY_TO_MARK_CORRELATE\n- MARK_TO_SEMI_OR_ANTI_JOIN_RULE\n- SEMI_JOIN_JOIN_TRANSPOSE\n\nI also ran exhaustive combinations for sizes 1/2/3; none reproduced, so the minimum size is 4.\n\n2. Added a plain round-trip test without extra rules:\n- testPostgresqlRoundTripCorrelatedProjectWithoutExtraRules\n\nBoth new/updated tests pass locally in this branch.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-5175642602
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "bvolpato-dd (via GitHub)" <gi...@apache.org>.
bvolpato-dd commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r2940922566
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11887,6 +11887,39 @@ public Sql schema(CalciteAssert.SchemaSpec schemaSpec) {
sql(sql).schema(CalciteAssert.SchemaSpec.JDBC_SCOTT).ok(expected);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7440">[CALCITE-7440]
+ * RelToSqlConverter throws NPE when correlation scope is missing after
+ * semi-join rewrites.</a>. */
+ @Test void testPostgresqlRoundTripCorrelatedProjectWithSemiJoinRules() {
Review Comment:
You were right. The plain round-trip test passed on main, so it was not validating the bug.
I removed that test and kept only the semi-join rewrite reproducer.
I also re-ran the retained test against main locally by copying it into a detached main worktree. It fails there with `NullPointerException: variable $cor1 is not found`, and it passes on this branch.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4068533713
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4028751095
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [46.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4321349815
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4828&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4828&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4828&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4828)
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4850413244
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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-7440] RelToSqlConverter throws NPE (variable $cor1 not found) for correlated projection after semi-join rewrites [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #4828:
URL: https://github.com/apache/calcite/pull/4828
--
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-7440] RelToSqlConverter throws NPE (variable $cor1 not found) for correlated projection after semi-join rewrites [calcite]
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r3712998293
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -507,9 +507,13 @@ private boolean isCommaJoin(Join join) {
/** Visits a Correlate; called by {@link #dispatch} via reflection. */
public Result visit(Correlate e) {
+ final Set<String> usedNames = new HashSet<>(aliasSet);
Review Comment:
fixed. We now visit the left input first, reserve `t`, and allocate the correlation alias from the shared alias set. This avoids collisions with input aliases and nested `VALUES ... AS t` relations.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on PR #4828:
URL: https://github.com/apache/calcite/pull/4828#issuecomment-4378364138
[review-prs-verification]
Head: 614ce4ea1fafdda7fc590d30ef145b19af904440
Ready to merge: yes
Files reviewed:
- core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
- core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
- core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterStructsTest.java
- core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
- piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
Tests/checks run and results:
- Local: `./gradlew :core:test --tests org.apache.calcite.rel.rel2sql.RelToSqlConverterTest --tests org.apache.calcite.rel.rel2sql.RelToSqlConverterStructsTest :piglet:test --tests org.apache.calcite.test.PigRelOpTest :core:checkstyleMain :core:checkstyleTest :core:forbiddenApisMain :core:forbiddenApisTest :piglet:checkstyleTest :piglet:forbiddenApisTest :core:assemble :piglet:assemble` -> BUILD SUCCESSFUL
- GitHub PR checks (`gh pr checks 4828 --watch`): all required checks passing (CI matrix + Jenkins + SonarCloud), with optional `Linux (JDK 8) Slow Tests` skipped.
Fixes committed/pushed:
- None. No completion blockers found in reviewed diff.
Final CI status:
- Passing on current head `614ce4ea1fafdda7fc590d30ef145b19af904440`.
--
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-7440] Preserve correlated scope mapping in RelToSqlConverter [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4828:
URL: https://github.com/apache/calcite/pull/4828#discussion_r3190805076
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -507,9 +507,13 @@ private boolean isCommaJoin(Join join) {
/** Visits a Correlate; called by {@link #dispatch} via reflection. */
public Result visit(Correlate e) {
+ final Set<String> usedNames = new HashSet<>(aliasSet);
Review Comment:
Can you explain why it's safe to use this name?
--
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