-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Rollback the tmp storage of BytesRefHash to -1 after sort #13014
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@gf2121 I've added the 9.9.2 milestone to this PR. Do you agree? If so, is it possible to merge and backport to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gf2121 !
Thanks for finding/fixing this @gf2121!
Hmm -- that sounds buggy (on |
Thanks for the input! Wondering if perhaps I was too hasty closing out the naive #13019 change ... WDYT? |
+1 -- let's reopen? I would rather move towards making this API prevent such abuse than being lenient about it, if we can. Thanks @cpoerschke. |
In #12784 we cache buckets into extra slots in
BytesRefHash
to speed upBytesRefHash#sort
. This causes an AssertError forTermsQuery
because it could callsort
more than once on aBytesRefHash
instance. Not sure if any other users were relying on this but it would be great to keep consistent with before.