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

fix wrong result with WrappedImmutableConciseBitmap#difference, issue… #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lushuifeng
Copy link

@lushuifeng lushuifeng commented Apr 9, 2018

#40 for details, please take a look, thanks

@lemire
Copy link
Contributor

lemire commented Apr 9, 2018

Why is the difference implemented as the intersection with the complement when there is an andNot method that would be (no doubt) more efficient and simpler?

@lushuifeng
Copy link
Author

I do not think andNot method is exist in ImmutableBitmap, I would perfer using andNot method if there is one. The bitmap is deserialized from file content which is ImmutableBitmap, and then a lot of and, or, andNot is performed on ImmutableBitmap

@lemire
Copy link
Contributor

lemire commented Apr 9, 2018

@lushuifeng
Copy link
Author

there is one in ImmutableRoaringBitmap but not in ImmutableConciseBitmap, when it comes to ImmutableConciseBitmap#difference method, no better function call I've found. the type of bitmap(roaring or concise) is deserialized from file which is not fixed

@lemire
Copy link
Contributor

lemire commented Apr 9, 2018

Ok. Thanks, I understand.

@lushuifeng
Copy link
Author

thanks

@gianm
Copy link

gianm commented Apr 9, 2018

Hi @lushuifeng, is this related to an issue you have noticed with Druid? If so, could you please raise this as a patch in the druid-io/druid repo? We have incorporated this code into Druid itself, so this library is no longer used.

@lushuifeng
Copy link
Author

Sorry, not yet. It seems that this method is not invoked in druid, not deep into this

@lushuifeng
Copy link
Author

latest version is not checked, verson 0.9.3 seems ok.

@lushuifeng
Copy link
Author

lushuifeng commented Apr 10, 2018

It should work in latest druid, the filters and bitmapFactory implementations do not involve with this issue. BTW, how can I use the latest bitmap dependency since this is incorporated into druid?

@gianm
Copy link

gianm commented Apr 10, 2018

The Druid libraries are on Maven Central so you could include them from there.

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.

3 participants