Skip to content

perf(select): filtered positional take stops early; sort by an unprojected column - #634

Open
ser-vasilich wants to merge 7 commits into
devfrom
perf/topk-early-stop
Open

ser-vasilich wants to merge 7 commits into
devfrom
perf/topk-early-stop

Conversation

@ser-vasilich

Copy link
Copy Markdown
Collaborator

Problem

  1. (select {… from: T where: <pred> take: K}) without an ordering evaluated the predicate over the whole table, gathered every passing row and cut K afterwards. take: -K did the same for the last rows (on a 10M-row table with a like filter it took 8.6 s for ten rows).
  2. (select {a: a from: T asc: c}) — a sort by a source column the projection does not output — failed with nyi whenever the fused top-k did not take the query (no take:, a take range, or a filter the fused path does not admit): the sort ran over the projected table and looked the key up there.
  3. alter … set wrote into a vector marked sorted and kept the marker.

Fix

  • Fused positional take. The table is walked in 64k-row chunks from the end the answer comes from, one pool task per chunk in that order; each worker keeps at most |K| passing row ids. A worker that holds |K| publishes the row beyond which nothing can be part of the answer and every later chunk returns at once. The lists are merged by row id and only the |K| rows are gathered. asc: key take: K on a column marked sorted (no nulls) takes the same path: its first K passing rows are the K smallest, ties in table order. Only a literal K is admitted, so a take: expression is still evaluated once.
  • Unprojected sort key. A sort key that names a source column no output carries is projected too and dropped from the result after the sort (by position). An output that is a bare scan of the key column under another alias counts as the key, so the output is kept as before.
  • alter … set clears the sorted marker on write.

Result

On a 10M-row table (48 threads):

query before after
where: (like URL "*google*") take: 10 251 ms 9.5 ms
where: (like URL "*google*") take: -10 8629 ms 14.7 ms
where: (== CounterID 62) take: 10 79 ms 0.34 ms

Results equal the filter-then-take and sort-then-take answers (checked over 1,080 predicate × K × column-type cases at 1 and 4 cores, nulls included).

Tests: test/rfl/fused/fused_take_early_stop.rfl, test/rfl/query/sort_key_not_projected.rfl.

ser-vasilich and others added 3 commits September 27, 2026 15:45
`(select {… from: T where: <pred> take: K})` with no ordering evaluated
the predicate over the whole table, gathered every passing row and cut
K afterwards; `take: -K` did the same for the last rows.

The fused path now answers it: the table is walked in 64k-row chunks
from the end the answer comes from, one pool task per chunk in that
order, each worker keeping at most |K| passing row ids.  A worker that
holds |K| publishes the row beyond which nothing can be part of the
answer, and every later chunk returns at once.  The lists are merged by
row id and only the |K| rows are gathered.  `asc: key take: K` on a
column marked sorted (no nulls) takes the same path: its first K passing
rows are the K smallest, ties in table order.

Test: fused/fused_take_early_stop (first and last K over many chunks,
AND predicates, fewer matches than K, none, all columns, aliases, K
larger than one worker's share, the sorted-key form).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`(select {a: a from: T asc: c})` failed with `nyi` whenever the sort did
not go through the fused top-k: the sort runs over the projected table
and looked the key up there.  A sort key that names a source column no
output carries is now projected too and dropped from the result after
the sort (an output with the same name is used as before).

Test: query/sort_key_not_projected (no filter, a filter the fused path
does not take, with and without take, two keys, computed outputs, a key
that is also an output).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rted

- An output that renames the key column (`{b: a … asc: a}`) is a bare
  scan the projection names by its source column; the key was added as
  a hidden column under the same name and the drop took the output with
  it.  Such a scan now counts as the key being present, and hidden keys
  are dropped by position (they are projected last).
- `alter … set` wrote into a vector marked sorted and kept the marker;
  the ascending take on a sorted key trusts it.  The marker is cleared
  on write.
- Without a sort, the positional take path admits only a literal K, so
  a `take:` expression is evaluated once, by the general path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@singaraiona singaraiona left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alias, _e0 collision and 17-key cases are fixed at 1832e07; the certified ASan/UBSan suite (3939/3939), release build and targeted TSan probes pass. One blocking error path remains in src/ops/query.c:11821–11826: if the table used to drop hidden sort keys cannot be allocated, the OOM is discarded and the original table is returned with extra columns. With (set OOMT (table [a b] (list [3 1 2] [0 2 1]))), deterministic failure of each of that constructor's three allocations makes (cols (select {z: (+ a 10) from: OOMT asc: b})) return [z b] rather than an error; alias b: a returns duplicate [b b]. Please propagate the stripping error through cleanup instead of returning the original result, and cover constructor failures with fault-injection regressions.

…planner

Projections resolve left to right: in `{b: a z: b …}` the output z reads
the alias b (column a), not the source column b.  The fused positional
take and the fused top-k gather source columns by name, so z came back
as column b — `take: 2` gave [30 10] instead of [3 1], and the sorted
top-k had the same wrong answer before this branch.  When an output
names an alias bound by an earlier output (other than itself), the
fused paths now decline and the planner resolves the projections.

Tests: fused/fused_take_early_stop (positive and negative take, sorted
take with and without a filter, an identity alias that stays fused).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ser-vasilich

Copy link
Copy Markdown
Collaborator Author

Fixed in 5724742. The fused take and fused top-k now decline when an output names an alias bound by an earlier output, so the planner resolves projections left to right. The sorted top-k had the same wrong answer on dev (asc: a take: 2 gave [10 20]); it is covered too. Added positive/negative literal-take and sorted-take regressions.

singaraiona and others added 3 commits September 27, 2026 19:43
…urce columns

The sort of a projection resolved its keys by NAME over the projected
table, whose columns are named by source column for scans and `_e<c>`
for expressions.  Three wrong outcomes followed:

- a hidden key whose source column is called `_e0` collided with the
  first computed output: `{z: (+ a 10) asc: _e0}` sorted by z;
- `{b: a asc: b}` treated the output alias b as the key, found no column
  b in the projection (that scan is named a) and failed with `nyi`;
- hidden keys were capped at 16, so a 17-key sort failed with `nyi`.

A sort key now names a SOURCE column, like where: and by:.  In the
planner it binds to the projected column that scans it — an output, or
the hidden key appended for it — and exec_sort reads a key that is one
of the SELECT's columns by position, so no name lookup can pick another
column.  Output aliases are not consulted.  The hidden-key capacity is
one slot per sort key name.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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