[CALCITE-7064] Test introduced in [CALCITE-7009] breaks the build for main

[korlov42]

https://issues.apache.org/jira/browse/CALCITE-7064

[suibianwanwank]

BTW, I think we could move changeCorrelateReferences into RelOptUtil. Also, we could consider renaming it, for example, changing change to mapping.

[korlov42]

@suibianwanwank thanks! The only thing I'm a little bit concerned about is that LogicalProject now exposes variablesSet in certain cases. But as far as I see this is valid approach to force RelBuilder not to merge projections:

// org.apache.calcite.tools.RelBuilder#project_:
    <...>
    
    // Do not merge projection when top projection has correlation variables
    bloat:
    if (frame.rel instanceof Project
        && config.bloat() >= 0
        && variables.isEmpty()) {

[suibianwanwank]

@suibianwanwank thanks! The only thing I'm a little bit concerned about is that LogicalProject now exposes variablesSet in certain cases. But as far as I see this is valid approach to force RelBuilder not to merge projections:

// org.apache.calcite.tools.RelBuilder#project_:
    <...>
    
    // Do not merge projection when top projection has correlation variables
    bloat:
    if (frame.rel instanceof Project
        && config.bloat() >= 0
        && variables.isEmpty()) {

I don't quite understand what 'exposes' means here. From my understanding, variablesSet'is indispensable, in nested subQueries, it clearly defines the source of variables. But in many rules and programs, 'variablesSet' gets discarded. In my other uncommitted branches, I've made some related fixes.

[korlov42]

BTW, I think we could move changeCorrelateReferences into RelOptUtil. Also, we could consider renaming it, for example, changing change to mapping.

thanks for suggestion, done. See changes in this commit.

[korlov42]

From my understanding, variablesSet'is indispensable, in nested subQueries, it clearly defines the source of variables.

I absolutely agree with you. I only wanted to highlight that this particular change (even though I believe it's perfectly valid) is not directly relates to the problem I'm addressing in this patch, and different parts of the system may or may not consider variablesSet returned by Project rel. In other words, this particular change may cause a different execution path in some environments, hence may cause a "regression" (again, there are a lot of IF's, and I don't think this change is or will be the root cause, but rather trigger which may highlight problem somewhere else).

Anyway, I just wanted to share my thoughts on the matter.

[mihaibudiu]

This is pretty impressive: delete 100 lines of code and make the code more correct!

[suibianwanwank]

@korlov42 You can squash your commits, and i will merge it. I believe there are still some issues, but they are more general and also present elsewhere in Calcite, so they are not directly related to this PR.

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[beikov]

Thanks for the fix @korlov42, this also solved query planning issues of correlated subqueries that we recently faced. Looking forward to the next release!