fix(sort,query): radix key width across dispatch tasks; keep aggregate columns on zero groups - #545
Merged
Merged
Conversation
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>
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.
What & why
Two long-standing wrong-result bugs, each with a test that fails on
devand passes here.asc/desc/xascreturn an unsorted vector.compute_key_nbytesderives the radix key width by letting every dispatch task OR togetherkeys[i] ^ keys[start]— the differences against the task's own first element — and covers cross-task differences with onlynw-1sampled 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 1any 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 againstkeys[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 1drops 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_vecfrom the first value's type, andnonagg_eval_per_group_coreonly 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 withper-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 branchmake test(ASan/UBSan) 3825/3825,make tsan-test20/20.Checklist
dev(notmaster)feat:/fix:/perf:/docs:/ …)makebuilds cleanly (no new warnings)make testpasses; tests added/updated for behaviour changes