Skip to content

[CALCITE-7257] Subqueries cannot be decorrelated if join condition contains RexFieldAccess #4659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mihaibudiu merged 1 commit into from
Dec 25, 2025
Merged

[CALCITE-7257] Subqueries cannot be decorrelated if join condition contains RexFieldAccess #4659

mihaibudiu merged 1 commit into from
Dec 25, 2025

Conversation

@iwanttobepowerful
Copy link
Contributor

@iwanttobepowerful iwanttobepowerful commented Dec 2, 2025

@iwanttobepowerful iwanttobepowerful force-pushed the calcite-7257 branch 5 times, most recently from 0254e6d to 58db579 Compare December 8, 2025 03:38
iwanttobepowerful
iwanttobepowerful commented Dec 10, 2025
View reviewed changes
@iwanttobepowerful iwanttobepowerful force-pushed the calcite-7257 branch 8 times, most recently from cde48df to a4571f9 Compare December 13, 2025 13:35
@iwanttobepowerful iwanttobepowerful force-pushed the calcite-7257 branch 6 times, most recently from 5e89482 to 1d2486c Compare December 13, 2025 23:53
@iwanttobepowerful
Copy link
Contributor Author

iwanttobepowerful commented Dec 14, 2025

@mihaibudiu @suibianwanwank
Hi there, could you please review this PR? I think it’s ready for review now.

mihaibudiu
mihaibudiu reviewed Dec 18, 2025
View reviewed changes
Copy link
Contributor

@mihaibudiu mihaibudiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine, but @suibianwanwank knows this code much better, so it would be nice to get their opinion as well if possible. If that's not possible, I will approve this.

@iwanttobepowerful
Copy link
Contributor Author

iwanttobepowerful commented Dec 22, 2025

I will approve this

I think this PR is ready to be merged.

@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Dec 22, 2025
@mihaibudiu
Copy link
Contributor

mihaibudiu commented Dec 22, 2025

Last chance to make comments before merging!

@mihaibudiu
Copy link
Contributor

mihaibudiu commented Dec 23, 2025

The checker framework seems to have failed, do you want to look into it?

@mihaibudiu
Copy link
Contributor

mihaibudiu commented Dec 23, 2025

Maybe it was due to github problems, yesterday they had quite a few

@mihaibudiu
Copy link
Contributor

mihaibudiu commented Dec 23, 2025

Can you rebase so we can just make sure all CI tests pass?

@mihaibudiu
Copy link
Contributor

mihaibudiu commented Dec 23, 2025

This also has a conflict now, please rebase and fix and I will merge.

@iwanttobepowerful
Copy link
Contributor Author

iwanttobepowerful commented Dec 23, 2025

This also has a conflict now, please rebase and fix and I will merge.

Thanks.

@iwanttobepowerful iwanttobepowerful force-pushed the calcite-7257 branch 2 times, most recently from 5fd0ae8 to 88a4b43 Compare December 24, 2025 05:47
@mihaibudiu
Copy link
Contributor

mihaibudiu commented Dec 24, 2025

I don't understand why the merge button is disabled.

@xiedeyantu
Copy link
Member

xiedeyantu commented Dec 24, 2025

I don't understand why the merge button is disabled.

+1

@xiedeyantu xiedeyantu marked this pull request as draft December 24, 2025 22:57
@xiedeyantu xiedeyantu marked this pull request as ready for review December 24, 2025 22:57
@xiedeyantu
Copy link
Member

xiedeyantu commented Dec 24, 2025

I can't find a viable solution. Perhaps you could try force pushing again?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 25, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

@xiedeyantu
Copy link
Member

xiedeyantu commented Dec 25, 2025

The button is now working properly, and you can merge them at any time. @mihaibudiu

@iwanttobepowerful
Copy link
Contributor Author

iwanttobepowerful commented Dec 25, 2025

Thanks. @xiedeyantu @mihaibudiu

@mihaibudiu mihaibudiu merged commit fcb86f0 into apache:main Dec 25, 2025
20 checks passed
@iwanttobepowerful iwanttobepowerful deleted the calcite-7257 branch December 26, 2025 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mihaibudiu mihaibudiu mihaibudiu approved these changes

Assignees

No one assigned

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iwanttobepowerful @mihaibudiu @xiedeyantu