-
Notifications
You must be signed in to change notification settings - Fork 9
Add n-way table unions #489
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
Conversation
|
This is a WIP, so not reviewable just yet |
0b7b775 to
d18d2fb
Compare
d18d2fb to
d5c8ba4
Compare
|
Should be ready for review now! Note that this PR is built on top of #486. |
dcoutts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 looks fine, though seems to me there's no need to exclude the trivial case of n == 1.
dcoutts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, meant to approve, not just comment.
Originally, I had put underscores in the field names so that they wouldn't show up in "unused identifier" compiler errors. But, since they're exported now anyway, we can get rid of the underscores
d5c8ba4 to
ad5d621
Compare
dcoutts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
n-way table unions should be more performant than repeated 2-way unions