[BUG] Wildcard sort error when `doc_value` enabled and `size` less than total docs

[HUSTERGS]

Describe the bug

Current wildcard field implementation uses SortedSet as the doc value type, which will trigger an dynamic pruning in lucene when the size is less than total docs and query contains sort.

Spefically, lucene expect the term dict also contains the doc values, but this is not true for wildcard, because it uses N-gram to generate terms, but full value in doc value, which will cause an IllegalState Exception

https://github.com/apache/lucene/blob/485141dd34ea866ad9dc59843770969d1b0c8fa2/lucene/core/src/java/org/apache/lucene/search/comparators/TermOrdValComparator.java#L569-L572

Related component

Search

To Reproduce

I add another yaml test case under my repo HUSTERGS@8c7cb9b

Expected behavior

Wildcard sort can work properly

Additional Details

Additional context
I'm thinking about backward compatibility, the simplest way is to change SortedSetDocValue to BinaryDocValue, but already written data might have problem when upgrade the OS, we can add some version check to solve this. Another way is to add the full value to the term dict explicitly by WildcardFieldTokenizer which may help address this problem

[ajleong623]

hi, this seems like an interesting issue and a great learning opportunity. I could look into it.

[msfroh]

Oh... that is very interesting. @ajleong623 -- please feel free to nag me to review your fix once it's ready.

Both of @HUSTERGS's ideas are worth exploring, I think. I have another idea that may be practical, but I don't want to spoil it. 😁

[bugmakerrrrrr]

For existing indices, maybe we can get around this issue by explicitly disabling skipping for the wildcard field.

[ajleong623]

@bugmakerrrrrr That is true. The wildcard field implementation might not be compatible with the dynamic pruning. I wanted to try out the other solutions, first.

[ajleong623]

@HUSTERGS @msfroh I am still working on the problem. I am just learning about how wildcards are actually parsed and how sort actually works at a deeper level. So, it looks like the comparison is not valid because in the terms are not parsed into the lucene terms dict.

Based on the solution mentioned above, when we’re adding the documents to the index, make sure that the N gram of the value is also added to the terms dict? (Let me know if this was the intended solution @HUSTERGS )

I am still a bit new to a lot of the codebase, so my understanding at the moment is a bit fuzzy, but I’ve been trying to dig through the path of the error to have a better understanding of what went wrong. But I think this is a great learning experience, and I’m glad I have this opportunity. I’ll plan to have the solution implementation by the middle of the week.

[HUSTERGS]

Based on the solution mentioned above, when we’re adding the documents to the index, make sure that the N gram of the value is also added to the terms dict? (Let me know if this was the intended solution @HUSTERGS )

@ajleong623 Yes, your understanding is correct! My previous thought is to add orignal full text into the term dict (just like how KeywordFieldMapper does), so we don't need to worry about version, old version index will still get the same error, and new version index can proceed normally, but the downside is that the index storage size might inscrease a little bit, there comes the other options which do not increase index size.

[ajleong623]

@HUSTERGS @msfroh I made a temporary solution to the issue. It took me a while to understand how the mappers are applied and what actually happens during indexing and dig through the code behind mappers and parsers. But I found that when the document is indexed, there is an Iterable of IndexableField objects to represent the fields in the document. So for WildcardFieldMappers, I added a new field to be applied to the document which would make it so that the value of the field would be available in the index for comparison.

The issue mentioned above is that this would increase the storage, but the temporary solution does work. How else would the comparator linked in the issue work if the full values are not indexed? The draft pull request is linked.