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

Lowering: Fix multiplicity error #7359

Merged
merged 1 commit into from
Jul 9, 2021
Merged

Conversation

wangandi
Copy link
Contributor

@wangandi wangandi commented Jul 9, 2021

Closes MaterializeInc/database-issues#2209

exists(bar).applied_to(foo), when bar is "simple", was being converted to:

bar.applied_to(foo).distinct_by(foo_cols).map(true) 
union
(foo join 
   (distinct(foo) - (bar.applied_to(foo)).distinct_by(foo_cols))
).map(false)

This caused the loss of all but one copy of each row for where exists(bar).

The correct conversion should be

(foo join 
  bar.applied_to(foo).distinct_by(foo_cols)
).map(true) 
union
(foo join 
   (distinct(foo) - (bar.applied_to(foo)).distinct_by(foo_cols))
).map(false)

Copy link
Contributor

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

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

I don't understand this code, unfortunately! It seems to pass tests, and I could imagine it is correct. @benesch might know more about it!

@benesch
Copy link
Contributor

benesch commented Jul 9, 2021

Er, I think you added this code, @frankmcsherry, in #5651! Seems good though.

@wangandi wangandi merged commit 3f6b341 into MaterializeInc:main Jul 9, 2021
@wangandi wangandi deleted the issue7121 branch July 9, 2021 21:40
@frankmcsherry
Copy link
Contributor

I don't think I did. It was added in #5651 though I think by you. In any case, I don't understand it! :D

aljoscha pushed a commit to aljoscha/materialize that referenced this pull request Jul 21, 2021
The multiplicity of the rows from outer query was not preserved there is an EXISTS subquery involving only constants/mpf/flatmaps.
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

3 participants