box: fix key corruption when pagination by tuple is used#11677
Merged
box: fix key corruption when pagination by tuple is used#11677
Conversation
816a928 to
39860aa
Compare
3f063eb to
56e2601
Compare
56e2601 to
399e66b
Compare
e4a2989 to
140a6e8
Compare
xuniq
approved these changes
Jul 25, 2025
nshy
requested changes
Jul 31, 2025
2850445 to
d40fc25
Compare
nshy
approved these changes
Aug 11, 2025
Contributor
|
I'd also try to use new |
5a09174 to
636a4b9
Compare
Contributor
Author
Good idea! Done, right in the first commit. PTAL. |
A Lua helper `tuple_encode` encodes tuple on ibuf and returns its `rpos` and `wpos` as pointers to the tuple beginning and end. If the ibuf is not empty, the returned pointer to the tuple beginning will actually point to the previously encoded tuple. Now it doesn't break anything, but anyway such behavior is inconvinient, so let's fix this function and return pointer to the actual beginning of the encoded tuple. Along the way, rewrite `index:quantile()` in a simpler manner using the new function behavior. Part of #11221 NO_TEST=refactoring NO_CHANGELOG=refactoring NO_DOC=refactoring
Function `iterator_pos_set` works completely incorrectly when tuple is passed to the option `after`. Firstly, before extracting position, we call `ibuf:consume()` for the memory allocated before position (this method is actually ASAN poisoning) - that's not correct since consumed memory contains query key that will be used later. Let's remove this call. Secondly, new allocation on ibuf can lead to its reallocation, so pointers to old allocations can be invalidated. Let's set the pointers to the key after all allocations are done, just like in `index:quantile()`. What's about performance, `perf/lua/box_select.lua` shows no degradation. Closes #11221 Closes #11648 NO_DOC=bugfix
636a4b9 to
c829142
Compare
nshy
approved these changes
Aug 12, 2025
locker
approved these changes
Aug 13, 2025
Member
|
Cherry-picked to 3.3 and 3.4. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Function
iterator_pos_setworks completely incorrectly when tupleis passed to the option
after.Firstly, before extracting position, we call
ibuf:consume()forthe memory allocated before position (this method is actaully ASAN
poisoning) - that's not correct since consumed memory contains query key
that will be used later. Let's remove this call.
Secondly, new allocation on ibuf can lead to its reallocation, so
pointers to old allocations can be invalidated. Let's set the pointers
to the key after all allocations are done, just like in
index:quantile().What's about performance,
perf/lua/box_select.luashows no degradation.Closes #11221
Closes #11648