Buffer lean rows in ORDER BY and GROUP BY to bound memory#46
Open
platypii wants to merge 4 commits into
Open
Conversation
5a1d79a to
edc344f
Compare
Buffering operators must hold their whole input, but late materialization made each buffered row retain O(columns) cell closures over the ~150 bytes of actual data. For a 1M-row sort that was ~1GB peak for ~48 bytes/row of real values. Make the expression evaluator read columns from the pre-materialized `resolved` object first, falling back to lazy cells. ORDER BY can then buffer lean rows (resolved + an empty cells map for cached derived sort keys) and drop the per-column closures, rebuilding full cells one row at a time only at emit. 1M rows x 6 cols: peak heap ~1002MB -> ~489MB, ~5.0s -> ~2.5s. 2M rows now finish under a 1GB heap where master OOMs. Still O(N) in row count.
Apply the same lean-row buffering as ORDER BY to the hash aggregate, which buffered its whole input as full AsyncRows and additionally spread each group's cell closures into per-group context rows. A 1M-row GROUP BY hit ~2GB peak (and OOM'd under a 1GB heap). Buffer already-materialized rows as resolved + an empty cells map, dropping the per-column closures; the star projection and the HAVING/ORDER-BY context row now read base columns from `resolved`. Also skip building the context row entirely when neither HAVING nor a grouped ORDER BY needs it. 1M rows x 6 cols: peak heap ~2GB -> ~1GB and now fits a 1GB heap where master OOMs; ~3x faster from reduced GC. Still O(N) in row count.
edc344f to
2cda551
Compare
A correlated subquery containing a lateral UNNEST of an outer column (e.g. UNNEST(o.arr)) threw ColumnNotFoundError when the outer query had an ORDER BY or GROUP BY. Buffering reduces the outer row to its resolved values with an empty cells map, but mergeOuterRows built the merged outer row only from outerRow.cells, so the outer columns became unreadable. Rehydrate the outer row's cells from resolved in mergeOuterRows. Also apply cleanups: reuse asyncRow when rebuilding cells from resolved in aggregates, drop the redundant leanRows WeakSet in sort in favor of a resolved check, and stop projecting undefined into resolved for cells-only keys.
Centralize readCell, hasCell, cellThunk, and rowCells in dataSource.js so every consumer reads a row's cells the same way, transparently rebuilding from resolved for lean buffered rows. Also fixes a latent bug where the HAVING row spread a lean group row's empty cells, so GROUP BY ... HAVING on a base column could read undefined.
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.
Problem
Buffering operators (ORDER BY, GROUP BY) must hold their whole input in memory before they can emit. Late materialization means each buffered row is an
AsyncRowcarrying one cell closure per column, and those closures retain far more than the ~150 bytes of actual data. Measured on a streaming source (so the buffer is the only thing holding rows):Fix
Where a row is already fully materialized (
resolvedpresent), keep only the plain object and drop the per-column cell closures. The expression evaluator now reads columns fromresolvedfirst, falling back to lazy cells — safe because the planner only setsresolvedwhen it completely and faithfully mirrors the output columns (never for derived expressions).readCell/hasCellhelpers readresolved-first.resolved+ an empty cells map for cached derived sort keys), rebuilding full cells one row at a time only at emit.resolved; the context row is skipped entirely when neither HAVING nor a grouped ORDER BY needs it.DISTINCT was left as-is: it already streams and only retains a dedup key-set (~152 MB for 1M rows), which is inherent to deduplication, not closure overhead.
Results (1M rows × 6 cols)
Both remain O(N) in row count — this halves the constant factor, it does not remove the buffer. Truly unbounded inputs still need spill-to-disk (separate work).
Testing
npx tscclean,npm run lintclean