Skip to content

fix(group): table distinct declared zero-length agg VLAs on the hash path (#515) - #516

Merged
singaraiona merged 1 commit into
devfrom
fix/distinct-zero-agg-vla
Sep 11, 2026
Merged

singaraiona merged 1 commit into
devfrom
fix/distinct-zero-agg-vla

Conversation

@singaraiona

Copy link
Copy Markdown
Collaborator

What & why

A table distinct is executed as a grouped plan with no aggregates. Once the input is large enough to take the hash group path, exec_group_run declared agg_outs[n_aggs] / agg_cols[n_aggs] with n_aggs == 0. A zero-length VLA is undefined behaviour (C17 6.7.6.2p5); the release build tolerated it and answered correctly, but the default sanitized debug build aborted with variable length array bound evaluates to non-positive value 0.

The function already carries a vla_aggs (= max(n_aggs, 1)) guard for its other aggregate scratch. This sizes both VLAs by that guard. Every loop over them is bounded by n_aggs, so behaviour is unchanged for real aggregates. The other variable-bound VLAs in group.c were checked: key-sized ones use vla_keys, and the expression-input scratch sits behind an early return that fires when there are no aggregates.

Adds test/rfl/group/distinct_zero_agg_vla.rfl with the reported STR-key shape (100k rows / 9k distinct) plus SYM and two-column keys. Before the fix the sanitized test binary aborts on it; after, it passes and the full suite reports 3777/3777 with no UBSan runtime errors.

Closes #515

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

https://claude.ai/code/session_01UQRURGab9d9Fjma6uekEzY

… (n_aggs == 0) is not UB

A table `distinct` is executed as a grouped plan with no aggregates. Once the
input is large enough to take the hash group path, exec_group_run declared
`agg_outs[n_aggs]` / `agg_cols[n_aggs]` with n_aggs == 0. A zero-length VLA
is undefined behaviour; the release build tolerated it and answered
correctly, the default sanitized debug build aborted with "variable length
array bound evaluates to non-positive value 0".

Size both VLAs by the existing vla_aggs (= max(n_aggs, 1)) guard that the
function already uses for its other agg scratch. Every loop over them is
bounded by n_aggs, so behaviour is unchanged. Adds a regression .rfl with
the reported STR-key shape plus SYM and two-column keys.

Closes #515

Claude-Session: https://claude.ai/code/session_01UQRURGab9d9Fjma6uekEzY
@singaraiona
singaraiona merged commit ac0f7f8 into dev Sep 11, 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

1 participant