Skip to content

Conversation

@max-hoffman
Copy link
Contributor

@max-hoffman max-hoffman commented Jan 13, 2025

Full join should exhaust right side, not return as soon as we EOF the left iterator.

fixes: dolthub/dolt#8735

Copy link
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

LGTM

@max-hoffman max-hoffman merged commit 09d646b into main Jan 13, 2025
8 checks passed
@max-hoffman max-hoffman deleted the max/fix-full-join-exhaust-right-iter branch January 13, 2025 22:11
@samjewell
Copy link
Contributor

This is awesome 🎉
Thanks for making this change @max-hoffman @jycor

return nil, err
}
if _, ok := i.seenLeft[key]; !ok {
// (left, null) only if we haven't matched left
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment still correct? It says null but you've replaced the nil in the LoC below.

if _, ok := i.seenRight[key]; ok {
continue
}
// (null, right) only if we haven't matched right
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment still correct? It says null but you've replaced the nil in the LoC below.

@samjewell
Copy link
Contributor

@max-hoffman I asked a question in a previous comment after you merged, but figured I should tag you in case you missed my comments.

Also:
Your docs here say:

FULL OUTER joins are not supported.

Should this be updated now?

@max-hoffman
Copy link
Contributor Author

@max-hoffman I asked a question in a previous comment after you merged, but figured I should tag you in case you missed my comments.

Also: Your docs here say:

FULL OUTER joins are not supported.

Should this be updated now?

Yes thanks for the reminder, I fixed the docs here.
dolthub/docs#2479

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.

[Bug] FULL OUTER JOIN not commutative, and not giving correct results

4 participants