8000 fix(query): align ordinary aggregate value handling (#789) by adityamukho · Pull Request #792 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- **Ordinary average** (#777): `(avg ?v)` no longer raises PRS-027. Grouped and ungrouped queries use the same numeric population and arithmetic as window `avg`: skip nil and nonnumeric values, sum exactly, round the total once to Float, then divide by the numeric count. Integer totals are not saturated. Results are Float, or nil for an existing group without numbers; no matching rows yields no rows. The total can overflow before division, and division rounds separately. Existing window behavior is unchanged. Retained exact-sum allocations for ordinary `sum` and `avg` are now charged to the query memory limit.

- **Align ordinary aggregate value handling with windows** (#789, v3.0.0).
`sum` and `sum-distinct` skip nil and nonnumeric arguments; `min`/`max` compare
every non-nil type using the existing window total order, including mixed
Integer/Float values. Count variants retain their non-nil counting and
distinctness. Existing groups with no numeric sum arguments return Integer(0);
all-nil min/max groups remain omitted, and no-match query conventions remain
unchanged. INT-041/043/044 are deprecated for these built-ins. No v2 runtime
backport, window behavior, ordered-group eligibility, or storage change.

### Documentation

- Retire tracker #383 in favor of the v3.0.0 milestone and its release gates. Other ordinary/window value-handling differences are tracked in #789.
- Retire tracker #383 in favor of the v3.0.0 milestone and its release gates. Ordinary/window value-handling consistency is completed by #789.

### Performance

Expand Down
6 changes: 4 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,17 +218,19 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are

## Test Coverage

**2018 tests** (2001 passing, 17 ignored; unit + integration + doc). Engine answers once checked by the executor are frozen in `tests/answers/` (#551).
**2030 tests** (2013 active, 17 ignored; unit + integration + doc). Engine answers once checked by the executor are frozen in `tests/answers/` (#551).
See `docs/TEST_COVERAGE.md` for the full per-file breakdown.

The #760 deterministic storage suite adds three native unit tests in a separate `--cfg minigraf_shuttle` build, outside the default count. Run only `graph::storage::deterministic::` with that cfg; ordinary unit tests do not enter its scheduler. `deterministic-schedules.yml` runs relevant v3 PRs on Linux/macOS/Windows plus Linux MSRV, and larger nightly/manual searches with replay artifacts. The identical main workflow activates the v3 schedule after both PRs merge. See `docs/testing/deterministic-schedules.md` for scope, controls and findings.

**Testing conventions** — see the Testing Conventions section below before writing any tests.

On v3, ordinary `avg` skips nil/nonnumeric values and divides the exact rounded total by the numeric count, matching window `avg` (#777). It returns Float, or nil for an existing group with no numbers; no matching bindings yields no rows. Other aggregate consistency work is #789; ordered-avg optimization remains #781.
On v3, ordinary `avg` skips nil/nonnumeric values and divides the exact rounded total by the numeric count, matching window `avg` (#777). It returns Float, or nil for an existing group with no numbers; no matching bindings yields no rows. Sum and extrema follow the value rules from #789; ordered-avg optimization remains #781.

## Current Maintainer Context

On `v3`, ordinary sum/sum-distinct skip nonnumbers and min/max compare all non-nil types by the window total order (#789). Empty-query conventions and all-nil min/max group dropping are preserved; INT-041/043/044 are deprecated for these paths. Ordered Aggregate remains count/count-distinct only (#781 owns expansion). Ordinary avg shipped in #777.

On `v3`, expression `=` / `!=` resolves literal keywords once per operator (#774), including nested expressions and substituted prepared slots. Dynamic values do not populate the cache. This is a performance-only change, with no v2.x backport; see `docs/benchmarks/774-expression-keywords.md`.

**v2.0.5 released** (2026-10-11) — short-file open refused with STG-045 (#506), commit after a torn WAL entry no longer lost, WAL-007 (#513); backported rule-query `max_results` (#528), all-checkpointed WAL removal (#457) and `usize::MAX` close-time save (#476); weekly soak test (#392), real coverage gates (#396). **v2.0.4** (2026-10-08) — `BrowserDb` IndexedDB flush returns an error for an unreadable dirty page (#470); golden-file v7 corpus (#391) and SIGKILL committed-data crash test (#384). **v2.0.3** (2026-10-06) — data-integrity patch under the support policy: indexed queries with >65,535 uncheckpointed facts (#445), tx counter rewound on reopen next to an already-checkpointed WAL (#447). **v2.0.2** (2026-09-27) — last planned v2.x feature/fix release: query fixes (#297, #405, #407, #410), directory fsync (#389), point-query/attribute-scan/checkpoint perf mitigations (#323, #381, #315), CI policy checks (#395), support policy and tiers (#397, #399, #400). **v2.0.1** (2026-09-25) fixed index rebuild on open (#370). **v2.0.0** introduced kernel file locking (#317, #304), structured runtime error codes (#277), `OpenOptions` `#[non_exhaustive]`, and an MSRV of Rust 1.89. See `CHANGELOG.md` for the full rationale and release history.
Expand Down
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ On v3.0.0, an ascending value-only query such as `[:find ?v :where [?e :score ?v

On the unreleased v3 branch, selective in-memory ancestor queries can use entity probes instead of reading the whole edge attribute (#72 8000 3). This is a query-planning optimization; it adds no storage index or configuration.

On v3.0.0, ordinary `sum` and `sum-distinct` skip nonnumeric values. Ordinary
`min`/`max` compare every non-nil type using the window value order, including
mixed integers/floats (#789). Existing all-nil min/max groups are still omitted;
no matching rows still returns no rows for these aggregates. `count` and
`count-distinct` continue to count non-nil values of every type. No v2 runtime
change is included.

**Status**: See [ROADMAP.md](ROADMAP.md) for planned work and current direction.

The unreleased v3 branch also avoids resolving the same keyword literal on every row of an expression comparison such as `[(= ?entity :alice)]` (#774); see the [benchmark report](docs/benchmarks/774-expression-keywords.md).
Expand Down
4 changes: 3 additions & 1 deletion ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

**Philosophy**: Embedded graph memory for agents, mobile, and the browser — built on the SQLite approach: be boring, be reliable, be embeddable.

Ordinary `avg` is implemented for v3.0.0 (#777), matching window numeric filtering. Consistent handling for the other ordinary aggregates remains a v3.0.0 release requirement (#789).
Ordinary `avg` is implemented for v3.0.0 (#777), matching window numeric filtering. Ordinary sum and extrema also use window-compatible value handling (#789); their existing empty-group conventions are preserved.

Completed releases and their implementation details live in the [CHANGELOG](CHANGELOG.md). This document tracks planned, exploratory, and other future work.

Expand Down Expand Up @@ -96,6 +96,8 @@ Scope — format and storage integrity:

Scope — query API:

- Align ordinary aggregate value handling with windows (#789; implemented for v3.0.0): sum/sum-distinct skip nonnumbers; min/max accept all non-nil types and mixed numbers. Preserve empty-query and all-nil min/max group-drop behavior. Ordinary avg is #777; ordered-group expansion remains #781.

- Selective in-memory ancestor joins avoid building a whole-attribute value map (#723; implemented for eligible three-pattern recursive bodies). Populated committed generations keep AVET probes; general in-memory value joins remain unchanged.

- Optional `:distinct` result tuples, preserving bag output by default (#493; done on `v3`)
Expand Down
39 changes: 11 additions & 28 deletions docs/ERROR_REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,10 @@ with no `CodedError` anywhere in its chain.
| INT-038 | Header too short for a specific field | Internal |
| INT-039 | Invalid magic number | Internal |
| INT-040 | Function/predicate name already registered | Internal |
| INT-041 | sum: unsupported value type | Internal |
| INT-041 | sum: unsupported value type (deprecated) | Internal |
| INT-042 | min/max: no non-null values in group | Internal |
| INT-043 | Cannot compare values of different types | Internal |
| INT-044 | Aggregate: unsupported value type | Internal |
| INT-043 | Cannot compare values of different types (deprecated) | Internal |
| INT-044 | Aggregate: unsupported value type (deprecated) | Internal |
| INT-045 | Pending fact index out of bounds | Internal |
| INT-046 | Missing CommittedFactReader for a committed FactRef | Internal |
| INT-047 | into_backend: backend Arc has multiple owners | Internal |
Expand Down Expand Up @@ -4194,46 +4194,29 @@ previously registered user-defined function.

**Error text**: `sum: expected Integer, Float, or Null, got {}`

**Cause**: The built-in `sum` aggregate encountered a value in its input
group that is not an `Integer`, `Float`, or `Null`.

**Resolution**:
- Ensure the attribute being summed only ever holds numeric or null
values.
> **Deprecated in v3.0.0 (#789).** Ordinary `sum` and `sum-distinct` skip nil and all nonnumeric values. An existing group without numbers returns Integer(0); a query with no matching rows returns no rows. This code and its message remain reserved for compatibility; these built-in paths no longer emit it. In v2, the type restriction still applies.

### INT-042 min/max: no non-null values in group

**Error text**: `min/max: no non-null values in group`

**Cause**: The built-in `min`/`max` aggregate was applied to a group whose
values were all `Null`, leaving nothing to compare.

**Resolution**:
- Filter out rows with a null value before aggregating, or handle the
all-null-group case in your query.
**Behavior**: An ordinary `min`/`max` group containing only nil values has no
extremum and is omitted. On v3 this is an internal group-drop condition, not
an error returned by the query. A window instead emits nil for each affected
source row. Queries with no matching rows do not create a min/max group.
The code and message remain reserved; #789 preserves this behavior.

### INT-043 Cannot compare values of different types

**Error text**: `{}: cannot compare {} and {} values`

**Cause**: An aggregate that orders values (`min`, `max`) encountered two
values of different, non-comparable `Value` types within the same group.

**Resolution**:
- Ensure the attribute being aggregated holds a consistent value type
across all matching facts.
> **Deprecated in v3.0.0 (#789).** Ordinary `min`/`max` accept mixed types, including Integer/Float, using the same total value order as their window forms. This code and its message remain reserved for compatibility; these built-in paths no longer emit it. In v2, the type restriction still applies.

### INT-044 Aggregate: unsupported value type

**Error text**: `{}: expected Integer, Float, String, or Null, got {}`

**Cause**: An aggregate that requires an orderable scalar type (e.g. `min`,
`max`) encountered a value type it does not support (such as a `Boolean`
or `Ref`).

**Resolution**:
- Ensure the aggregated attribute only holds Integer, Float, String, or
Null values.
> **Deprecated in v3.0.0 (#789).** Ordinary `min`/`max` accept every non-nil Value variant, including Boolean, Ref, Keyword and Set. They use the window total order. This code and its message remain reserved for compatibility; these built-in paths no longer emit it. In v2, the type restriction still applies.

### INT-045 Pending fact index out of bounds

Expand Down
Loading
Loading
0