Skip to content

test(group): oracles for the fixes in #540 and the coverage gaps - #546

Merged
singaraiona merged 2 commits into
perf/grouping-engine-scalingfrom
test/pr540-fix-oracles
Sep 17, 2026
Merged

singaraiona merged 2 commits into
perf/grouping-engine-scalingfrom
test/pr540-fix-oracles

Conversation

@ser-vasilich

Copy link
Copy Markdown
Collaborator

What & why

Tests for #540, targeted at its branch because they pin behaviour that only exists there.

The four fixes this branch carries, each red on the merge-base cb6b0371 and green here (f92150bc):

  • group/topk_full_size_k.rfltop/bot with K ≥ the group's value count came back in heap order (bot v 1024 over [5 3 9 1 7][1 3 7 9 5]); sorted output is asserted, with and without nulls thinning the group, and on a 200k-row group for the parallel consumer. The null case is the "serial heap not heapified when nulls leave fewer than K values" fix seen from the query language.
  • group/count_distinct_str_many_groups.rflcount (distinct text) over 65536 groups with a four-word vocabulary crashed with SIGSEGV; every group's distinct count is checked against its row count.
  • group/list_key_count_distinct.rfl — LIST-typed keys with count (distinct v) beside a streaming aggregate failed with vec_new: … got LIST.

Coverage the review found missing (7f49fd2d):

  • group/first_last_parallel_oracle.rflfirst/last on a 280k-row group that the two-worker suite splits at the parallel grain, checked against the rows themselves rather than another grouping path, with a nullable column and a selection.
  • group/topk_k_limits.rfl — K admitted for 1 and 1024, rejected for 0, −1 and 1025, on a small table and past the parallel threshold.
  • test_wide_count_distinct gains a table whose distinct count depends on the key's parity, with expectations read from the emitted key column, so a count landing on the wrong key is detected; the uniform table asserts the same value for every group and cannot tell.

All six pass in the ASan runner on 19154617.

Checklist

  • PR targets dev (not master) — targets perf/grouping-engine-scaling on purpose; four of the tests fail or crash on dev
  • 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:49
Each test fails on the merge-base and passes here.

- top/bot with K >= the group's value count returned the partially filled
  heap in heap order; sorted output is now asserted, with and without nulls
  thinning the group, and on a group large enough for the parallel consumer.
- count (distinct STR) over 65536 groups with a four-word vocabulary crashed
  with SIGSEGV; every group's distinct count is checked against its row count.
- LIST-typed keys with count (distinct) beside a streaming aggregate failed
  with a type error building the key column.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ed count-distinct

- first/last on a 280k-row group that the two-worker suite splits, checked
  against the rows themselves, with a nullable column and a selection.
- top/bot K admitted for 1 and 1024 and rejected for 0, -1 and 1025, on a
  small table and past the parallel threshold.
- test_wide_count_distinct: a second table whose distinct count depends on
  the key's parity, so a count landing on the wrong key is detected; the
  uniform table could not tell.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@singaraiona
singaraiona merged commit d35d768 into perf/grouping-engine-scaling Sep 17, 2026
1 check 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