Skip to content

feat(query): aggregate and window answers independent of input order (#547, PR 1/3) - #642

Merged
adityamukho merged 1 commit into
v3from
feat/547-rules
Oct 20, 2026
Merged

adityamukho merged 1 commit into
v3from
feat/547-rules

Conversation

@adityamukho

Copy link
Copy Markdown
Collaborator

PR 1 of 3 for #547 (part of #473): aggregate and window rules on the executor. Spec: docs/superpowers/specs/2026-10-20-aggregate-window-engine-design.md (#640). Plan: docs/superpowers/plans/2026-10-20-547-aggregate-window.md.

No aggregate or window answer depends on the order rows reach it any more.

Changes

  • New src/query/datalog/aggregate.rs, shared by both engines (PRs 2–3 will use it):
    • value_order: one total order of values. Numbers compare exactly across Integer and Float, and NaN comes after every other number. After the numbers come strings, booleans, refs, keywords, and nil last.
    • Exact sums: integers add in an i128 and saturate once. Floats add into a fixed-point accumulator and round once, half-even.
    • Agg and Running: the accumulators for aggregates and windows. Type errors (INT-041, INT-043, INT-044) and dropped groups are decided once all rows are in, so the answer is the same whatever order the rows arrive in.
  • Aggregation: one pass into BTreeMap<Vec<Value>, Vec<Agg>>; groups finish in key order.
  • Windows:
    • partitions are keyed by Value's Ord, so all NaN keys share one partition;
    • rows sort by :order-by, then the other :find-named variables, then input position;
    • peers (rows that tie on the order key) share rank and the running value at the last peer, as in SQL.
  • PRS-082 (new): a window in an aggregate query may read only plain :find variables.
  • functions.rs: the built-in functions become an enum. AggState, WindowOps, apply_builtin_aggregate, value_lt, value_cmp, is_known and is_window_compatible are removed.

Changed existing tests

  • tests/window_functions_test.rs::mixed_aggregate_and_window_in_same_find and parser.rs::parse_mixed_aggregate_and_window_allowed used exactly the query PRS-082 now rejects (a window reading a variable that is not grouped). Both now use a grouping variable. New tests check the rejection.

Tests

  • New: 15 unit tests (aggregate/tests.rs), 8 integration tests (tests/aggregate_window_rules_test.rs), and 8 aggregate/window queries added to the plan-independence matrix.
  • Totals: 1616 tests, 1606 passing and 10 ignored. test-oracle is green.
  • Clippy (lib and --all-features) is clean, and so is the wasm browser build.

Not in this PR

Codex review (read-only, on the commit) found no issues.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PtCdzbJMSS1V8qEPaJ4jXz

…547)

PR 1 of #547 (rules on the executor): one accumulator module shared by
both engines, so no aggregate or window answer follows the plan's row order.

- aggregate.rs: exact sums (i128 integers saturated once; floats in a
  fixed-point accumulator rounded once), value_order (one total order of
  values), Agg / Running accumulators deciding errors at finish
- windows: partitions by Value Ord, sort by :order-by then the other
  :find-named variables then input position; peers share rank and the
  running value at the last peer
- PRS-082: a window in an aggregate query may read only plain :find
  variables
- functions.rs built-ins become an enum; value_lt, value_cmp,
  apply_builtin_aggregate, WindowOps, AggState removed

Plan: docs/superpowers/plans/2026-10-20-547-aggregate-window.md

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PtCdzbJMSS1V8qEPaJ4jXz
@adityamukho adityamukho added this to the v3.0.0 milestone Oct 20, 2026
@codecov

codecov Bot commented Oct 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.80488% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/query/datalog/aggregate.rs 97.00% 9 Missing ⚠️

📢 Thoughts on this report? Let us know!

@adityamukho
adityamukho merged commit 234cc3e into v3 Oct 20, 2026
25 checks passed
@adityamukho
adityamukho deleted the feat/547-rules branch October 20, 2026 08:04
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.

1 participant