Repository navigation
perf: linear-time toon_agg via C transition + set-based rows_to_toon - #11
Merged
Merged
Conversation
toon_agg was quadratic in row count from two independent causes:
1. The transition function. PostgreSQL flattens and re-expands SQL and
plpgsql transition state on every call, so ANY custom transition that
accumulates an array is O(n²) regardless of its body (measured: a
minimal 'state := state || x' plpgsql transition takes 17 s for 40k
rows; a SQL wrapper around array_append, 15 s). Only a C transition
function stays linear (pg_catalog.array_append: 92 ms for 160k).
2. The finalizer built the result with a per-row string concatenation
loop, quadratic on its own (8 s of the 39 s total at 40k rows).
The default-delimiter toon_agg(anycompatible) now collects rows with
pg_catalog.array_append (STYPE anycompatiblearray) and encodes once at
finalization through a new public function:
rows_to_toon(rows anyarray, delim text DEFAULT ',')
which serializes the array once via array_to_json and encodes all rows
in one set-based pass. It deliberately never subscripts the array in a
loop: flat varlena arrays have O(i) element access, so rows[i] over all
i is itself O(n²) — measured and avoided. It is LANGUAGE sql because
plpgsql cannot accept record[] from anonymous subquery rows, and STABLE
(not IMMUTABLE) because array_to_json's rendering is GUC-dependent.
NULL records (e.g. unmatched LEFT JOIN rows) are skipped in both
aggregate variants and in rows_to_toon, with [N] counting only encoded
rows. The previous implementation NULL-poisoned the whole result (2-arg)
or returned NULL for everything (mixed cases); tests pin the new
semantics. Non-record elements raise a clear error.
The two-argument toon_agg(anyelement, text) cannot use array_append (no
way to carry the delimiter), so it keeps a plpgsql transition (text[]
state, linear finalizer) and remains quadratic at the transition
boundary; README documents rows_to_toon(array_agg(q), delim) as the
linear escape hatch for large sets. The toon_agg_state composite type
is gone.
Measured, 2-column rows:
40k rows: 39.2 s -> 0.9 s
200k rows: >10 min -> 2-6 s
Output verified byte-identical against the previous implementation for
comma/pipe/tab across 5k mixed rows (quoted values, embedded newlines,
NULLs, float NaN, text 'NaN'), plus empty-set/single-row cases, and
independently fuzzed across 30+ record shapes by an adversarial review
pass. Suite grows to 77 assertions.
Fixes #4
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEYjgkWDYiRMYwwRS985g5
This was referenced Jul 13, 2026
Resolve conflicts with #8 (volatility), #9 (delimiter validation) and #10 (NaN docs), which landed on master first: - toon_agg_sfunc: keep this branch's text[] state, mark it STABLE per #8, and run #9's delimiter guard before the NULL-record skip so an invalid delimiter raises even when the row is NULL. - Volatility test: replace toon_agg_sfunc_default with the functions this branch adds (rows_to_toon, toon_agg_rows_ffunc, toon_delim_ok, toon_rows_ok). - AGENTS.md: list the new aggregate helpers in place of the removed toon_agg_state / sfunc_default entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014JAaWhjV4FjNThbcAAWksS
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
2 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.
Fixes #4.
The quadratic cost had two independent causes (the issue initially blamed only the finalizer; measurement showed the transition function was the bigger half):
state := state || xplpgsql = 17 s @ 40k rows; SQL wrapper aroundarray_append= 15 s). Only a C transition stays linear (pg_catalog.array_append: 92 ms @ 160k).Design: the default
toon_agg(anycompatible)collects rows withpg_catalog.array_append(STYPEanycompatiblearray) and encodes once at finalization via a new public functionrows_to_toon(anyarray, delim DEFAULT ',')— one set-based pass overjson_array_elements(array_to_json(rows)). (It never subscripts the array in a loop: flat varlena arrays have O(i) element access, sorows[i]over all i is itself O(n²) — measured.) It's LANGUAGE sql because plpgsql can't acceptrecord[], and STABLE becausearray_to_jsonis GUC-dependent.The 2-arg
toon_agg(q, delim)can't usearray_append(no third argument), so it keeps a plpgsql transition (text[]state, linear finalizer) and stays quadratic at the boundary; README documentsrows_to_toon(array_agg(q), '|')as the linear escape hatch and adds a Performance section. Thetoon_agg_statecomposite type is gone.Numbers (2-column rows): 40k: 39.2 s → 0.9 s. 200k: >10 min → 2–6 s.
Semantics change (deliberate, tested): NULL records (unmatched LEFT JOIN rows) are now skipped, with
[N]counting only encoded rows. Previously a NULL record NULL-poisoned the entire 2-arg result. Non-record elements raise a clear error.Verification: output byte-identical to the old implementation for comma/pipe/tab across 5k mixed rows (quoting, embedded newlines, NULLs, float NaN, text 'NaN') plus empty/single-row cases; independently fuzzed across 30+ record shapes (unicode, 22-col, domains, composites, arrays-as-columns, DISTINCT/FILTER/window/GROUP BY) by an adversarial review pass. Suite: 77/77.
Merge order: conflicts with #8 (volatility — function list in its test) and #9 (delimiter validation — sfunc guard). Suggest merging the small ones first; I'll rebase this on top (re-adding the delim guard to the rewritten sfunc and the new names to the volatility test).
🤖 Generated with Claude Code
https://claude.ai/code/session_01FEYjgkWDYiRMYwwRS985g5