Skip to content

Conversation

@xiedeyantu
Copy link
Member

@xiedeyantu xiedeyantu commented Jan 5, 2026

See CALCITE-4765

Only add tests.

@xiedeyantu
Copy link
Member Author

@NobiGo If you have time, could you please check if it's working correctly now? Since I couldn't find an emps table containing commission in quidem test, I've temporarily replaced it with emp from scott.

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.

The CI is unhappy for some reason too

@xiedeyantu
Copy link
Member Author

The CI is unhappy for some reason too

Fixed, thanks!

@xiedeyantu
Copy link
Member Author

This is the result compared with PGSQL.

@xiedeyantu
Copy link
Member Author

I tried constructing emps and resubmitting the test.

@mihaibudiu
Copy link
Contributor

This is the result compared with PGSQL.

Is this the right link?

@xiedeyantu
Copy link
Member Author

This is the result compared with PGSQL.

Is this the right link?

Sorry, I have updated the original SQL to the SQL in the current PR.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 6, 2026

!}

!if (use_new_decorr) {
# TODO: TopDownGeneralDecorrelator should fix trimmer error
Copy link
Contributor

Choose a reason for hiding this comment

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

Is anyone working on fixing this?
Should we wait for the fix?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an issue of compatibility between the new decorrelator and Trimmer(I think this is a problem we expected). I think we could create a Jira instance to document this problem. Waiting until this issue is fixed before merging this PR is also a good option.

Copy link
Member Author

Choose a reason for hiding this comment

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

If no one wants to work on it, I will try to fix the problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Start by creating a new issue and check with @NobiGo, he has been working on the trimmer recently.

Copy link
Contributor

Choose a reason for hiding this comment

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

If an issue is created, please @ me.


!ok

# [CALCITE-4765] Complex correlated EXISTS sub-queries used as scalar subqueries can return wrong results
Copy link
Contributor

Choose a reason for hiding this comment

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

why is blank the right file for this test?

Copy link
Member Author

@xiedeyantu xiedeyantu Jan 6, 2026

Choose a reason for hiding this comment

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

Since no emps table with a commission column was found in quidem test, I need to create one. Alternatively, we can use !use blank in other .iq files.

!}

!if (use_new_decorr) {
# TODO: TopDownGeneralDecorrelator should fix trimmer error
Copy link
Contributor

Choose a reason for hiding this comment

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

If an issue is created, please @ me.

SELECT e2.deptno FROM emps e2 where e1.commission = e2.commission) as table3
where table3.deptno <> e1.deptno)
from emps e1;
!if (use_old_decorr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Which one is used when running this specific test case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you so much for your prompt attention to this issue. It involves new Quidm tests, new TopDownGeneralDecorrelator, and new MARK JOINs. Please feel free to @ me if you'd like to discuss it. @silundong If you have time, you can also look into this issue, because you might understand it better. Thanks!

@xiedeyantu
Copy link
Member Author

@NobiGo @mihaibudiu I have file a new jira CALCITE-7356 to describe it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants