Skip to content

fix(string): a STR view keeps only the bytes it points at - #631

Merged
singaraiona merged 5 commits into
devfrom
fix/str-view-pool-retention
Sep 27, 2026
Merged

singaraiona merged 5 commits into
devfrom
fix/str-view-pool-retention

Conversation

@ser-vasilich

Copy link
Copy Markdown
Collaborator

Bug

The descriptor views introduced for substr and if over STR columns pinned more than they used. An if whose two sides come from different pools — including two columns of one table under a where: that keeps a few rows — copied both whole parent pools into the result (a 10k-row result out of 500k rows carried the two columns' 43 MB); the eager arm did the same for an unfiltered if. A substr with a per-row start or length retained the parent pool even when every result fitted inline and nothing pointed into it.

Fix

  • if builds a pool of exactly the chosen rows' bytes whenever the sides come from different pools, a pooled scalar is involved, or the rows keep a small share (under an eighth) of one shared pool. One shared pool whose rows keep most of it is still pointed into, so a derived-key expression over one column stays copy-free.
  • A substr view whose descriptors are all inline drops its pool reference; one that points into the pool stays a view.

test/rfl/strop/str_view_pool_compact.rfl pins the retained bytes through direct-bytes: an if under a where over two columns, over two substring views, with a pooled scalar side, the eager if against the two-pool sum, and an inline substr view outliving its table.

ser-vasilich and others added 3 commits September 26, 2026 22:55
The descriptor views introduced for substr and `if` over STR columns pinned
more than they used.  An `if` whose two sides came from different pools —
including two columns of one table under a where: that kept a few rows —
copied BOTH whole parent pools into the result (the compacted branch
inputs share the full column pool, so a 10k-row result out of 500k rows
carried the two columns' 43 MB); the eager arm did the same for an
unfiltered `if`.  A substr with a per-row start or length retained the
parent pool even when every result fitted inline and nothing pointed
into it.

`if` now builds a pool of exactly the chosen rows' bytes whenever the
sides come from different pools, a pooled scalar is involved, or the rows
keep a small share (under an eighth) of one shared pool; one shared pool
whose rows keep most of it is still pointed into as before, so a
derived-key expression over one column stays copy-free.  A substr view
whose descriptors are all inline drops its pool reference; one that
points into the pool stays a view (the column is alive anyway, and a
sibling `if` over two such views can then pick either side without a
copy).

test/rfl/strop/str_view_pool_compact.rfl pins the retained bytes through
direct-bytes: an `if` under a where over two columns, over two substring
views, with a pooled scalar side, the eager `if` against the two-pool
sum, and an inline substr view outliving its table.
@singaraiona

Copy link
Copy Markdown
Collaborator

The case labelled “eager if” in str_view_pool_compact.rfl still takes exec_if_selected: STR is excluded from the eager shortcut at pivot.c:892, and both column scans support selected evaluation. This leaves the separately changed eager two-pool offset/copy path without demonstrated regression coverage. Please add a case that demonstrably reaches that path (a scalar BOOL condition is one candidate), checking values after source release and pool retention with serial and multiple-worker execution; also correct the current eager-coverage label.

ser-vasilich and others added 2 commits September 27, 2026 21:58
The case labelled eager in str_view_pool_compact went through the
selected path (a per-row condition column is always a row mask there),
so the eager arm's two-pool offset/copy had no regression coverage.

A scalar condition (a variable) is not a row mask: the selected path
declines and the eager fill runs over two different pools.  New cases
take it at 500k rows (parallel fill) and 1000 rows (below the parallel
threshold, serial fill), for both sides, check every row after all the
sources are released, and bound the retained bytes to one side's pool.
On the pre-fix tree the retention checks fail.  The old case is
relabelled as the per-row selected path it is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@singaraiona
singaraiona merged commit 1ae2435 into dev Sep 27, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants