Skip to content

fix(sort,query): radix key width across dispatch tasks; keep aggregate columns on zero groups - #545

Merged
singaraiona merged 2 commits into
devfrom
fix/group-sort-followups
Sep 17, 2026
Merged

singaraiona merged 2 commits into
devfrom
fix/group-sort-followups

Conversation

@ser-vasilich

Copy link
Copy Markdown
Collaborator

What & why

Two long-standing wrong-result bugs, each with a test that fails on dev and passes here.

asc/desc/xasc return an unsorted vector. compute_key_nbytes derives the radix key width by letting every dispatch task OR together keys[i] ^ keys[start] — the differences against the task's own first element — and covers cross-task differences with only nw-1 sampled rows, assuming workers own contiguous chunks. The pool dispatches 8192-row tasks, so a high byte that changes only between tasks is dropped from the key width and the sort silently orders by the low bytes alone. 200000 values where rows 40960..49151 carry bit 16 come back as [0 0 0 0 0 65536 0 …]; at -c 1 any nearly sorted vector past 65536 rows with one swap sorts by its low 16 bits. Present since the v2 import (4a3a0c7). Every task now diffs against keys[0], the reference the serial path already uses, which covers every bit exactly; the sampling loop goes. Sort throughput is unchanged on random input (10M rows on the pool 53.6 → 52.7 ms), and an already sorted 10M-row vector at -c 1 drops from 201 to 50 ms because the key width is now right.

A grouped select that matches no row loses its aggregate columns. The eval-fallback grouping paths create each result column on the first group: streaming aggregates allocate agg_vec from the first value's type, and nonagg_eval_per_group_core only creates its result inside the per-group loop. With zero groups both stay NULL, so a single-key select silently dropped those columns (17 requested, 2 returned) and a composite key failed with per-group projection evaluation failed. The expression is now probed once over an empty slice (projections) or the aggregate over an empty vector of the source type (streaming, at all three sites) and an empty column of that type is emitted; when the probe cannot run an empty LIST column stands in. aggr_unary_per_group_buf's I64 guess takes the same probe.

Tests: test/rfl/sort/asc_key_width.rfl, test/rfl/group/zero_groups_keep_columns.rfl. On this branch make test (ASan/UBSan) 3825/3825, make tsan-test 20/20.

Checklist

  • PR targets dev (not master)
  • Commits follow Conventional Commits (feat: / fix: / perf: / docs: / …)
  • make builds cleanly (no new warnings)
  • make test passes; tests added/updated for behaviour changes

ser-vasilich and others added 2 commits September 17, 2026 10:50
compute_key_nbytes lets each dispatch task OR together `keys[i] ^ keys[start]`,
the differences against the task's own first element, and only samples
nw-1 cross-task points on the assumption that workers own contiguous
chunks. The pool dispatches 8192-row tasks, so a high byte that changes only
between tasks — never inside one and never at a sampled row — is dropped from
the key width, and asc/desc/xasc silently sort by the low bytes alone.
200000 values where rows 40960..49151 carry bit 16 come back with 65536 in
the middle of the zeros; at -c 1 any nearly sorted vector past 65536 rows
with one swap sorts by its low 16 bits.

Every task now diffs against keys[0], the reference the serial path already
uses, which covers every bit exactly; the sampling loop goes.

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

The eval-fallback grouping paths build each result column lazily, on the
first group: the streaming aggregates allocate `agg_vec` from the first
value's type, and nonagg_eval_per_group_core only creates its result inside
the per-group loop. With zero groups both stay NULL, so a single-key select
silently dropped those columns (17 requested, 2 returned) and a composite
key failed with "per-group projection evaluation failed".

Probe the expression once over an empty slice (projections) or the
aggregate over an empty vector of the source type (streaming) and emit an
empty column of that type; when the probe cannot run, an empty LIST column
stands in. aggr_unary_per_group_buf's I64 guess takes the same probe.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@singaraiona
singaraiona merged commit 7176e03 into dev Sep 17, 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