Skip to content

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

Open
lushuifeng wants to merge 1 commit into
metamx:masterfrom
lushuifeng:master
Open

fix wrong result with WrappedImmutableConciseBitmap#difference, issue…#41
lushuifeng wants to merge 1 commit into
metamx:masterfrom
lushuifeng:master

Conversation

@lushuifeng

@lushuifeng lushuifeng commented Apr 9, 2018

Copy link
Copy Markdown

#40 for details, please take a look, thanks

@lemire

lemire commented Apr 9, 2018

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

lemire commented Apr 9, 2018

Copy link
Copy Markdown
Contributor

@lushuifeng

Copy link
Copy Markdown
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

lemire commented Apr 9, 2018

Copy link
Copy Markdown
Contributor

Ok. Thanks, I understand.

@lushuifeng

Copy link
Copy Markdown
Author

thanks

@gianm

gianm commented Apr 9, 2018

Copy link
Copy Markdown

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
Copy Markdown
Author

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

@lushuifeng

Copy link
Copy Markdown
Author

latest version is not checked, verson 0.9.3 seems ok.

@lushuifeng

lushuifeng commented Apr 10, 2018

Copy link
Copy Markdown
Author

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

gianm commented Apr 10, 2018

Copy link
Copy Markdown

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