You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "ehds (via GitHub)" <gi...@apache.org> on 2026/09/28 06:09:16 UTC
[PR] fix [calcite]
ehds opened a new pull request, #5298:
URL: https://github.com/apache/calcite/pull/5298
<!--
Thanks for sending a pull request! Here are some critical tips for you:
1. READ THE GUIDE FIRST: https://calcite.apache.org/develop/#contributing
*For significant contributions, please discuss on the dev mailing list or Jira BEFORE coding.*
2. JIRA IS USUALLY MANDATORY: Ensure you have created an issue on the Calcite Jira:
https://issues.apache.org/jira/projects/CALCITE/issues
*Check existing issues first to avoid duplicates.*
*Note: A Jira is NOT required for typos and cosmetic changes (i.e., changes that are neither bugs nor features).*
3. TIMING: Strongly recommended to create the Jira BEFORE you start writing code (e.g., a day or so before posting a PR).
This gives others a chance to weigh in on your specification.
4. CRITICAL CONSISTENCY RULE
The following three items MUST match exactly in wording and meaning:
(A) The Jira Issue Title
(B) This Pull Request Title
(C) Your Git Commit Message
Format: [CALCITE-XXXX] <Description>
Example: [CALCITE-0000] Add IF NOT EXISTS clause to CREATE TABLE
Guidelines for a good Title:
- Illustrate using SQL keywords (in ALL-CAPS) rather than Java method names.
- Focus on the specification and user experience, not the implementation.
- Make it clear whether it is a bug or a feature.
- Use words like "should" to indicate desired behavior vs. current behavior (e.g., "Validator should not close model file").
5. REPRODUCTION: If fixing a bug, please provide a concise SQL example or test case to reproduce the issue for a faster review.
6. TESTING: Ensure `./gradlew build` passes and appropriate tests are added/updated.
-->
## Jira Link
[CALCITE-7825](https://issues.apache.org/jira/browse/CALCITE-7825)
RelMdTableReferences misses tables in RexSubQuery expressions in Project, Filter, Join, and Calc. It also has no Correlate handler, so rewriting a sub-query can change the result to null.
Other nodes also return null despite having determinable table references: Values should return an empty set, unary nodes should pass through their input references, and RepeatUnion and Combine should merge theirs.
## Changes Proposed
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR.
-->
Extend RelMdTableReferences to cover sub-queries, Correlate, and other nodes that previously returned null despite having determinable table references. Refactor Join to use the shared reference-merging helper.
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5298:
URL: https://github.com/apache/calcite/pull/5298#issuecomment-5864698545
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5298) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5298&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5298&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5298&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5298&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5298&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5298)
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5298:
URL: https://github.com/apache/calcite/pull/5298#discussion_r4125626615
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -160,31 +157,40 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
* references.
*/
public @Nullable Set<RelTableRef> getTableReferences(SetOp rel, RelMetadataQuery mq) {
- return getTableReferences(rel.getInputs(), mq);
+ return getTableReferences(rel.getInputs(), ImmutableList.of(), mq);
}
/**
- * Table references from the relational inputs of a TableFunctionScan.
+ * Table references from TableFunctionScan.
*
- * <p>Returns an empty set if there are no inputs, and {@code null} if the table
- * references of any input cannot be determined. Tables accessed internally
Review Comment:
why delete this comment?
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -270,4 +277,29 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
return mq.getTableReferences(rel.getInput());
}
+ /** Table references from Snapshot. */
+ public @Nullable Set<RelTableRef> getTableReferences(Snapshot rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
Review Comment:
don't you need to analyze the period operand to?
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -270,4 +277,29 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
return mq.getTableReferences(rel.getInput());
}
+ /** Table references from Snapshot. */
+ public @Nullable Set<RelTableRef> getTableReferences(Snapshot rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Match. */
+ public @Nullable Set<RelTableRef> getTableReferences(Match rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Uncollect. */
+ public @Nullable Set<RelTableRef> getTableReferences(Uncollect rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Collect. */
+ public @Nullable Set<RelTableRef> getTableReferences(Collect rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Spool. */
+ public @Nullable Set<RelTableRef> getTableReferences(Spool rel, RelMetadataQuery mq) {
Review Comment:
apparently there are still some classes not handled, like Window, Sort, TableModify
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -270,4 +277,29 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
return mq.getTableReferences(rel.getInput());
}
+ /** Table references from Snapshot. */
+ public @Nullable Set<RelTableRef> getTableReferences(Snapshot rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Match. */
+ public @Nullable Set<RelTableRef> getTableReferences(Match rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
Review Comment:
don't you need to analyze the pattern and measures operands?
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -160,31 +157,40 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
* references.
*/
public @Nullable Set<RelTableRef> getTableReferences(SetOp rel, RelMetadataQuery mq) {
- return getTableReferences(rel.getInputs(), mq);
+ return getTableReferences(rel.getInputs(), ImmutableList.of(), mq);
}
/**
- * Table references from the relational inputs of a TableFunctionScan.
+ * Table references from TableFunctionScan.
*
- * <p>Returns an empty set if there are no inputs, and {@code null} if the table
- * references of any input cannot be determined. Tables accessed internally
- * by the table function are not included.
+ * <p>Returns an empty set when its inputs and call reference no tables.
*/
public @Nullable Set<RelTableRef> getTableReferences(TableFunctionScan rel,
RelMetadataQuery mq) {
- return getTableReferences(rel.getInputs(), mq);
+ return getTableReferences(rel.getInputs(), ImmutableList.of(rel.getCall()), mq);
}
- /** Returns the union of the table references of {@code inputs}, assigning
- * distinct entity numbers to repeated references to the same table, or {@code null}
- * if the references of any input cannot be determined. */
+ /** Returns the union of the table references of {@code inputs} and expression
+ * sub-queries, assigning distinct entity numbers to repeated references to
+ * the same table, or {@code null} if the references of any input or sub-query
+ * cannot be determined. */
private static @Nullable Set<RelTableRef> getTableReferences(
- List<RelNode> inputs, RelMetadataQuery mq) {
+ List<RelNode> inputs, List<? extends RexNode> expressions, RelMetadataQuery mq) {
+ final List<RelNode> rels = new ArrayList<>(inputs);
+ final RexVisitorImpl<Void> visitor = new RexVisitorImpl<Void>(true) {
Review Comment:
you can reuse the SubQueryCollector visitor
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on PR #5298:
URL: https://github.com/apache/calcite/pull/5298#issuecomment-5924184621
> Please squash the commits for merging.
Done.
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5298:
URL: https://github.com/apache/calcite/pull/5298#issuecomment-5903954675
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5298) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5298&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5298&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5298&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [65.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5298&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5298&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5298)
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #5298:
URL: https://github.com/apache/calcite/pull/5298#issuecomment-5922874461
Please squash the commits for merging.
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on code in PR #5298:
URL: https://github.com/apache/calcite/pull/5298#discussion_r4140659743
##########
core/src/test/java/org/apache/calcite/test/RelMetadataTest.java:
##########
@@ -4895,6 +4899,141 @@ public void checkAllPredicatesAndTableSetOp(String sql) {
assertNull(tableReferences);
}
+ @Test void testTableReferencesValues() {
Review Comment:
These tests use SQL or RelBuilder to construct the different Rel types being tested, following the same approach as the existing tests.I've checked each plan and its expected table references.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -160,31 +157,40 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
* references.
*/
public @Nullable Set<RelTableRef> getTableReferences(SetOp rel, RelMetadataQuery mq) {
- return getTableReferences(rel.getInputs(), mq);
+ return getTableReferences(rel.getInputs(), ImmutableList.of(), mq);
}
/**
- * Table references from the relational inputs of a TableFunctionScan.
+ * Table references from TableFunctionScan.
*
- * <p>Returns an empty set if there are no inputs, and {@code null} if the table
- * references of any input cannot be determined. Tables accessed internally
Review Comment:
The shared behavior is already documented at the class level, and this PR also includes tables referenced by sub-queries in TableFunctionScan’s RexCall. Therefore, I simplified the comment to keep it consistent with the other methods.
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on code in PR #5298:
URL: https://github.com/apache/calcite/pull/5298#discussion_r4140663317
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -270,4 +277,29 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
return mq.getTableReferences(rel.getInput());
}
+ /** Table references from Snapshot. */
+ public @Nullable Set<RelTableRef> getTableReferences(Snapshot rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Match. */
+ public @Nullable Set<RelTableRef> getTableReferences(Match rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
Review Comment:
Yes, I've updated the handler to inspect the pattern, measures, and other RexNode expressions in Match as well.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -270,4 +277,29 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
return mq.getTableReferences(rel.getInput());
}
+ /** Table references from Snapshot. */
+ public @Nullable Set<RelTableRef> getTableReferences(Snapshot rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Match. */
+ public @Nullable Set<RelTableRef> getTableReferences(Match rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Uncollect. */
+ public @Nullable Set<RelTableRef> getTableReferences(Uncollect rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Collect. */
+ public @Nullable Set<RelTableRef> getTableReferences(Collect rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
+ }
+
+ /** Table references from Spool. */
+ public @Nullable Set<RelTableRef> getTableReferences(Spool rel, RelMetadataQuery mq) {
Review Comment:
Window, Sort, and TableModify already had handlers in the existing code.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -160,31 +157,40 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
* references.
*/
public @Nullable Set<RelTableRef> getTableReferences(SetOp rel, RelMetadataQuery mq) {
- return getTableReferences(rel.getInputs(), mq);
+ return getTableReferences(rel.getInputs(), ImmutableList.of(), mq);
}
/**
- * Table references from the relational inputs of a TableFunctionScan.
+ * Table references from TableFunctionScan.
*
- * <p>Returns an empty set if there are no inputs, and {@code null} if the table
- * references of any input cannot be determined. Tables accessed internally
- * by the table function are not included.
+ * <p>Returns an empty set when its inputs and call reference no tables.
*/
public @Nullable Set<RelTableRef> getTableReferences(TableFunctionScan rel,
RelMetadataQuery mq) {
- return getTableReferences(rel.getInputs(), mq);
+ return getTableReferences(rel.getInputs(), ImmutableList.of(rel.getCall()), mq);
}
- /** Returns the union of the table references of {@code inputs}, assigning
- * distinct entity numbers to repeated references to the same table, or {@code null}
- * if the references of any input cannot be determined. */
+ /** Returns the union of the table references of {@code inputs} and expression
+ * sub-queries, assigning distinct entity numbers to repeated references to
+ * the same table, or {@code null} if the references of any input or sub-query
+ * cannot be determined. */
private static @Nullable Set<RelTableRef> getTableReferences(
- List<RelNode> inputs, RelMetadataQuery mq) {
+ List<RelNode> inputs, List<? extends RexNode> expressions, RelMetadataQuery mq) {
+ final List<RelNode> rels = new ArrayList<>(inputs);
+ final RexVisitorImpl<Void> visitor = new RexVisitorImpl<Void>(true) {
Review Comment:
The existing SubQueryCollector exposed only `collect(Project)`, and its visitSubQuery stopped after recording a sub-query, so it could miss sub-queries nested in a sub-query’s operands.
It also had no callers in the current codebase. I updated it to accept a deep option and reused the visitor in RelMdTableReferences to scan each expression with deep=true.
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "ehds (via GitHub)" <gi...@apache.org>.
ehds commented on code in PR #5298:
URL: https://github.com/apache/calcite/pull/5298#discussion_r4140660843
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdTableReferences.java:
##########
@@ -270,4 +277,29 @@ public Set<RelTableRef> getTableReferences(TableScan rel, RelMetadataQuery mq) {
return mq.getTableReferences(rel.getInput());
}
+ /** Table references from Snapshot. */
+ public @Nullable Set<RelTableRef> getTableReferences(Snapshot rel, RelMetadataQuery mq) {
+ return mq.getTableReferences(rel.getInput());
Review Comment:
Yes, the period expression should also be inspected. For example, [MariaDB allows a sub-query here](https://onecompiler.com/mariadb/454rjfxva). I've updated the handler accordingly.
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5298:
URL: https://github.com/apache/calcite/pull/5298#discussion_r4125556758
##########
core/src/test/java/org/apache/calcite/test/RelMetadataTest.java:
##########
@@ -4895,6 +4899,141 @@ public void checkAllPredicatesAndTableSetOp(String sql) {
assertNull(tableReferences);
}
+ @Test void testTableReferencesValues() {
Review Comment:
these tests are not easy for a human to review, I hope they are right
--
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-7825] RelMdTableReferences does not account for tables in RexSubQuery expressions or Correlate [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #5298:
URL: https://github.com/apache/calcite/pull/5298
--
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