Skip to content

add DictionaryEncodedStringValueIndex implementation to NestedFieldLiteralColumnIndexSupplier#12837

Merged
clintropolis merged 1 commit intoapache:masterfrom
clintropolis:fix-nested-column-search-query
Aug 2, 2022
Merged

add DictionaryEncodedStringValueIndex implementation to NestedFieldLiteralColumnIndexSupplier#12837
clintropolis merged 1 commit intoapache:masterfrom
clintropolis:fix-nested-column-search-query

Conversation

@clintropolis
Copy link
Member

Description

This PR adds an implementation to DictionaryEncodedStringValueIndex to NestedFieldLiteralColumnIndexSupplier, which is used in a handful of places for doing stuff with dictionary encoded columns, such as native search and segment metadata queries, fixing some bugs.

I also removed getIndex and hasNulls from DictionaryEncodedStringValueIndex, which were only used by tests.

There is another issue not addressed in this PR, which is that these things rely on ColumnCapabilities.hasBitmapIndexes to check if they can use indexes, which is really no longer adequate to know if something will have the type of indexes that they are going to ask for after #12388. I think in the future we might want to either update column capabilities to include all of the classes the ColumnIndexSupplier can supply, or make these callers check the ColumnIndexSupplier themselves.

This PR has:

  • been self-reviewed.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@clintropolis clintropolis merged commit 6046a39 into apache:master Aug 2, 2022
@clintropolis clintropolis deleted the fix-nested-column-search-query branch August 2, 2022 04:40
@abhishekagarwal87 abhishekagarwal87 added this to the 24.0.0 milestone Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants