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.

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

feat(api): support the inner join convenience to not repeat fields known to be equal #8127

Merged
merged 4 commits into from
Feb 2, 2024
Merged

feat(api): support the inner join convenience to not repeat fields known to be equal #8127

merged 4 commits into from
Feb 2, 2024

Conversation

kszucs
Copy link
Member

@kszucs kszucs commented Jan 28, 2024

  • feat(api): support the inner join convenience to not repeat fields known to be equal
  • test(sql): regenerate SQL snapshot for the inner join convenience feature

kszucs
kszucs commented Jan 28, 2024
View reviewed changes
ibis/backends/tests/snapshots/test_generic/test_many_subqueries/bigquery/out.sql
t5.street,
ROW_NUMBER() OVER (ORDER BY t5.street ASC) - 1 AS key
FROM (
SELECT
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to remove these redundant selects soon-ish.

@kszucs kszucs requested review from jcrist and cpcloud January 28, 2024 22:55
@kszucs kszucs force-pushed the inner-join-convenience branch from c176af6 to 0717430 Compare January 28, 2024 22:58
@kszucs
Copy link
Member Author

kszucs commented Jan 28, 2024 •
edited
Loading

@cpcloud could you please update the snapshots for the failing backends? I'm unable to run them locally.

@kszucs kszucs force-pushed the inner-join-convenience branch 5 times, most recently from e0c042e to c7935db Compare January 29, 2024 10:09
@cpcloud cpcloud added tes-required-for-release Things that must be addressed before *release* of `main` after merging in `the-epic-split` ux User experience related issues labels Jan 29, 2024
@kszucs kszucs force-pushed the inner-join-convenience branch from 8c1ba0d to 33cc1b6 Compare January 29, 2024 14:28
@kszucs kszucs mentioned this pull request Jan 29, 2024
Merged
cpcloud
cpcloud reviewed Jan 29, 2024
View reviewed changes
ibis/expr/tests/test_newrels.py Show resolved Hide resolved
ibis/expr/types/joins.py Show resolved Hide resolved
cpcloud
cpcloud reviewed Feb 1, 2024
View reviewed changes
ibis/expr/types/joins.py
object.__setattr__(self, "_collisions", collisions or set())
object.__setattr__(self, "_equalities", equalities or DisjointSet())
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm: there's an instance of DisjoinSet per instance of Join, and each instance of Join corresponds to a chain of joins. Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. The disjoint set gets copied as new links aded to the join chain.

cpcloud
cpcloud approved these changes Feb 1, 2024
View reviewed changes
Copy link
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

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

LGTM, excellent work and novel use of a disjoint set. Thanks for doing this.

🚢 it

@cpcloud cpcloud added this to the 9.0 milestone Feb 1, 2024
@kszucs kszucs force-pushed the the-epic-split branch 2 times, most recently from 3376022 to 432d151 Compare February 1, 2024 17:39
@cpcloud cpcloud enabled auto-merge (squash) February 2, 2024 10:00
@cpcloud cpcloud force-pushed the inner-join-convenience branch from 33cc1b6 to 401bcdc Compare February 2, 2024 10:01
@cpcloud cpcloud merged commit e7fd1fc into ibis-project:the-epic-split Feb 2, 2024
59 of 65 checks passed
kszucs added a commit that referenced this pull request Feb 2, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
d5e256f 
…own to be equal (#8127)

Co-authored-by: Phillip Cloud <[email protected]>
@kszucs kszucs mentioned this pull request Feb 3, 2024
Closed
1 task
cpcloud added a commit to cpcloud/ibis that referenced this pull request Feb 4, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
bfc6168 
…own to be equal (ibis-project#8127)

Co-authored-by: Phillip Cloud <[email protected]>
cpcloud added a commit to cpcloud/ibis that referenced this pull request Feb 5, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
deadc1b 
…own to be equal (ibis-project#8127)

Co-authored-by: Phillip Cloud <[email protected]>
kszucs added a commit that referenced this pull request Feb 5, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
71b1b1a 
…own to be equal (#8127)

Co-authored-by: Phillip Cloud <[email protected]>
kszucs added a commit that referenced this pull request Feb 6, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
12f7a56 
…own to be equal (#8127)

Co-authored-by: Phillip Cloud <[email protected]>
kszucs added a commit that referenced this pull request Feb 6, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
c23c51d 
…own to be equal (#8127)

Co-authored-by: Phillip Cloud <[email protected]>
cpcloud added a commit to cpcloud/ibis that referenced this pull request Feb 12, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
92da3f5 
…own to be equal (ibis-project#8127)

Co-authored-by: Phillip Cloud <[email protected]>
cpcloud added a commit that referenced this pull request Feb 12, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
c67aa8d 
…own to be equal (#8127)

Co-authored-by: Phillip Cloud <[email protected]>
cpcloud added a commit to cpcloud/ibis that referenced this pull request Feb 12, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
1e580fe 
…own to be equal (ibis-project#8127)

Co-authored-by: Phillip Cloud <[email protected]>
cpcloud added a commit that referenced this pull request Feb 12, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
62284da 
…own to be equal (#8127)

Co-authored-by: Phillip Cloud <[email protected]>
kszucs added a commit that referenced this pull request Feb 12, 2024
@kszucs @cpcloud
feat(api): support the inner join convenience to not repeat fields kn…
798088d 
…own to be equal (#8127)

Co-authored-by: Phillip Cloud <[email protected]>
ncclementi pushed a commit to ncclementi/ibis that referenced this pull request Feb 21, 2024
@kszucs @cpcloud @ncclementi
feat(api): support the inner join convenience to not repeat fields kn…
94a07f1 
…own to be equal (ibis-project#8127)

Co-authored-by: Phillip Cloud <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpcloud cpcloud cpcloud approved these changes

@jcrist jcrist Awaiting requested review from jcrist

Assignees
No one assigned
Labels
tes-required-for-release Things that must be addressed before *release* of `main` after merging in `the-epic-split` ux User experience related issues
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.
2 participants
@kszucs @cpcloud