fix(exec): release the input table of window, sort and limit queries - #604
Merged
Merged
Conversation
…ionally Every node the executor evaluates returns an owned reference — the constant table node included, it retains its literal. The OP_WINDOW, OP_SORT and OP_HEAD cases released their input only when it differed from the graph's table, taking an equal pointer for a borrowed one. A query's root is a constant node over that very table, so the reference was never released: one input table per windowed, sorted or limited query. Invisible while a global kept the table alive; a whole table per call when the input was built for that call, as a service that windowed a freshly concatenated buffer on a timer found (#602). The four cases now release the input on every path, as the join and the plain head/tail cases already did. Test: window, sorted and limited selects over a table built per call, measured with the two-window bytes-allocated method of the other memory probes, plus a shared input reused across fifty calls. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… queries Twenty calls of each shape over one global table; the table's refcount must be exactly what it was before, and the table still whole after. Fails on the unfixed executor at the first window shape. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The same borrowed-query-table guard sat under the reductions; no child evaluates to the query table there today, so nothing leaked, but the premise is the one the sort, window and limit cases just dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
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.
Bug
A windowed, sorted or limited query leaks its input table once per call.
Every node the executor evaluates returns an owned reference; the constant table node a query is rooted on is no exception, it retains its literal (
OP_CONST,src/ops/exec.c). TheOP_WINDOW,OP_SORTand bothOP_HEADbranches released their input only when it differed from the graph's table, taking an equal pointer for a borrowed reference. A query's root is a constant node over that very table, so the reference was never released.While the input is a global the leak is one reference on an object that stays alive anyway, so nothing grows. When the input is built for the call —
(select {from: (concat a b) asc: k}), a window over a projection, a limited select over a filtered table — the whole input table stays allocated after every call. A service that windowed a freshly concatenated buffer on a timer grew by one buffer per pass with every global flat (#602).Fix
The four cases release their input on every path, as the join and the plain head/tail cases already did; the reduction case, which carried the same guard without a reachable shape, is aligned with them. No result changes; a shared input keeps its lifetime.
Tests
test/rfl/mem/query_input_release.rfl: window, sorted and limited selects over a table built per call, with the two-windowbytes-allocatedmethod of the other memory probes, plus a shared input reused across fifty calls. Every probe fails on the unfixed executor.test/test_lang.clang/select/releases_input_table: twenty calls of each shape over one table; its refcount must be exactly what it was before. Fails on the unfixed executor at the first window shape.Fixes #602.