Skip to content
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.

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

Fix nullable colums for subqueries in left and full joins #1462

Closed
wants to merge 3 commits into from

Conversation

richardbizik
Copy link

There is an issue when performing join on subquery. Fields that should be nullable are not because node is processed as ordinary select node. This pull request fixes the issue.

Copy link
Collaborator

@kyleconroy kyleconroy left a comment

Choose a reason for hiding this comment

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

Sorry, the current approach taken in this PR isn't going to work. The sample query added to the left join test is helpful to identify the problem.

internal/sql/astutils/walk.go Outdated Show resolved Hide resolved
internal/sql/ast/range_subselect.go Outdated Show resolved Hide resolved
@richardbizik
Copy link
Author

I made some changes. Would this approach work better?

@sbres
Copy link
Contributor

sbres commented Mar 16, 2022

I've tried to run the sql query but I seem unable to make it work as is.

Could you check it ?
https://www.db-fiddle.com/f/hXNdApf4mV3Qi1jEgu2egZ/3

@richardbizik
Copy link
Author

here is fixed fiddle https://www.db-fiddle.com/f/f8yeYgvggnmw1t7p3M5YE6/1
I am going to update code as well.

Copy link
Collaborator

@kyleconroy kyleconroy left a comment

Choose a reason for hiding this comment

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

Is this trying to fix the same problem as #1486? If not, I'd like to merge that first before we continue working on this change.

You have some merge conflicts here. Can you rebase?

@sbres
Copy link
Contributor

sbres commented Mar 19, 2022

I've closed #1486 as #1491 applies the same fix.
But they do not fix what this PR is trying to achieve

@richardbizik
Copy link
Author

As @sbres pointed out they are fixing a different issue. This PR is trying to solve an issue with nested select statements where an information is lost about the parent being join that should produce nullable results.

@kdubovikov
Copy link

I think this fix is what we need to solve #1334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants