PhysicalTopN: Buffer-allocated StringHeap
#16770
Merged
Conversation
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
|
Will this already be shipped with v1.2.2 or only with v1.3? |
|
This currently targets v1.3 - but I think we can re-target this to v1.2.2 since this looks low risk enough. @lnkuiper can you retarget to the v1.2 branch? |
0cc429a to
e6a5a55
Compare
|
I think this is ready to go, the failed CI run can't find vcpkg on Windows. |
|
Thanks! |
krlmlr
added a commit
to duckdb/duckdb-r
that referenced
this pull request
Apr 8, 2025
`PhysicalTopN`: Buffer-allocated `StringHeap` (duckdb/duckdb#16770) Remove delta from extensions built on a nightly basis (vs 1.2-histrionicus) (duckdb/duckdb#16794)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This memory was not yet being taken into account by the buffer manager, which would cause us to go over the limit with larger TopN queries.
EDIT: I've also reduced the jemalloc decay delay to 1s as this reduces our memory footprint without affecting performance much at all.
EDIT2: I've made sure we also use the buffer allocator for sorted aggregate states, which makes sure we also throw an OOM exception for https://github.com/duckdblabs/duckdb-internal/issues/4131