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

Relation::from_antijoin is broken #39

Closed
ecstatic-morse opened this issue Aug 18, 2021 · 1 comment
Closed

Relation::from_antijoin is broken #39

ecstatic-morse opened this issue Aug 18, 2021 · 1 comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Aug 18, 2021

Relation::from_antijoin always returns an empty Relation, regardless of its inputs. That's because the antijoin helper, which takes a JoinInput as its first parameter, operates only on recent tuples.

datafrog/src/join.rs

Lines 65 to 73 in 5bda2f0

let results = input1
.recent()
.iter()
.filter(|(ref key, _)| {
tuples2 = gallop(tuples2, |k| k < key);
tuples2.first() != Some(key)
})
.map(|(ref key, ref val)| logic(key, val))
.collect::<Vec<_>>();

This is correct for variables, but Relations, which don't change during iteration, only have stable tuples. See #36 (comment) for the reason this must be the case.

To fix this, we should refactor the antijoin helper to work directly on Relations, and pass the proper input from Variable::from_antijoin and Relation::from_antijoin. A regression test is needed as well.

@ecstatic-morse ecstatic-morse added help wanted Extra attention is needed good first issue Good for newcomers labels Aug 18, 2021
michalt added a commit to michalt/datafrog that referenced this issue Sep 12, 2021
Now the `antijoin` helper accepts a `Relation`, which does exactly what
we want in `Relation::from_antijoin`, while in `Variable::from_antijoin`
we now explicitly pass just the `recent` `Relation`.

Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
ecstatic-morse added a commit that referenced this issue Sep 13, 2021
Fix `Relation::from_antijoin` (issue #39)
@ecstatic-morse
Copy link
Contributor Author

Fixed in #42.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant