specialized FixedIndexed implementations for java value types #12846
Conversation
| { | ||
| this.segmentWriteOutMedium = segmentWriteOutMedium; | ||
| // this is a matter of faith, nothing checks | ||
| this.isSorted = sorted; |
There was a problem hiding this comment.
Do we need to know if it's sorted at this level? If we are being told that it is supposed to be sorted, then I think that we should verify that it's sorted. We can do that relatively easily by just having the most-recent value, checking that the next one is greater and then storing that.
Or, if we are trusting something external from us to know that it is already sorted, then we shouldn't even bother asking it to tell us if we are sorted or not and instead just expect the external code to do the right thing.
| final boolean hasNull = (flags & NullHandling.IS_NULL_BYTE) == NullHandling.IS_NULL_BYTE ? true : false; | ||
| final boolean isSorted = (flags & FixedIndexed.IS_SORTED_MASK) == FixedIndexed.IS_SORTED_MASK ? true : false; | ||
| Preconditions.checkState(!hasNull, "Cannot use FixedIndexedInts for FixedIndex with null values"); | ||
| Preconditions.checkState(!(hasNull && !isSorted), "cannot have null values if not sorted"); |
There was a problem hiding this comment.
I'm pretty sure that this precondition will always be true if the precondition of !hasNull from the line above is true.
| @@ -622,7 +630,7 @@ public Iterable<ImmutableBitmap> getBitmapIterable() | |||
| final DruidLongPredicate longPredicate = matcherFactory.makeLongPredicate(); | |||
|
|
|||
| // in the future, this could use an int iterator | |||
| final Iterator<Integer> iterator = dictionary.iterator(); | |||
| final Iterator<Integer> iterator = dictionary.intIterator(); | |||
There was a problem hiding this comment.
you switched to an intIterator, but are still using an Iterator<Integer>?
| } | ||
|
|
||
| @Override | ||
| public Iterator<Integer> iterator() |
There was a problem hiding this comment.
It looks like IntIterator already implements Iterator<Integer> so you don't need to wrap it again.
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request/issue has been closed due to lack of activity. If you think that |
|
This pull request has been marked as stale due to 60 days of inactivity. |
Description
Adds specialized implementations for java long, double, and int value type implementations of
FixedIndexed, which is used by the nested data columns added in #12753.While not entirely attributable to this PR (the range filtering tests owe that to #12830), repeating the benchmarks done in show
improvement:
This PR has: