From ef9d4dad4ed5adc72023c0bb38f5c2c475e586da Mon Sep 17 00:00:00 2001 From: Isaac Date: Wed, 7 Oct 2026 16:12:50 -0600 Subject: [PATCH 01/12] test(db): bound grouped aggregate work per change and choose the smallest exact group value Adds a work law for groupBy and include aggregates: one inserted or deleted member costs the same number of iterator steps at every group size. Revises the group-value law so members equal under query equality supply the value with the smallest exact value, checked in both arrival orders against an independent model. Co-authored-by: Isaac --- packages/db/tests/query/group-by-work.test.ts | 207 +++++++++++++++++- packages/db/tests/query/group-by.test.ts | 65 +++++- 2 files changed, 263 insertions(+), 9 deletions(-) diff --git a/packages/db/tests/query/group-by-work.test.ts b/packages/db/tests/query/group-by-work.test.ts index 00153d9c88..2878df49a6 100644 --- a/packages/db/tests/query/group-by-work.test.ts +++ b/packages/db/tests/query/group-by-work.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { createCollection } from '../../src/collection/index.js' -import { createLiveQueryCollection } from '../../src/query/index.js' +import { createLiveQueryCollection, eq } from '../../src/query/index.js' import { count } from '../../src/query/builder/functions.js' import { mockSyncCollectionOptions } from '../utils.js' @@ -37,8 +37,8 @@ describe(`group representative work`, () => { } finally { spy.mockRestore() } - // Group reduction still scans members. Encoding its stable input - // keys must scale with the delta, not the retained group size. + // Encoding the stable input keys scales with the delta, not the + // retained group size. expect(calls).toBeLessThanOrEqual(4) expect(grouped.toArray).toMatchObject([ { value: 1, count: size + (type === `insert` ? 1 : 0) }, @@ -51,3 +51,204 @@ describe(`group representative work`, () => { }, ) }) + +/** + * # Does one change re-read its whole group? + * + * Law: the work a grouped aggregate does for one inserted or deleted member + * does not depend on how many members the group already holds. This holds + * for a `groupBy` aggregate and for an aggregate inside an include, which is + * grouped by its correlation route. + * + * Authority: incremental view maintenance. A count changes by the delta's + * multiplicity, so re-reading every member is not needed for the result. + * + * Observation: the number of Map and Set iterator steps taken during one + * synchronous commit. A reduction that re-reads a group walks the group's + * value map, so its steps grow with the group. The counter observes all + * iteration in the commit, so any other per-member walk also fails the law. + * The checkpoint is the commit's return, where the published count must also + * equal the model's count. + */ +const mapIteratorPrototype = Object.getPrototypeOf(new Map().values()) +const setIteratorPrototype = Object.getPrototypeOf(new Set().values()) + +function countIteratorSteps(run: () => void): number { + const mapNext = mapIteratorPrototype.next + const setNext = setIteratorPrototype.next + let steps = 0 + mapIteratorPrototype.next = function (this: Iterator) { + steps++ + return mapNext.call(this) + } + setIteratorPrototype.next = function (this: Iterator) { + steps++ + return setNext.call(this) + } + try { + run() + } finally { + mapIteratorPrototype.next = mapNext + setIteratorPrototype.next = setNext + } + return steps +} + +const groupSizes = [10, 100, 1000, 5000] + +async function groupByCommitSteps(size: number): Promise> { + const source = createCollection( + mockSyncCollectionOptions<{ id: number; value: number }>({ + id: `group-reduce-work-${size}`, + getKey: (row) => row.id, + initialData: Array.from({ length: size }, (_, id) => ({ id, value: 1 })), + }), + ) + const grouped = createLiveQueryCollection((q) => + q + .from({ row: source }) + .groupBy(({ row }) => row.value) + .select(({ row }) => ({ value: row.value, count: count(row.id) })), + ) + try { + await grouped.preload() + return [`insert`, `delete`].map((type) => { + const steps = countIteratorSteps(() => { + source.utils.begin() + source.utils.write({ + type: type as `insert` | `delete`, + value: { id: size, value: 1 }, + }) + source.utils.commit() + }) + expect(grouped.toArray).toMatchObject([ + { value: 1, count: size + (type === `insert` ? 1 : 0) }, + ]) + return steps + }) + } finally { + await grouped.cleanup() + await source.cleanup() + } +} + +type Issue = { id: number; title: string } +type Comment = { id: number; issueId: number | null } + +function createCommentCount(size: number) { + const issues = createCollection( + mockSyncCollectionOptions({ + id: `include-reduce-issues-${size}-${Math.random()}`, + getKey: (row) => row.id, + initialData: [ + { id: 1, title: `one` }, + { id: 2, title: `two` }, + ], + }), + ) + const comments = createCollection( + mockSyncCollectionOptions({ + id: `include-reduce-comments-${size}-${Math.random()}`, + getKey: (row) => row.id, + initialData: Array.from({ length: size }, (_, id) => ({ + id, + issueId: 1, + })), + }), + ) + const counts = createLiveQueryCollection((q) => + q.from({ issue: issues }).select(({ issue }) => ({ + id: issue.id, + title: issue.title, + comments: q + .from({ comment: comments }) + .where(({ comment }) => eq(comment.issueId, issue.id)) + .select(({ comment }) => ({ n: count(comment.id) })), + })), + ) + const write = ( + collection: typeof issues | typeof comments, + type: `insert` | `update` | `delete`, + value: Issue | Comment, + ) => { + collection.utils.begin() + collection.utils.write({ type, value } as never) + collection.utils.commit() + } + const commentCount = (issueId: number): number | undefined => + ( + counts.get(issueId) as + { comments: { toArray: Array<{ n: number }> } } | undefined + )?.comments.toArray[0]?.n + return { issues, comments, counts, write, commentCount } +} + +describe(`grouped aggregate work per change`, () => { + it(`keeps a groupBy count's work independent of group size`, async () => { + const steps = await Promise.all(groupSizes.map(groupByCommitSteps)) + for (const sizeSteps of steps) expect(sizeSteps).toEqual(steps[0]) + }) + + it(`keeps an include count's work independent of group size`, async () => { + const steps: Array = [] + for (const size of groupSizes) { + const fixture = createCommentCount(size) + try { + await fixture.counts.preload() + steps.push( + countIteratorSteps(() => + fixture.write(fixture.comments, `insert`, { + id: size, + issueId: 1, + }), + ), + ) + expect(fixture.commentCount(1)).toBe(size + 1) + } finally { + await fixture.counts.cleanup() + await fixture.issues.cleanup() + await fixture.comments.cleanup() + } + } + for (const sizeSteps of steps) expect(sizeSteps).toEqual(steps[0]) + }) + + // The route representative no longer distinguishes members, so every + // member of one correlation route contributes the same value. These + // histories check that retraction, emptying, refilling, an unmatched + // correlation key, and a parent change still publish the model's count. + it(`keeps include counts exact through retraction and refill`, async () => { + const fixture = createCommentCount(3) + const { comments, issues, write, commentCount } = fixture + try { + await fixture.counts.preload() + expect(commentCount(1)).toBe(3) + // Retract the member that arrived first. + write(comments, `delete`, { id: 0, issueId: 1 }) + expect(commentCount(1)).toBe(2) + // Empty the group, then refill it. + write(comments, `delete`, { id: 1, issueId: 1 }) + write(comments, `delete`, { id: 2, issueId: 1 }) + expect(commentCount(1)).toBeUndefined() + write(comments, `insert`, { id: 0, issueId: 1 }) + write(comments, `insert`, { id: 5, issueId: 1 }) + expect(commentCount(1)).toBe(2) + // A null correlation key joins no issue. + write(comments, `insert`, { id: 6, issueId: null }) + expect(commentCount(1)).toBe(2) + expect(commentCount(2)).toBeUndefined() + // Move a member between routes. + write(comments, `update`, { id: 5, issueId: 2 }) + expect(commentCount(1)).toBe(1) + expect(commentCount(2)).toBe(1) + // A parent change keeps the child count. + write(issues, `update`, { id: 1, title: `renamed` }) + expect(fixture.counts.get(1)).toMatchObject({ title: `renamed` }) + expect(commentCount(1)).toBe(1) + } finally { + await fixture.counts.cleanup() + await issues.cleanup() + await comments.cleanup() + } + }) +}) diff --git a/packages/db/tests/query/group-by.test.ts b/packages/db/tests/query/group-by.test.ts index d736d8244b..98156bcb7f 100644 --- a/packages/db/tests/query/group-by.test.ts +++ b/packages/db/tests/query/group-by.test.ts @@ -257,6 +257,45 @@ const equalityEquivalentGroupValues: Array< ], ] +/** + * Which member supplies a group's projected value when several members are + * equal under query equality but differ exactly? + * + * Law: the member with the smallest exact value. Exact values are ordered by + * kind first: a number before an object (a Date, a binary array, or a + * Temporal value). Among numbers, an ordinary number comes before -0, and + * -0 before NaN. Among objects, the type name decides (`Buffer` before + * `Date` before `Uint8Array`). Objects of one type with the same content are + * the same exact value, so either instance may be projected. The choice does + * not depend on row keys or on the order in which rows arrived. + * + * Before this law, the member with the smallest row key supplied the value. + * That choice made every member's contribution distinct, so each change + * re-read the whole group. + */ +function exactRank(value: unknown): [number, number | string] { + if (typeof value === `number`) { + if (Object.is(value, -0)) return [0, 1] + if (Number.isNaN(value)) return [0, 2] + return [0, 0] + } + const typeName = + value !== null && typeof value === `object` + ? value.constructor.name + : typeof value + return [1, typeName] +} + +function smallestExact(values: ReadonlyArray): unknown { + return values.reduce((best, value) => { + const [kind, sub] = exactRank(value) + const [bestKind, bestSub] = exactRank(best) + return kind < bestKind || (kind === bestKind && sub < bestSub) + ? value + : best + }) +} + function representativeSignature(value: unknown): string { if (value instanceof Date) return `date` if (Buffer.isBuffer(value)) return `buffer` @@ -345,10 +384,24 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { test.each(equalityEquivalentGroupValues)( `groups %s by query equality`, (_name, createValues) => { - const [left, right] = createValues() + for (const reversed of [false, true]) { + checkEqualityGroup(createValues, reversed) + } + }, + ) + + function checkEqualityGroup( + createValues: () => readonly [unknown, unknown], + reversed: boolean, + ): void { + { + // Both arrival orders: the projected value must not depend on which + // member arrived first or which has the smaller row key. + const [first, second] = createValues() + const [left, right] = reversed ? [second, first] : [first, second] const valuesCollection = createCollection( mockSyncCollectionOptions<{ id: number; value: unknown }>({ - id: `equality-group-values-${autoIndex}`, + id: `equality-group-values-${autoIndex}-${reversed}`, getKey: (row) => row.id, initialData: [ { id: 1, value: left }, @@ -381,7 +434,7 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { ) } - expectSingleGroup(2, left) + expectSingleGroup(2, smallestExact([left, right])) valuesCollection.utils.begin() valuesCollection.utils.write({ @@ -397,9 +450,9 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { value: { id: 1, value: left }, }) valuesCollection.utils.commit() - expectSingleGroup(2, left) - }, - ) + expectSingleGroup(2, smallestExact([left, right])) + } + } test.each([ `__group_value_0`, From 9b021bbdcae98b87b0c46ee8f6f7b5dc707742ec Mon Sep 17 00:00:00 2001 From: Isaac Date: Wed, 7 Oct 2026 16:12:50 -0600 Subject: [PATCH 02/12] perf(db): consolidate group representatives so a change does not re-read its group A group representative carried the member's row key, so every contribution was distinct and each change re-read the whole group. The correlation route representative now carries no row key, since equal route identities mean equal routes. The groupBy value representative orders by exact value, with Dates, binary arrays and Temporal values compared by type and content. Co-authored-by: Isaac --- .../group-representative-exact-value.md | 7 ++ docs/contributing/oracle-coverage.md | 7 ++ .../group-representative-exact-value.md | 103 ++++++++++++++++++ packages/db/src/query/compiler/group-by.ts | 34 +++++- packages/db/src/query/live/ARCHITECTURE.md | 13 ++- 5 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 .changeset/group-representative-exact-value.md create mode 100644 docs/contributing/oracle-reviews/group-representative-exact-value.md diff --git a/.changeset/group-representative-exact-value.md b/.changeset/group-representative-exact-value.md new file mode 100644 index 0000000000..78019b46a6 --- /dev/null +++ b/.changeset/group-representative-exact-value.md @@ -0,0 +1,7 @@ +--- +'@tanstack/db': patch +--- + +Update a grouped aggregate in work that does not depend on the group's size. Before, each change to a `groupBy` group or to an aggregate inside an include re-read every member of the group, because each member's contribution carried its row key. For example, an include that counts the comments of an issue re-read all of that issue's comments on each new comment. + +A `groupBy` value now comes from the member with the smallest exact value, when several members are equal under query equality but differ exactly. A number comes before a Date, a binary array, or a Temporal value; an ordinary number comes before `-0`, and `-0` before `NaN`; binary arrays are ordered by type name, so a `Buffer` comes before a `Uint8Array` with the same bytes. Before, the member with the smallest row key supplied the value. For example, a group that holds `new Date(0)` and `0` now projects `0`, and a group that holds `-0` and `0` now projects `0`. Values that are not equal under query equality are in different groups and do not change. diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index a4ae6dca48..32eaa4182c 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -411,6 +411,13 @@ or during that deferral. The includes publication owner still needs a compiled includes witness for cleanup during a discarded source deferral, followed by a parent or child publication at its callback boundary. +`packages/db/tests/query/group-by-work.test.ts` owns the work law for grouped +aggregates: a change costs the same number of iterator steps at every group +size, for a `groupBy` count and for an include count. `group-by.test.ts` +checks which member supplies the value of a group whose members are equal +under query equality but differ exactly, in both arrival orders +([review](oracle-reviews/group-representative-exact-value.md)). + The [Temporal group-key oracle](https://github.com/TanStack/db/blob/main/packages/db-ivm/tests/temporal-group-key-oracle.test.ts) owns the db-ivm `groupBy` value boundary for the eight Temporal kinds recognized by structural hashing. It compares public group counts with fixture-defined diff --git a/docs/contributing/oracle-reviews/group-representative-exact-value.md b/docs/contributing/oracle-reviews/group-representative-exact-value.md new file mode 100644 index 0000000000..ed0f8721e2 --- /dev/null +++ b/docs/contributing/oracle-reviews/group-representative-exact-value.md @@ -0,0 +1,103 @@ +# Group representative work and exact-value choice review + +Evidence by revision, on `perf-aggregate-representatives` from `origin/main` +`ea51b67b0`: the RED results ran on `ea51b67b0` with the new tests, and the +GREEN and mutant results ran on the branch's production change. + +## Laws and authority + +1. **Work law.** The work that a grouped aggregate does for one inserted or + deleted member does not depend on the number of members that the group + holds. The law applies to a `groupBy` aggregate and to an aggregate inside + an include, which the compiler groups by its correlation route. Authority: + incremental view maintenance. A count changes by the multiplicity of the + delta, so the result does not need a re-read of each member. +2. **Route law.** One representative carries the whole correlation route. + The route has two fields, `correlationKey` and `parentContext`. Its exact + identity is the exact identity of the correlation key and the parent + context identity. The parent context identity contains the exact identity + of each projected parent value. Thus two members with equal route identities + have the same correlation key and parent values, and the route needs no row + key to select one member. +3. **Group-value law (revised by a product decision).** When several members + are equal under query equality but differ exactly, the projected value comes + from the member with the smallest exact value: + - a number before an object (a Date, a binary array, or a Temporal value); + - an ordinary number before `-0`, and `-0` before `NaN`; + - objects by type name, so `Buffer` before `Date` before `Uint8Array`. + + Objects of one type and content are one exact value, so either instance can + be projected. The previous law selected the member with the smallest row key. + +## Old and new predictions + +The six cases in `group-by.test.ts` (`groups %s by query equality`, autoIndex +off and eager). Each case observes three checkpoints: both members present, +after the delete of row 1, and after the reinsert of row 1. Arrival order is +row 1, then row 2. + +| Members (row 1, row 2) | Old prediction | New prediction | +| --- | --- | --- | +| `Date(0)`, `0` | Date, `0`, Date | `0`, `0`, `0` | +| invalid Date, `NaN` | Date, `NaN`, Date | `NaN`, `NaN`, `NaN` | +| `-0`, `0` | `-0`, `0`, `-0` | `0`, `0`, `0` | + +The test now also runs each case with the members in reversed order. A +`Buffer` and a `Uint8Array` with the same bytes passed under the old code only +in the original order. In the reversed order, the old code projected the +`Uint8Array`. + +## Evidence + +- **Work law.** `group-by-work.test.ts` counts the Map and Set iterator steps + in one synchronous commit, at 10, 100, 1,000 and 5,000 members. On + `ea51b67b0`, a `groupBy` count took 157 steps at 100 members against 67 at 10 + members, and an include count took 219 against 129. On the branch, each count + takes the same number of steps at every size: 55 and 117 in the probe runs. + The published count is also checked at each size. +- **Route histories.** The same file deletes the member that arrived first, + empties and refills a route, adds a null correlation key, moves a member + between routes, and updates the parent. Each history checks the published + count. The histories pass on `ea51b67b0` and on the branch, because the + change keeps the result and changes only the work. +- **Group-value law.** The test model `smallestExact` orders values by kind and + type name without production code. On `ea51b67b0`, 8 cells fail. For + example, `expected 'date' to be 'number'`. + +## Mutants + +| Mutant | Outcome | +| --- | --- | +| Row key restored in the route representative | Assertion failure: the include work law (219 against 129) | +| Row key restored in the `groupBy` representative | Assertion failure: the `groupBy` work law and 6 group-value cells | +| Largest exact value selected | Assertion failure in 8 group-value cells | +| First member that arrived selected | Assertion failure in 8 group-value cells | + +## ORC-012 requirement audit + +| Requirement | Outcome | +| --- | --- | +| ORC-001 | Applicable. The three laws and their authority are above. The group-value law is a product decision of this change. | +| ORC-002 | Applicable. The expected counts come from the fixture sizes. The expected group value comes from the test model `smallestExact`, which does not use production identity or serialization. | +| ORC-003 | Applicable. Both test files state each law, its observation and its checkpoint before the assertions. | +| ORC-004 | Applicable to the finite matrices only. No generated grammar is claimed. The sizes 10, 100, 1,000 and 5,000 bound the work law. | +| ORC-005 | Applicable. The tests use the public live-query Collection and its published rows at the return of the commit. | +| ORC-006 | Applicable. The four mutants above fail at the intended checkpoints. | +| ORC-007 | Inapplicable. These tests are finite, not generated properties. | +| ORC-008 | Inapplicable. No stateful reference model changes. | +| ORC-009 | Applicable. "Iterator step" is a test observation, not a production concept. "Exact value" is the value relation that `ValueIdentity.exact` keeps, with Dates, binary arrays and Temporal values compared by type and content. | +| ORC-010 | Applicable, with a gap. Cleanup runs in `finally` blocks, so a cleanup error can replace an assertion error. | +| ORC-011 | Inapplicable. No shared fault was named. | +| ORC-013 | Applicable. The work law is a scaling law. Four sizes over two orders of magnitude separate a constant cost from a cost per member. | +| ORC-014 | Inapplicable. No controlled provider supplies a premise. | + +## Limits + +- The iterator-step counter observes Map and Set iteration only. A re-read + through array iteration would not count. The reduce operator reads its group + through a Map, so the counter reaches the regression. +- A `groupBy` over values whose exact identity is a reference, such as plain + objects, still has one contribution per distinct object. Equality for those + values is also by reference, so each such group holds one value. +- The fixed per-change overhead from #1740 is a separate cost and is outside + this change. diff --git a/packages/db/src/query/compiler/group-by.ts b/packages/db/src/query/compiler/group-by.ts index ce4c485c5d..95f8d268fd 100644 --- a/packages/db/src/query/compiler/group-by.ts +++ b/packages/db/src/query/compiler/group-by.ts @@ -13,6 +13,8 @@ import { isExpressionLike, } from '../ir.js' import { isRefProxy } from '../builder/ref-proxy-identity.js' +import { isTemporal } from '../../utils.js' +import { isUint8Array, normalizeValue } from '../../utils/comparison.js' import { AggregateFunctionNotInSelectError, NonAggregateExpressionNotInGroupByError, @@ -114,18 +116,33 @@ function attachPublicGroupKey( } function createRepresentative( - rowKey: string, value: T, identity: unknown, ): Representative { // Encode once per contribution, not once per member on every group change. + // Members with equal identities consolidate, so a change does not re-read + // the group. const representative = { - key: serializeValue([rowKey, identity]), + key: serializeValue(identity), } as Representative Object.defineProperty(representative, RAW_REPRESENTATIVE, { value }) return representative } +/** + * Members equal under query equality supply the value with the smallest exact + * identity. A number sorts before an object, and an object of one type and + * content is one exact value, whichever instance supplies it. + */ +function exactValueIdentity( + value: unknown, + valueIdentity: ValueIdentity, +): unknown { + return value instanceof Date || isTemporal(value) || isUint8Array(value) + ? [`object`, value.constructor.name, normalizeValue(value)] + : valueIdentity.exact(value) +} + function getRepresentative( values: Array<[Representative, number]>, ): Representative | undefined { @@ -171,9 +188,11 @@ function addCorrelationRouteAggregate( valueIdentity: ValueIdentity, ): void { aggregates[fields.route] = { - preMap: ([rowKey, row]: [string, NamespacedRow]) => { + preMap: ([, row]: [string, NamespacedRow]) => { const route = getNamespacedRouteMetadata(row, mainSource) - return createRepresentative(rowKey, route, [ + // Equal route identities mean equal routes, so members need no row + // key and consolidate into one contribution. + return createRepresentative(route, [ valueIdentity.exact(route?.correlationKey), getParentContextIdentity(route?.parentContext), ]) @@ -379,9 +398,12 @@ export function processGroupBy( for (let i = 0; i < compiledGroupByExpressions.length; i++) { const compiledExpr = compiledGroupByExpressions[i]! aggregates[fields.groupValues[i]!] = { - preMap: ([rowKey, row]: [string, NamespacedRow]) => { + preMap: ([, row]: [string, NamespacedRow]) => { const value = compiledExpr(row) - return createRepresentative(rowKey, value, valueIdentity.exact(value)) + return createRepresentative( + value, + exactValueIdentity(value, valueIdentity), + ) }, reduce: getRepresentative, postMap: unwrapRepresentative, diff --git a/packages/db/src/query/live/ARCHITECTURE.md b/packages/db/src/query/live/ARCHITECTURE.md index e60216b58e..8e5ba65881 100644 --- a/packages/db/src/query/live/ARCHITECTURE.md +++ b/packages/db/src/query/live/ARCHITECTURE.md @@ -225,10 +225,15 @@ identity and opaque public group keys keep their own runtime scope because equivalent query plans and retained public keys must survive graph replacement. For grouping, the equality token is the D2 group key. The group retains a raw value from a currently positive contributor only as the projected -representative. The representative is chosen by stable source-row identity, so -restoring the same source state restores the same value regardless of update -history. D2 sees only safe exact-value identity for that representative, not -the raw value itself. A separate public group key preserves primitive keys and +representative. The representative is the contributor with the smallest exact +value: a number before an object (a Date, a binary array, or a Temporal value); +an ordinary number before -0, and -0 before NaN; objects by type name. Objects +of one type and content are one exact value, so either instance may be +projected. The choice does not depend on row keys or on update history, and +contributors with one exact value consolidate in D2, so a change does not +re-read the group. A correlated include's route representative also carries +no row key: equal route identities mean equal routes. D2 sees only safe +exact-value identity for a representative, not the raw value itself. A separate public group key preserves primitive keys and serializes opaque equality identity; graph-local identity tokens never cross the Collection boundary. Compiler group fields use a query-local namespace disjoint from every selected alias. Direct correlated joins canonicalize both From 871bdea11b20b48879b2614f6aeadd30694c0de5 Mon Sep 17 00:00:00 2001 From: Isaac Date: Wed, 7 Oct 2026 16:55:22 -0600 Subject: [PATCH 03/12] test(db): require projected and min/max values from positive members; bound array walks and binary encoding The group-value oracle compared only a type label, so a merged contribution that projected a deleted member's instance passed. It now requires the projected value to be an instance a positive member holds, and adds two Date instances with equal time. New min/max cases require the value of the remaining member after -0/0 and equal-Date merges. The work counter also counts array iteration and array callbacks, and a large binary group value may be encoded at most once. Co-authored-by: Isaac --- packages/db/tests/query/group-by-work.test.ts | 111 +++++++++++++++--- packages/db/tests/query/group-by.test.ts | 61 +++++++++- 2 files changed, 150 insertions(+), 22 deletions(-) diff --git a/packages/db/tests/query/group-by-work.test.ts b/packages/db/tests/query/group-by-work.test.ts index 2878df49a6..78f20c2d60 100644 --- a/packages/db/tests/query/group-by-work.test.ts +++ b/packages/db/tests/query/group-by-work.test.ts @@ -63,33 +63,66 @@ describe(`group representative work`, () => { * Authority: incremental view maintenance. A count changes by the delta's * multiplicity, so re-reading every member is not needed for the result. * - * Observation: the number of Map and Set iterator steps taken during one - * synchronous commit. A reduction that re-reads a group walks the group's - * value map, so its steps grow with the group. The counter observes all - * iteration in the commit, so any other per-member walk also fails the law. + * Scope: the law holds when the group's members contribute exactly identical + * aggregate inputs, as a count over primitive group values does. A sum, avg, + * min, or max over distinct values keeps one contribution per distinct exact + * input, so its reduction still visits each of them. + * + * Observation: the number of Map, Set, and array iterator steps, plus array + * callback calls (`map`, `filter`, `forEach`, `reduce`, `some`, `every`), + * taken during one synchronous commit. A reduction that re-reads a group + * walks the group's values, so its steps grow with the group. The counter + * observes all such iteration in the commit, so any other per-member walk + * also fails the law. * The checkpoint is the commit's return, where the published count must also * equal the model's count. */ -const mapIteratorPrototype = Object.getPrototypeOf(new Map().values()) -const setIteratorPrototype = Object.getPrototypeOf(new Set().values()) +const iteratorPrototypes = [ + Object.getPrototypeOf(new Map().values()), + Object.getPrototypeOf(new Set().values()), + Object.getPrototypeOf([][Symbol.iterator]()), +] as Array<{ next: (this: Iterator) => IteratorResult }> +const arrayCallbackMethods = [ + `map`, + `filter`, + `forEach`, + `reduce`, + `some`, + `every`, +] as const function countIteratorSteps(run: () => void): number { - const mapNext = mapIteratorPrototype.next - const setNext = setIteratorPrototype.next let steps = 0 - mapIteratorPrototype.next = function (this: Iterator) { - steps++ - return mapNext.call(this) - } - setIteratorPrototype.next = function (this: Iterator) { - steps++ - return setNext.call(this) - } + const nexts = iteratorPrototypes.map((prototype) => prototype.next) + const arrayMethods = arrayCallbackMethods.map( + (name) => Array.prototype[name] as (...args: Array) => unknown, + ) + iteratorPrototypes.forEach((prototype, index) => { + const next = nexts[index]! + prototype.next = function (this: Iterator) { + steps++ + return next.call(this) + } + }) + arrayCallbackMethods.forEach((name, index) => { + const method = arrayMethods[index]! + ;(Array.prototype as any)[name] = function ( + this: Array, + ...args: Array + ) { + steps += this.length + return method.apply(this, args) + } + }) try { run() } finally { - mapIteratorPrototype.next = mapNext - setIteratorPrototype.next = setNext + iteratorPrototypes.forEach((prototype, index) => { + prototype.next = nexts[index]! + }) + arrayCallbackMethods.forEach((name, index) => { + ;(Array.prototype as any)[name] = arrayMethods[index] + }) } return steps } @@ -213,6 +246,48 @@ describe(`grouped aggregate work per change`, () => { for (const sizeSteps of steps) expect(sizeSteps).toEqual(steps[0]) }) + // The group's representative must not serialize a large binary group + // value's bytes. The group key still encodes them once; that cost predates + // this law and is tracked in the review record. + it(`encodes a large binary group value's bytes at most once`, async () => { + const bytes = new Uint8Array(1024 * 1024) + const source = createCollection( + mockSyncCollectionOptions<{ id: number; value: Uint8Array }>({ + id: `group-binary-work-${Math.random()}`, + getKey: (row) => row.id, + initialData: [{ id: 1, value: bytes }], + }), + ) + const grouped = createLiveQueryCollection((q) => + q + .from({ row: source }) + .groupBy(({ row }) => row.value) + .select(({ row }) => ({ value: row.value, count: count(row.id) })), + ) + try { + await grouped.preload() + const stringify = JSON.stringify + let fullEncodings = 0 + JSON.stringify = function (...args: Parameters) { + const result = stringify.apply(JSON, args) + if (result.length >= bytes.length) fullEncodings++ + return result + } as typeof stringify + try { + source.utils.begin() + source.utils.write({ type: `insert`, value: { id: 2, value: bytes } }) + source.utils.commit() + } finally { + JSON.stringify = stringify + } + expect(grouped.toArray[0]?.count).toBe(2) + expect(fullEncodings).toBeLessThanOrEqual(1) + } finally { + await grouped.cleanup() + await source.cleanup() + } + }) + // The route representative no longer distinguishes members, so every // member of one correlation route contributes the same value. These // histories check that retraction, emptying, refilling, an unmatched diff --git a/packages/db/tests/query/group-by.test.ts b/packages/db/tests/query/group-by.test.ts index 98156bcb7f..ec98cc38a7 100644 --- a/packages/db/tests/query/group-by.test.ts +++ b/packages/db/tests/query/group-by.test.ts @@ -229,6 +229,7 @@ const equalityEquivalentGroupValues: Array< [`a Date and its timestamp`, () => [new Date(0), 0]], [`an invalid Date and NaN`, () => [new Date(Number.NaN), Number.NaN]], [`signed zero`, () => [-0, 0]], + [`two Date instances with the same time`, () => [new Date(0), new Date(0)]], [ `binary values with the same bytes`, () => [Buffer.from([1, 2, 3]), new Uint8Array([1, 2, 3])], @@ -269,6 +270,10 @@ const equalityEquivalentGroupValues: Array< * the same exact value, so either instance may be projected. The choice does * not depend on row keys or on the order in which rows arrived. * + * Law: the projected value is an instance that a currently positive member + * holds. After a member is deleted, its instance is never projected, even + * when a remaining member holds an equal instance. + * * Before this law, the member with the smallest row key supplied the value. * That choice made every member's contribution distinct, so each change * re-read the whole group. @@ -426,15 +431,21 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { const expectSingleGroup = ( expectedCount: number, representative: unknown, + members: ReadonlyArray, ) => { expect(summary.toArray).toHaveLength(1) expect(summary.toArray[0]?.count).toBe(expectedCount) - expect(representativeSignature(summary.toArray[0]?.value)).toBe( + const projected = summary.toArray[0]?.value + expect(representativeSignature(projected)).toBe( representativeSignature(representative), ) + // The projected instance belongs to a positive member. + expect(members.some((member) => Object.is(member, projected))).toBe( + true, + ) } - expectSingleGroup(2, smallestExact([left, right])) + expectSingleGroup(2, smallestExact([left, right]), [left, right]) valuesCollection.utils.begin() valuesCollection.utils.write({ @@ -442,7 +453,7 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { value: { id: 1, value: left }, }) valuesCollection.utils.commit() - expectSingleGroup(1, right) + expectSingleGroup(1, right, [right]) valuesCollection.utils.begin() valuesCollection.utils.write({ @@ -450,10 +461,52 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { value: { id: 1, value: left }, }) valuesCollection.utils.commit() - expectSingleGroup(2, smallestExact([left, right])) + expectSingleGroup(2, smallestExact([left, right]), [left, right]) } } + test.each([ + [`signed zero`, () => [0, -0] as const], + [`Date instances with the same time`, () => [new Date(0), new Date(0)]], + ])( + `min and max return a value a remaining member holds for %s`, + (_name, createValues) => { + const [left, right] = createValues() + const valuesCollection = createCollection( + mockSyncCollectionOptions<{ id: number; group: number; value: any }>({ + id: `minmax-exact-${autoIndex}-${_name}`, + getKey: (row) => row.id, + initialData: [ + { id: 1, group: 1, value: left }, + { id: 2, group: 1, value: right }, + ], + autoIndex, + }), + ) + const summary = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ row: valuesCollection }) + .groupBy(({ row }) => row.group) + .select(({ row }) => ({ + group: row.group, + low: min(row.value), + high: max(row.value), + })), + }) + valuesCollection.utils.begin() + valuesCollection.utils.write({ + type: `delete`, + value: { id: 1, group: 1, value: left }, + }) + valuesCollection.utils.commit() + // Only `right` remains, so both aggregates must return it exactly. + expect(Object.is(summary.toArray[0]?.low, right)).toBe(true) + expect(Object.is(summary.toArray[0]?.high, right)).toBe(true) + }, + ) + test.each([ `__group_value_0`, `__key_0`, From e5449edc262ca854c1a8436720cd16e3e4d9f6ea Mon Sep 17 00:00:00 2001 From: Isaac Date: Wed, 7 Oct 2026 16:55:26 -0600 Subject: [PATCH 04/12] fix(db): keep exactly different group contributions apart D2 consolidates contributions whose hashes match, and the hash treats -0 as 0 and equal Dates as one value. Without the row key, such contributions merged, and the merge kept the latest record, so min, max, or the projected group value could come from a deleted row. Each contribution now carries the exact identity of every value it can supply: the representative's instance and every sum, avg, min, or max input. The route representative uses the parent context instance. The exact-value order uses explicit type tags instead of constructor names, and no longer serializes binary contents. Co-authored-by: Isaac --- .../group-representative-exact-value.md | 4 +- packages/db/src/query/compiler/group-by.ts | 68 +++++++++++++------ packages/db/src/query/live/ARCHITECTURE.md | 24 ++++--- 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/.changeset/group-representative-exact-value.md b/.changeset/group-representative-exact-value.md index 78019b46a6..6b03b3db47 100644 --- a/.changeset/group-representative-exact-value.md +++ b/.changeset/group-representative-exact-value.md @@ -2,6 +2,6 @@ '@tanstack/db': patch --- -Update a grouped aggregate in work that does not depend on the group's size. Before, each change to a `groupBy` group or to an aggregate inside an include re-read every member of the group, because each member's contribution carried its row key. For example, an include that counts the comments of an issue re-read all of that issue's comments on each new comment. +Update a grouped count without re-reading the group when the group's members contribute identical inputs. Before, each change to a `groupBy` group or to a count inside an include re-read every member of the group, because each member's contribution carried its row key. For example, an include that counts the comments of an issue re-read all of that issue's comments on each new comment. A `sum`, `avg`, `min`, or `max` over distinct values still visits one contribution for each distinct exact input. -A `groupBy` value now comes from the member with the smallest exact value, when several members are equal under query equality but differ exactly. A number comes before a Date, a binary array, or a Temporal value; an ordinary number comes before `-0`, and `-0` before `NaN`; binary arrays are ordered by type name, so a `Buffer` comes before a `Uint8Array` with the same bytes. Before, the member with the smallest row key supplied the value. For example, a group that holds `new Date(0)` and `0` now projects `0`, and a group that holds `-0` and `0` now projects `0`. Values that are not equal under query equality are in different groups and do not change. +A `groupBy` value now comes from the member with the smallest exact value, when several members are equal under query equality but differ exactly. A primitive comes before a Date, a binary array, or a Temporal value; another number comes before `-0`; a `Buffer` comes before a `Uint8Array` with the same bytes. Before, the member with the smallest row key supplied the value. For example, a group that holds `new Date(0)` and `0` now projects `0`, and a group that holds `-0` and `0` now projects `0`. The projected value is always an instance that a current member holds. Values that are not equal under query equality are in different groups and do not change. diff --git a/packages/db/src/query/compiler/group-by.ts b/packages/db/src/query/compiler/group-by.ts index 95f8d268fd..a46295d12a 100644 --- a/packages/db/src/query/compiler/group-by.ts +++ b/packages/db/src/query/compiler/group-by.ts @@ -14,7 +14,6 @@ import { } from '../ir.js' import { isRefProxy } from '../builder/ref-proxy-identity.js' import { isTemporal } from '../../utils.js' -import { isUint8Array, normalizeValue } from '../../utils/comparison.js' import { AggregateFunctionNotInSelectError, NonAggregateExpressionNotInGroupByError, @@ -62,6 +61,7 @@ function createInternalGroupFields(groupCount: number, selectClause?: Select) { return { virtual: `${prefix}virtual`, route: `${prefix}route`, + exactInputs: `${prefix}exact_inputs`, correlationIdentity: `${prefix}correlation_identity`, parentContextIdentity: `${prefix}parent_context_identity`, singleGroup: `${prefix}single_group`, @@ -120,8 +120,9 @@ function createRepresentative( identity: unknown, ): Representative { // Encode once per contribution, not once per member on every group change. - // Members with equal identities consolidate, so a change does not re-read - // the group. + // D2 consolidates contributions whose hashes match. The identity must + // therefore separate every exactly different value, so a merged + // contribution never projects a value that no positive member holds. const representative = { key: serializeValue(identity), } as Representative @@ -131,16 +132,20 @@ function createRepresentative( /** * Members equal under query equality supply the value with the smallest exact - * identity. A number sorts before an object, and an object of one type and - * content is one exact value, whichever instance supplies it. + * value: another number before -0, and every primitive before an object. + * Objects are ordered by an explicit type tag, so the order survives + * minification. Members of one tag are equal in content; the instance + * identity that follows keeps each instance a separate contribution. */ -function exactValueIdentity( - value: unknown, - valueIdentity: ValueIdentity, -): unknown { - return value instanceof Date || isTemporal(value) || isUint8Array(value) - ? [`object`, value.constructor.name, normalizeValue(value)] - : valueIdentity.exact(value) +function exactValueOrder(value: unknown): unknown { + if (Object.is(value, -0)) return 1 + if (value === null || typeof value !== `object`) return 0 + if (value instanceof Date) return [2, `Date`] + if (isTemporal(value)) return [2, (value as any)[Symbol.toStringTag]] + if (typeof Buffer !== `undefined` && value instanceof Buffer) { + return [2, `Buffer`] + } + return [2, value instanceof Uint8Array ? `Uint8Array` : `Object`] } function getRepresentative( @@ -190,11 +195,12 @@ function addCorrelationRouteAggregate( aggregates[fields.route] = { preMap: ([, row]: [string, NamespacedRow]) => { const route = getNamespacedRouteMetadata(row, mainSource) - // Equal route identities mean equal routes, so members need no row - // key and consolidate into one contribution. + // Members of one route share its correlation key and parent context + // instances, so they consolidate into one contribution. A changed + // parent builds a new context, which stays separate. return createRepresentative(route, [ valueIdentity.exact(route?.correlationKey), - getParentContextIdentity(route?.parentContext), + valueIdentity.exact(route?.parentContext), ]) }, reduce: getRepresentative, @@ -400,21 +406,33 @@ export function processGroupBy( aggregates[fields.groupValues[i]!] = { preMap: ([, row]: [string, NamespacedRow]) => { const value = compiledExpr(row) - return createRepresentative( - value, - exactValueIdentity(value, valueIdentity), - ) + return createRepresentative(value, [ + exactValueOrder(value), + valueIdentity.exact(value), + ]) }, reduce: getRepresentative, postMap: unwrapRepresentative, } } + // D2 consolidates contributions whose hashes match, and the hash treats -0 + // as 0 and equal Dates as one value. A sum, avg, min, or max must still see + // each exactly different input, so contributions carry those inputs' + // exact identities. A count's input is only 0 or 1. + const exactInputs: Array<(row: NamespacedRow) => unknown> = [] + const addAggregate = (alias: string, aggExpr: Aggregate) => { + aggregates[alias] = getAggregateFunction(aggExpr) + if (aggExpr.name.toLowerCase() !== `count`) { + exactInputs.push(compileExpression(aggExpr.args[0]!)) + } + } + if (selectClause) { // Scan the SELECT clause for aggregate functions for (const [alias, expr] of Object.entries(selectClause)) { if (expr.type === `agg`) { - aggregates[alias] = getAggregateFunction(expr) + addAggregate(alias, expr) } else if (containsAggregate(expr)) { const { transformed, extracted } = extractAndReplaceAggregates( expr as SelectValueExpression, @@ -422,7 +440,7 @@ export function processGroupBy( fields.aggregatePrefix, ) for (const [syntheticAlias, aggExpr] of Object.entries(extracted)) { - aggregates[syntheticAlias] = getAggregateFunction(aggExpr) + addAggregate(syntheticAlias, aggExpr) } wrappedAggExprs[alias] = compileGroupedSelectValue( singleGroup @@ -437,6 +455,14 @@ export function processGroupBy( } } + if (exactInputs.length > 0) { + aggregates[fields.exactInputs] = { + preMap: ([, row]: [string, NamespacedRow]) => + exactInputs.map((input) => valueIdentity.exact(input(row))), + reduce: () => undefined, + } + } + // Apply the groupBy operator pipeline = pipeline.pipe(groupBy(keyExtractor, aggregates)) diff --git a/packages/db/src/query/live/ARCHITECTURE.md b/packages/db/src/query/live/ARCHITECTURE.md index 8e5ba65881..6743afd86f 100644 --- a/packages/db/src/query/live/ARCHITECTURE.md +++ b/packages/db/src/query/live/ARCHITECTURE.md @@ -226,14 +226,22 @@ equivalent query plans and retained public keys must survive graph replacement. For grouping, the equality token is the D2 group key. The group retains a raw value from a currently positive contributor only as the projected representative. The representative is the contributor with the smallest exact -value: a number before an object (a Date, a binary array, or a Temporal value); -an ordinary number before -0, and -0 before NaN; objects by type name. Objects -of one type and content are one exact value, so either instance may be -projected. The choice does not depend on row keys or on update history, and -contributors with one exact value consolidate in D2, so a change does not -re-read the group. A correlated include's route representative also carries -no row key: equal route identities mean equal routes. D2 sees only safe -exact-value identity for a representative, not the raw value itself. A separate public group key preserves primitive keys and +value: another number before -0, and every primitive before an object. Objects +are ordered by an explicit type tag (`Buffer`, `Date`, a Temporal type, then +`Uint8Array`). Contributors of one tag are equal in content, so any positive +instance may be projected; a deleted contributor's instance never is. The +choice does not depend on row keys or on update history. D2 consolidates +contributions whose hashes match, and its hash treats -0 as 0 and equal Dates +as one value. Each contribution therefore carries the exact identity of every +value a contributor can supply: the representative's exact value and +instance, and every sum, avg, min, or max input. Contributions merge only when +those are identical, so a merged contribution supplies only a value that a +positive contributor holds. A correlated include's route representative +carries the correlation key and parent context instances, which every member +of one route shares. A count over contributions that are identical in this +sense updates without re-reading the group; a sum, avg, min, or max over +distinct values keeps one contribution per distinct exact input. D2 sees only +safe exact-value identity for a representative, not the raw value itself. A separate public group key preserves primitive keys and serializes opaque equality identity; graph-local identity tokens never cross the Collection boundary. Compiler group fields use a query-local namespace disjoint from every selected alias. Direct correlated joins canonicalize both From e35d1ecfd1be0b1e6f83d4b241857340b4610999 Mon Sep 17 00:00:00 2001 From: Isaac Date: Wed, 7 Oct 2026 17:05:25 -0600 Subject: [PATCH 05/12] docs: record the review follow-up for group representatives Map each review finding to its evidence and disposition, add the review mutants and the include-count measurements, and correct the route and group-value laws. Co-authored-by: Isaac --- .../group-representative-exact-value.md | 92 +++++++++++++++---- 1 file changed, 75 insertions(+), 17 deletions(-) diff --git a/docs/contributing/oracle-reviews/group-representative-exact-value.md b/docs/contributing/oracle-reviews/group-representative-exact-value.md index ed0f8721e2..4f1df36999 100644 --- a/docs/contributing/oracle-reviews/group-representative-exact-value.md +++ b/docs/contributing/oracle-reviews/group-representative-exact-value.md @@ -1,8 +1,12 @@ # Group representative work and exact-value choice review -Evidence by revision, on `perf-aggregate-representatives` from `origin/main` -`ea51b67b0`: the RED results ran on `ea51b67b0` with the new tests, and the -GREEN and mutant results ran on the branch's production change. +Evidence by revision, on `perf-aggregate-representatives`: + +- First campaign: tests `ef9d4dad4`, production change `9b021bbdc`. The RED + results ran on `ea51b67b0` with the new tests. +- Review follow-up: tests `871bdea11`, production fix `e5449edc2`. The RED + results ran on `9b021bbdc` and the GREEN and mutant results on `e5449edc2`. + `f67789cd3` merges `origin/main` without changes to these files. ## Laws and authority @@ -13,21 +17,30 @@ GREEN and mutant results ran on the branch's production change. incremental view maintenance. A count changes by the multiplicity of the delta, so the result does not need a re-read of each member. 2. **Route law.** One representative carries the whole correlation route. - The route has two fields, `correlationKey` and `parentContext`. Its exact - identity is the exact identity of the correlation key and the parent - context identity. The parent context identity contains the exact identity - of each projected parent value. Thus two members with equal route identities - have the same correlation key and parent values, and the route needs no row - key to select one member. + The route has two fields, `correlationKey` and `parentContext`. Members of + one route share the correlation key instance and the parent context + instance, so the representative's identity is the exact identity of those + two instances. The first campaign used the parent context's equality + identity instead, which does not distinguish exactly different parent + values (review finding F4). +4. **Positive-contributor law.** A projected group value, and a `min` or `max` + result, is an exact value that a currently positive member holds + (`ARCHITECTURE.md`, "Value identity"). D2 consolidates contributions whose + hashes match, its hash treats `-0` as `0` and equal Dates as one value, and + a consolidated entry keeps the record of its latest change. Thus a + contribution must carry the exact identity of every value it can supply. 3. **Group-value law (revised by a product decision).** When several members are equal under query equality but differ exactly, the projected value comes from the member with the smallest exact value: - - a number before an object (a Date, a binary array, or a Temporal value); - - an ordinary number before `-0`, and `-0` before `NaN`; - - objects by type name, so `Buffer` before `Date` before `Uint8Array`. + - another number before `-0`, and every primitive before an object; + - objects by an explicit type tag: `Buffer`, `Date`, a Temporal type, then + `Uint8Array`. The tags do not come from constructor names, so + minification cannot change the order (review finding F7). - Objects of one type and content are one exact value, so either instance can - be projected. The previous law selected the member with the smallest row key. + `-0` and `NaN` are never equal under query equality, so their order is not + observable. Members of one tag are equal in content, and any positive + instance can be projected. The previous law selected the member with the + smallest row key. ## Old and new predictions @@ -91,11 +104,56 @@ in the original order. In the reversed order, the old code projected the | ORC-013 | Applicable. The work law is a scaling law. Four sizes over two orders of magnitude separate a constant cost from a cost per member. | | ORC-014 | Inapplicable. No controlled provider supplies a premise. | +## Review follow-up + +A high-effort review of `9b021bbdc` raised ten findings. Probes and the +extended oracle confirmed the product defects: + +| Finding | Verdict | Evidence on `9b021bbdc` | Disposition | +| --- | --- | --- | --- | +| F1 `min`/`max` returns a deleted row's value | Confirmed | Rows `0` and `-0`, delete `0`: `min` and `max` return `0`. On `main`: `-0`. Two equal Dates: the deleted row's instance. | Fixed: contributions carry the exact identity of each sum, avg, min, or max input | +| F2 projected value is a deleted row's instance | Confirmed | Two `Date(0)` instances, delete row 1: the deleted instance is projected | Fixed: the representative key holds the instance identity | +| F3 oracle compares only a type label | Confirmed | The F2 defect passed the old oracle | Fixed: the oracle requires a positive member's instance | +| F4 route identity uses parent equality | Evidence gap | An include aggregate cannot project a parent field, so no public observation was found. The mutant that restores the equality identity survives | Uses the parent context instance; recorded as unobserved | +| F5 Date and binary correlation keys never consolidate | Confirmed, accepted | Reference identity keeps distinct key instances apart | Limit below: correctness needs instance identity | +| F6 work law over-promises | Confirmed | Only identical inputs consolidate | The law, changeset and architecture text now state the scope | +| F7 order depends on constructor names and serialization | Confirmed by source | — | Fixed: explicit type tags | +| F8 binary contents serialized into each key | Confirmed | A 1 MiB group value is encoded twice per insert (12.6 MB); `main` encodes it once | Fixed: once, which is the group key's existing cost | +| F9 counter misses array walks | Confirmed by source | — | Fixed: the counter also counts array iteration and callbacks | +| F10 architecture text contradicted | Confirmed | — | Rewritten | + +Measured with the investigation probe (`NODE_ENV=production`, an include +count over one issue, median per comment insert, two runs each): + +| Comments | `main` `2c98b4992` | Branch `f67789cd3` | +| ---: | ---: | ---: | +| 10 | 0.109 / 0.125 ms | 0.168 / 0.162 ms | +| 100 | 0.096 / 0.095 ms | 0.129 / 0.113 ms | +| 1,000 | 0.175 / 0.153 ms | 0.098 / 0.108 ms | +| 5,000 | 0.557 / 0.439 ms | 0.101 / 0.091 ms | + +The 10-comment size runs first in each process, so it includes warm-up. + +Review mutants on `e5449edc2`: + +| Mutant | Outcome | +| --- | --- | +| No exact-input identity (merged retraction kept) | Assertion failure, 4 tests | +| No instance identity in the group-value representative | Assertion failure, 4 tests | +| Binary contents in the order key | Assertion failure, 1 test (binary encoding) | +| Parent context equality identity in the route | Survived: no public observation (F4) | +| `NaN` ordered before `-0` | Equivalent: the two never share a group | + ## Limits -- The iterator-step counter observes Map and Set iteration only. A re-read - through array iteration would not count. The reduce operator reads its group - through a Map, so the counter reaches the regression. +- The work counter observes Map, Set and array iteration and array callbacks. + A walk through an indexed `for` loop would not count. +- Contributions consolidate only when their inputs are exactly identical. A + sum, avg, min, or max over distinct values, and an include correlated on + Date, binary, or Temporal key instances, keep one contribution per distinct + input or instance. +- The group key serializes a large binary group value's contents once per + member. That cost predates this change. - A `groupBy` over values whose exact identity is a reference, such as plain objects, still has one contribution per distinct object. Equality for those values is also by reference, so each such group holds one value. From 8db25f3b0f8155d824e4e7925fdc76194cbbb9ad Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 8 Oct 2026 13:17:50 -0600 Subject: [PATCH 06/12] test(db): cancel rebuilt min/max retractions and choose equal instances by row key Add laws to the grouped aggregate work and group value oracles: an insert followed by its delete leaves cycle work constant when an inline subquery rebuilds the aggregate argument, and among content-equal group values the smallest row key supplies the instance, independent of arrival order. The include count's work law now covers updates and deletes, and the test model's exact order matches the documented law. Co-authored-by: Isaac --- packages/db/tests/query/group-by-work.test.ts | 102 +++++++++++++++--- packages/db/tests/query/group-by.test.ts | 81 ++++++++++---- 2 files changed, 152 insertions(+), 31 deletions(-) diff --git a/packages/db/tests/query/group-by-work.test.ts b/packages/db/tests/query/group-by-work.test.ts index 78f20c2d60..97b69b1ec7 100644 --- a/packages/db/tests/query/group-by-work.test.ts +++ b/packages/db/tests/query/group-by-work.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from 'vitest' import { createCollection } from '../../src/collection/index.js' import { createLiveQueryCollection, eq } from '../../src/query/index.js' -import { count } from '../../src/query/builder/functions.js' +import { count, max, sum } from '../../src/query/builder/functions.js' import { mockSyncCollectionOptions } from '../utils.js' describe(`group representative work`, () => { @@ -63,10 +63,11 @@ describe(`group representative work`, () => { * Authority: incremental view maintenance. A count changes by the delta's * multiplicity, so re-reading every member is not needed for the result. * - * Scope: the law holds when the group's members contribute exactly identical - * aggregate inputs, as a count over primitive group values does. A sum, avg, - * min, or max over distinct values keeps one contribution per distinct exact - * input, so its reduction still visits each of them. + * Scope: the law holds when the group's members contribute identical + * representative keys and min or max inputs, as a count, sum, or avg over + * primitive group values does. A min or max over distinct values keeps one + * contribution per distinct exact input, so its reduction still visits each + * of them. * * Observation: the number of Map, Set, and array iterator steps, plus array * callback calls (`map`, `filter`, `forEach`, `reduce`, `some`, `every`), @@ -223,20 +224,26 @@ describe(`grouped aggregate work per change`, () => { }) it(`keeps an include count's work independent of group size`, async () => { - const steps: Array = [] + const steps: Array> = [] for (const size of groupSizes) { const fixture = createCommentCount(size) try { await fixture.counts.preload() + const changes = [ + [`insert`, { id: size, issueId: 1 }, size + 1], + [`update`, { id: 0, issueId: 2 }, size], + [`update`, { id: 0, issueId: 1 }, size + 1], + [`delete`, { id: size, issueId: 1 }, size], + ] as const steps.push( - countIteratorSteps(() => - fixture.write(fixture.comments, `insert`, { - id: size, - issueId: 1, - }), - ), + changes.map(([type, value, expected]) => { + const count = countIteratorSteps(() => + fixture.write(fixture.comments, type, value), + ) + expect(fixture.commentCount(1)).toBe(expected) + return count + }), ) - expect(fixture.commentCount(1)).toBe(size + 1) } finally { await fixture.counts.cleanup() await fixture.issues.cleanup() @@ -327,3 +334,72 @@ describe(`grouped aggregate work per change`, () => { } }) }) + +/** + * # Does a retraction cancel its insert when the argument is rebuilt? + * + * Law: an insert followed by the matching delete leaves a grouped aggregate + * with the work it had before, however many such cycles ran. A query that + * reads its source through an inline subquery rebuilds the projected objects + * each time the subquery runs, so the retraction carries a new object for the + * same row. A contribution keyed by that object's instance would never cancel, + * and every later change would visit the leftover contributions. + * + * Observation: the iterator steps of one insert-and-delete cycle, measured + * after 10 cycles and after 300 cycles, with the published `max` and `sum` + * equal to the model's values at each checkpoint. + */ +describe(`retraction of a rebuilt aggregate argument`, () => { + it(`keeps cycle work constant for max and sum over a rebuilt value`, async () => { + const source = createCollection( + mockSyncCollectionOptions<{ id: number; g: number; x: number }>({ + id: `rebuilt-argument-${Math.random()}`, + getKey: (row) => row.id, + initialData: [ + { id: 1, g: 1, x: 1 }, + { id: 2, g: 1, x: 5 }, + ], + }), + ) + const grouped = createLiveQueryCollection((q) => + q + .from({ + s: q + .from({ row: source }) + .select(({ row }) => ({ + id: row.id, + g: row.g, + x: row.x, + box: { x: row.x }, + })), + }) + .groupBy(({ s }) => s.g) + .select(({ s }) => ({ + g: s.g, + hi: max(s.x), + total: sum(s.x), + boxed: max(s.box as never), + })), + ) + const cycle = () => + countIteratorSteps(() => { + for (const type of [`insert`, `delete`] as const) { + source.utils.begin() + source.utils.write({ type, value: { id: 99, g: 1, x: 7 } }) + source.utils.commit() + } + }) + try { + await grouped.preload() + for (let i = 0; i < 9; i++) cycle() + const early = cycle() + for (let i = 0; i < 289; i++) cycle() + const late = cycle() + expect(grouped.toArray).toMatchObject([{ g: 1, hi: 5, total: 6 }]) + expect(late).toBe(early) + } finally { + await grouped.cleanup() + await source.cleanup() + } + }) +}) diff --git a/packages/db/tests/query/group-by.test.ts b/packages/db/tests/query/group-by.test.ts index ec98cc38a7..82186356a5 100644 --- a/packages/db/tests/query/group-by.test.ts +++ b/packages/db/tests/query/group-by.test.ts @@ -262,13 +262,12 @@ const equalityEquivalentGroupValues: Array< * Which member supplies a group's projected value when several members are * equal under query equality but differ exactly? * - * Law: the member with the smallest exact value. Exact values are ordered by - * kind first: a number before an object (a Date, a binary array, or a - * Temporal value). Among numbers, an ordinary number comes before -0, and - * -0 before NaN. Among objects, the type name decides (`Buffer` before - * `Date` before `Uint8Array`). Objects of one type with the same content are - * the same exact value, so either instance may be projected. The choice does - * not depend on row keys or on the order in which rows arrived. + * Law: the member with the smallest exact value. Any primitive other than -0 + * comes first, then -0, then objects (a Date, a binary array, or a Temporal + * value). Among objects, the type tag decides (`Buffer` before `Date` before + * `Uint8Array`). Among objects of one type with the same content, the member + * with the smallest row key supplies the instance. The choice does not depend + * on the order in which rows arrived. * * Law: the projected value is an instance that a currently positive member * holds. After a member is deleted, its instance is never projected, even @@ -278,17 +277,13 @@ const equalityEquivalentGroupValues: Array< * That choice made every member's contribution distinct, so each change * re-read the whole group. */ -function exactRank(value: unknown): [number, number | string] { - if (typeof value === `number`) { - if (Object.is(value, -0)) return [0, 1] - if (Number.isNaN(value)) return [0, 2] - return [0, 0] - } - const typeName = - value !== null && typeof value === `object` - ? value.constructor.name - : typeof value - return [1, typeName] +function exactRank(value: unknown): [number, string] { + // Any primitive other than -0 comes first, then -0, then objects ordered by + // type tag. Object.prototype.toString reads the tag without production code. + if (Object.is(value, -0)) return [1, ``] + if (value === null || typeof value !== `object`) return [0, ``] + if (Buffer.isBuffer(value)) return [2, `Buffer`] + return [2, Object.prototype.toString.call(value).slice(8, -1)] } function smallestExact(values: ReadonlyArray): unknown { @@ -2563,3 +2558,53 @@ describe(`Query GROUP BY Execution`, () => { createGroupByTests(`off`) createGroupByTests(`eager`) }) + +/** + * Law: among members whose group values are equal in content, the projected + * instance does not depend on the order in which rows arrived. The member + * with the smallest row key supplies it, so it is also always a current + * member's instance. + */ +describe(`group value among equal instances`, () => { + for (const order of [ + [1, 2], + [2, 1], + ] as const) { + test(`projects the smallest row key's instance when rows arrive as ${order.join(`, `)}`, async () => { + const instances = new Map([ + [1, new Date(0)], + [2, new Date(0)], + ]) + const source = createCollection( + mockSyncCollectionOptions<{ id: number; v: Date }>({ + id: `equal-instances-${order.join(``)}-${Math.random()}`, + getKey: (row) => row.id, + initialData: [], + }), + ) + const grouped = createLiveQueryCollection((q) => + q + .from({ row: source }) + .groupBy(({ row }) => row.v) + .select(({ row }) => ({ v: row.v })), + ) + const write = (type: `insert` | `delete`, id: number) => { + source.utils.begin() + source.utils.write({ type, value: { id, v: instances.get(id)! } }) + source.utils.commit() + } + try { + await grouped.preload() + for (const id of order) write(`insert`, id) + expect((grouped.toArray[0] as { v: Date }).v).toBe(instances.get(1)) + write(`delete`, 1) + expect((grouped.toArray[0] as { v: Date }).v).toBe(instances.get(2)) + write(`insert`, 1) + expect((grouped.toArray[0] as { v: Date }).v).toBe(instances.get(1)) + } finally { + await grouped.cleanup() + await source.cleanup() + } + }) + } +}) From b86aaa7e2a0df0cc367bbf90fb1ac270f6eecbe8 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 8 Oct 2026 13:17:54 -0600 Subject: [PATCH 07/12] fix(db): key min/max exact inputs by the compared value and tie equal instances by row key A min or max contribution carried the identity of its raw argument, so a retraction that rebuilt the argument in an inline subquery did not cancel its insert and the deleted value stayed in the result. The identity now comes from the value the min or max compares. Sum and avg no longer carry exact inputs, since merging equal inputs cannot change their result. Among content-equal group values, the smallest row key supplies the instance. Co-authored-by: Isaac --- .../group-representative-exact-value.md | 4 +- packages/db/src/query/compiler/group-by.ts | 103 +++++++++++------- packages/db/src/query/live/ARCHITECTURE.md | 34 +++--- 3 files changed, 83 insertions(+), 58 deletions(-) diff --git a/.changeset/group-representative-exact-value.md b/.changeset/group-representative-exact-value.md index 6b03b3db47..2046c971fc 100644 --- a/.changeset/group-representative-exact-value.md +++ b/.changeset/group-representative-exact-value.md @@ -2,6 +2,6 @@ '@tanstack/db': patch --- -Update a grouped count without re-reading the group when the group's members contribute identical inputs. Before, each change to a `groupBy` group or to a count inside an include re-read every member of the group, because each member's contribution carried its row key. For example, an include that counts the comments of an issue re-read all of that issue's comments on each new comment. A `sum`, `avg`, `min`, or `max` over distinct values still visits one contribution for each distinct exact input. +Update a grouped count without re-reading the group when the group's members contribute identical inputs. Before, each change to a `groupBy` group or to a count inside an include re-read every member of the group, because each member's contribution carried its row key. For example, an include that counts the comments of an issue re-read all of that issue's comments on each new comment. A `sum` or `avg` consolidates equal inputs the same way. A `min` or `max` over distinct values still visits one contribution for each distinct exact input, and an include correlated on an object key, such as a Date or a binary array, still visits each member. -A `groupBy` value now comes from the member with the smallest exact value, when several members are equal under query equality but differ exactly. A primitive comes before a Date, a binary array, or a Temporal value; another number comes before `-0`; a `Buffer` comes before a `Uint8Array` with the same bytes. Before, the member with the smallest row key supplied the value. For example, a group that holds `new Date(0)` and `0` now projects `0`, and a group that holds `-0` and `0` now projects `0`. The projected value is always an instance that a current member holds. Values that are not equal under query equality are in different groups and do not change. +A `groupBy` value now comes from the member with the smallest exact value, when several members are equal under query equality but differ exactly. A primitive comes before a Date, a binary array, or a Temporal value; another number comes before `-0`; a `Buffer` comes before a `Uint8Array` with the same bytes. Before, the member with the smallest row key supplied the value. For example, a group that holds `new Date(0)` and `0` now projects `0`, and a group that holds `-0` and `0` now projects `0`. When members hold content-equal instances, the member with the smallest row key supplies the instance, so the choice does not depend on the order in which rows arrived. The projected value is always an instance that a current member holds. Values that are not equal under query equality are in different groups and do not change. diff --git a/packages/db/src/query/compiler/group-by.ts b/packages/db/src/query/compiler/group-by.ts index a46295d12a..0fe2778f5a 100644 --- a/packages/db/src/query/compiler/group-by.ts +++ b/packages/db/src/query/compiler/group-by.ts @@ -132,20 +132,34 @@ function createRepresentative( /** * Members equal under query equality supply the value with the smallest exact - * value: another number before -0, and every primitive before an object. + * value: another primitive before -0, and every primitive before an object. * Objects are ordered by an explicit type tag, so the order survives - * minification. Members of one tag are equal in content; the instance - * identity that follows keeps each instance a separate contribution. + * minification. Members of one tag are equal in content, so the smallest row + * key decides among them: the choice does not depend on arrival order, and + * the instance comes from a current member. Equal primitives share a key and + * consolidate. + * + * The key is compared as a serialized string. Its first two parts have a + * fixed format (a one-digit rank and a quoted tag), so the string order + * matches the order above; the last part only breaks ties. */ -function exactValueOrder(value: unknown): unknown { - if (Object.is(value, -0)) return 1 - if (value === null || typeof value !== `object`) return 0 - if (value instanceof Date) return [2, `Date`] - if (isTemporal(value)) return [2, (value as any)[Symbol.toStringTag]] - if (typeof Buffer !== `undefined` && value instanceof Buffer) { - return [2, `Buffer`] +function exactValueKey( + value: unknown, + rowKey: string, + valueIdentity: ValueIdentity, +): unknown { + if (Object.is(value, -0)) return [1, ``, ``] + if (value === null || typeof value !== `object`) { + return [0, ``, valueIdentity.exact(value)] } - return [2, value instanceof Uint8Array ? `Uint8Array` : `Object`] + return [2, exactObjectTag(value), rowKey] +} + +function exactObjectTag(value: object): string { + if (value instanceof Date) return `Date` + if (isTemporal(value)) return (value as any)[Symbol.toStringTag] + if (typeof Buffer !== `undefined` && value instanceof Buffer) return `Buffer` + return value instanceof Uint8Array ? `Uint8Array` : `Object` } function getRepresentative( @@ -404,12 +418,12 @@ export function processGroupBy( for (let i = 0; i < compiledGroupByExpressions.length; i++) { const compiledExpr = compiledGroupByExpressions[i]! aggregates[fields.groupValues[i]!] = { - preMap: ([, row]: [string, NamespacedRow]) => { + preMap: ([rowKey, row]: [string, NamespacedRow]) => { const value = compiledExpr(row) - return createRepresentative(value, [ - exactValueOrder(value), - valueIdentity.exact(value), - ]) + return createRepresentative( + value, + exactValueKey(value, rowKey, valueIdentity), + ) }, reduce: getRepresentative, postMap: unwrapRepresentative, @@ -417,14 +431,18 @@ export function processGroupBy( } // D2 consolidates contributions whose hashes match, and the hash treats -0 - // as 0 and equal Dates as one value. A sum, avg, min, or max must still see - // each exactly different input, so contributions carry those inputs' - // exact identities. A count's input is only 0 or 1. - const exactInputs: Array<(row: NamespacedRow) => unknown> = [] + // as 0 and equal Dates as one value. A min or max returns one of its + // inputs, so contributions carry the exact identity of the value it + // compares. A sum, avg, or count only adds its coerced numbers, so merging + // equal inputs cannot change its result. The identity is taken from the + // aggregate's own input, so a retraction that rebuilds its argument still + // cancels the insert. + const exactInputs: Array<(entry: [string, NamespacedRow]) => unknown> = [] const addAggregate = (alias: string, aggExpr: Aggregate) => { aggregates[alias] = getAggregateFunction(aggExpr) - if (aggExpr.name.toLowerCase() !== `count`) { - exactInputs.push(compileExpression(aggExpr.args[0]!)) + const name = aggExpr.name.toLowerCase() + if (name === `min` || name === `max`) { + exactInputs.push(minMaxInput(compileExpression(aggExpr.args[0]!))) } } @@ -457,8 +475,8 @@ export function processGroupBy( if (exactInputs.length > 0) { aggregates[fields.exactInputs] = { - preMap: ([, row]: [string, NamespacedRow]) => - exactInputs.map((input) => valueIdentity.exact(input(row))), + preMap: (entry: [string, NamespacedRow]) => + exactInputs.map((input) => valueIdentity.exact(input(entry))), reduce: () => undefined, } } @@ -641,6 +659,25 @@ function expressionsEqual(expr1: any, expr2: any): boolean { } } +/** The value a min or max compares: strings, numbers, Dates, and bigints keep + * their type, and anything else becomes a number. */ +function minMaxInput( + compiledExpr: (row: NamespacedRow) => unknown, +): (entry: [string, NamespacedRow]) => string | number | bigint | Date { + return ([, namespacedRow]) => { + const value = compiledExpr(namespacedRow) + if ( + typeof value === `number` || + typeof value === `string` || + typeof value === `bigint` || + value instanceof Date + ) { + return value + } + return value != null ? Number(value) : 0 + } +} + /** * Helper function to get an aggregate function based on the Agg expression */ @@ -658,23 +695,7 @@ function getAggregateFunction(aggExpr: Aggregate) { return value != null ? Number(value) : 0 } - // Create a value extractor function for min/max that preserves comparable types - const valueExtractorForMinMax = ([, namespacedRow]: [ - string, - NamespacedRow, - ]) => { - const value = compiledExpr(namespacedRow) - // Preserve strings, numbers, Dates, and bigints for comparison - if ( - typeof value === `number` || - typeof value === `string` || - typeof value === `bigint` || - value instanceof Date - ) { - return value - } - return value != null ? Number(value) : 0 - } + const valueExtractorForMinMax = minMaxInput(compiledExpr) // Create a raw value extractor function for the expression to aggregate const rawValueExtractor = ([, namespacedRow]: [string, NamespacedRow]) => { diff --git a/packages/db/src/query/live/ARCHITECTURE.md b/packages/db/src/query/live/ARCHITECTURE.md index 9ad8d10e8e..a36fba4070 100644 --- a/packages/db/src/query/live/ARCHITECTURE.md +++ b/packages/db/src/query/live/ARCHITECTURE.md @@ -226,21 +226,25 @@ equivalent query plans and retained public keys must survive graph replacement. For grouping, the equality token is the D2 group key. The group retains a raw value from a currently positive contributor only as the projected representative. The representative is the contributor with the smallest exact -value: another number before -0, and every primitive before an object. Objects -are ordered by an explicit type tag (`Buffer`, `Date`, a Temporal type, then -`Uint8Array`). Contributors of one tag are equal in content, so any positive -instance may be projected; a deleted contributor's instance never is. The -choice does not depend on row keys or on update history. D2 consolidates -contributions whose hashes match, and its hash treats -0 as 0 and equal Dates -as one value. Each contribution therefore carries the exact identity of every -value a contributor can supply: the representative's exact value and -instance, and every sum, avg, min, or max input. Contributions merge only when -those are identical, so a merged contribution supplies only a value that a -positive contributor holds. A correlated include's route representative -carries the correlation key and parent context instances, which every member -of one route shares. A count over contributions that are identical in this -sense updates without re-reading the group; a sum, avg, min, or max over -distinct values keeps one contribution per distinct exact input. D2 sees only +value: another primitive before -0, and every primitive before an object. +Objects are ordered by an explicit type tag (`Buffer`, `Date`, a Temporal type, +then `Uint8Array`). Contributors of one tag are equal in content, so the +contributor with the smallest row key supplies the instance; the choice does not +depend on update history, and a deleted contributor's instance is never +projected. Equal primitives share a representative key and consolidate. D2 +consolidates contributions whose hashes match, and its hash treats -0 as 0 and +equal Dates as one value. Each contribution therefore carries the exact identity +of every value a contributor can supply: the representative key, and the exact +input of every min or max, taken from the value that the min or max compares. +A sum, avg, or count adds coerced numbers, so merging its equal inputs cannot +change its result. Contributions merge only when those identities are equal, so +a merged contribution supplies only a value that a positive contributor holds. +A correlated include's route representative carries the correlation key and +parent context instances. Members that read the correlation key from their own +row hold distinct instances when the key is an object, such as a Date or a +binary array, so those members do not consolidate. A count, sum, or avg over +consolidated contributions updates without re-reading the group; a min or max +over distinct values keeps one contribution per distinct exact input. D2 sees only safe exact-value identity for a representative, not the raw value itself. A separate public group key preserves primitive keys and serializes opaque equality identity; graph-local identity tokens never cross the Collection boundary. Compiler group fields use a query-local namespace From c127bb210a3123fc761163e4915bfd29599a175f Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 8 Oct 2026 13:27:50 -0600 Subject: [PATCH 08/12] docs: record the medium review follow-up for group representatives Co-authored-by: Isaac --- .../group-representative-exact-value.md | 51 +++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/docs/contributing/oracle-reviews/group-representative-exact-value.md b/docs/contributing/oracle-reviews/group-representative-exact-value.md index 4f1df36999..028dc30401 100644 --- a/docs/contributing/oracle-reviews/group-representative-exact-value.md +++ b/docs/contributing/oracle-reviews/group-representative-exact-value.md @@ -148,10 +148,10 @@ Review mutants on `e5449edc2`: - The work counter observes Map, Set and array iteration and array callbacks. A walk through an indexed `for` loop would not count. -- Contributions consolidate only when their inputs are exactly identical. A - sum, avg, min, or max over distinct values, and an include correlated on - Date, binary, or Temporal key instances, keep one contribution per distinct - input or instance. +- Contributions consolidate only when their representative keys and min or + max inputs are identical. A min or max over distinct values, and an include + correlated on Date, binary, or Temporal key instances, keep one contribution + per distinct input or instance. - The group key serializes a large binary group value's contents once per member. That cost predates this change. - A `groupBy` over values whose exact identity is a reference, such as plain @@ -159,3 +159,46 @@ Review mutants on `e5449edc2`: values is also by reference, so each such group holds one value. - The fixed per-change overhead from #1740 is a separate cost and is outside this change. + +## Medium review follow-up (2026-10-08) + +Reviewed head `14d5cd49b`. Fix commits `8db25f3b0` (laws) and `b86aaa7e2` +(production). Ledger: `review-medium-ledger.md` in the task scratch notes. + +- **Rebuilt min/max argument.** An inline subquery rebuilds projected objects + each time it runs, so the retraction of a row carried a new argument + instance. The min or max contribution was keyed by that instance and did not + cancel its insert. RED on `14d5cd49b`: after inserting and deleting a row + with `x: 7`, `max` stayed `7`. The base commit before this pull request did + not have the fault. The identity now comes from the value the min or max + compares (`minMaxInput`), and the work law keeps cycle work constant over + 300 insert and delete cycles. +- **Sum and avg** no longer carry exact inputs. Their reduce adds coerced + numbers, so merging equal inputs cannot change the result. +- **Equal instances.** Among content-equal object group values, the member with + the smallest row key supplies the instance. RED on `14d5cd49b`: when rows + arrived as 2, 1, row 2's instance was projected. The choice no longer depends + on arrival order. +- **Test model.** `exactRank` now matches the documented order: another + primitive, then `-0`, then objects by type tag read with + `Object.prototype.toString`. +- **Include work law** covers insert, an update that moves a member out and + back, and delete. + +| Mutant | Outcome | +| --- | --- | +| Exact input from the raw argument (`14d5cd49b`) | Assertion failure: `max` keeps the deleted value | +| No min or max exact inputs | Assertion failure, 4 tests | +| Instance token as the tie among equal objects (`14d5cd49b`) | Assertion failure: arrival order 2, 1 | +| Row key in the route representative | Assertion failure: include work law | + +Not changed: + +- A parent field in an include aggregate's select is not projected into the + result on this commit or the base, so the route's parent context stays + without a public observation. +- A min or max argument compiles twice per query, once for the aggregate and + once for its exact input. This is a compile-time cost only. +- db-ivm merges hash-equal values by design. Choosing which instance remains + needs per-instance state, which the compiler identity supplies, so the fix + stays in the compiler. From 28a2f8c791e3aa2bbc82517d07fd67684538d45f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:29:12 +0000 Subject: [PATCH 09/12] ci: apply automated fixes --- packages/db/tests/query/group-by-work.test.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/db/tests/query/group-by-work.test.ts b/packages/db/tests/query/group-by-work.test.ts index 97b69b1ec7..dbdc0a6d87 100644 --- a/packages/db/tests/query/group-by-work.test.ts +++ b/packages/db/tests/query/group-by-work.test.ts @@ -364,14 +364,12 @@ describe(`retraction of a rebuilt aggregate argument`, () => { const grouped = createLiveQueryCollection((q) => q .from({ - s: q - .from({ row: source }) - .select(({ row }) => ({ - id: row.id, - g: row.g, - x: row.x, - box: { x: row.x }, - })), + s: q.from({ row: source }).select(({ row }) => ({ + id: row.id, + g: row.g, + x: row.x, + box: { x: row.x }, + })), }) .groupBy(({ s }) => s.g) .select(({ s }) => ({ From c2a09733fe749d782b80a8e6524135c7109b0f78 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 8 Oct 2026 15:19:17 -0600 Subject: [PATCH 10/12] test(db): check min and max against remaining members' exact values Delete members one at a time and require min and max to equal a remaining member's value that ties for the extreme, across signed zero and Dates and stored and rebuilt arguments. Run group-by tests in the node realm so the Buffer tag is reachable. Co-authored-by: Isaac --- docs/contributing/oracle-coverage.md | 9 +- packages/db/tests/query/group-by.test.ts | 123 +++++++++++++++++++++++ 2 files changed, 130 insertions(+), 2 deletions(-) diff --git a/docs/contributing/oracle-coverage.md b/docs/contributing/oracle-coverage.md index e750675030..3c4d1f07f5 100644 --- a/docs/contributing/oracle-coverage.md +++ b/docs/contributing/oracle-coverage.md @@ -416,8 +416,13 @@ followed by a parent or child publication at its callback boundary. aggregates: a change costs the same number of iterator steps at every group size, for a `groupBy` count and for an include count. `group-by.test.ts` checks which member supplies the value of a group whose members are equal -under query equality but differ exactly, in both arrival orders -([review](oracle-reviews/group-representative-exact-value.md)). +under query equality but differ exactly, in both arrival orders. It also +checks that `min` and `max` equal a remaining member's value after each delete, +for signed zero and Dates and for stored and rebuilt arguments +([review](oracle-reviews/group-representative-exact-value.md)). Open: no law +observes which parent supplies an include route's parent context, because a +single-group include aggregate drops non-aggregate select fields; parents equal +under query equality but different exactly share one route. The [Temporal group-key oracle](https://github.com/TanStack/db/blob/main/packages/db-ivm/tests/temporal-group-key-oracle.test.ts) owns the db-ivm `groupBy` value boundary for the eight Temporal kinds recognized diff --git a/packages/db/tests/query/group-by.test.ts b/packages/db/tests/query/group-by.test.ts index 82186356a5..06bcc56a43 100644 --- a/packages/db/tests/query/group-by.test.ts +++ b/packages/db/tests/query/group-by.test.ts @@ -1,3 +1,11 @@ +/** + * @vitest-environment node + * + * The binary group-value cases compare a Buffer with a Uint8Array. Under + * jsdom a Node Buffer is not an instance of jsdom's Uint8Array, so it would + * take the generic object tag and the Buffer-before-Uint8Array law would go + * unchecked. Node keeps both in one realm. + */ import { beforeEach, describe, expect, test } from 'vitest' import { Temporal } from 'temporal-polyfill' import { createLiveQueryCollection } from '../../src/query/index.js' @@ -502,6 +510,121 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { }, ) + // Law: after any delete, min and max equal the min and max of the + // remaining members' values. The model computes them from the remaining + // source rows, so it does not depend on how contributions are keyed. Each + // aggregate is checked alone and with the other, over the same or a + // different argument, and over a stored or rebuilt argument. A rebuilt + // argument is a new instance on every evaluation, as an inline subquery's + // `fn.select` produces, so a retraction cannot rely on instance identity. + const minMaxShapes = [ + [`min alone`, { low: `a` }], + [`max alone`, { high: `a` }], + [`min and max of one argument`, { low: `a`, high: `a` }], + [`min and max of different arguments`, { low: `a`, high: `b` }], + ] as const + const minMaxValues = [ + [`signed zero`, [0, -0, 0], [-0, 0, 0], (v: number) => v], + [`Dates`, [0, 5000, 0], [5000, 0, 2000], (v: number) => new Date(v)], + ] as const + const minMaxCases = minMaxShapes.flatMap(([shape, select]) => + minMaxValues.flatMap(([kind, aValues, bValues, toValue]) => + [false, true].map( + (rebuilt) => + [ + `${shape}, ${kind}, ${rebuilt ? `rebuilt` : `stored`} argument`, + select, + aValues, + bValues, + toValue, + rebuilt, + ] as const, + ), + ), + ) + test.each(minMaxCases)( + `min and max equal the remaining members' values: %s`, + (_name, select, aValues, bValues, toValue, rebuilt) => { + type Row = { id: number; group: number; a: unknown; b: unknown } + // A stored argument holds the final values in the source rows. A + // rebuilt argument holds numbers, and the subquery builds the value. + const rows: Array = aValues.map((a, i) => ({ + id: i + 1, + group: 1, + a: rebuilt ? a : toValue(a), + b: rebuilt ? bValues[i]! : toValue(bValues[i]!), + })) + const valueOf = (raw: unknown) => + rebuilt ? toValue(raw as number) : raw + const source = createCollection( + mockSyncCollectionOptions({ + id: `minmax-remaining-${autoIndex}-${_name}`, + getKey: (row) => row.id, + initialData: rows, + autoIndex, + }), + ) + const summary = createLiveQueryCollection({ + startSync: true, + query: (q) => { + const input = q.from({ r: source }).fn.select(({ r }) => ({ + id: r.id, + group: r.group, + a: valueOf(r.a), + b: valueOf(r.b), + })) + return q + .from({ row: input }) + .groupBy(({ row }) => row.group) + .select(({ row }) => ({ + group: row.group, + ...(`low` in select ? { low: min(row[select.low]) } : {}), + ...(`high` in select ? { high: max(row[select.high]) } : {}), + })) + }, + }) + // Content of a value under query equality: a Date by its time, -0 as 0. + const content = (value: unknown) => + value instanceof Date ? value.getTime() : (value as number) + 0 + // The exact identity a remaining member supplies: content plus the + // sign of zero. Two rebuilt Dates with one time are the same value. + const exact = (value: unknown) => + value instanceof Date + ? `Date(${value.getTime()})` + : Object.is(value, -0) + ? `-0` + : String(value) + const check = (remaining: Array) => { + const actual = summary.toArray[0] as Record + const expectOne = ( + alias: `low` | `high`, + field: `a` | `b`, + sign: 1 | -1, + ) => { + const values = remaining.map((row) => valueOf(row[field])) + const best = values + .map(content) + .reduce((l, r) => (sign * (r - l) < 0 ? r : l)) + // Any remaining member tied for the extreme may supply it. + const allowed = values + .filter((value) => content(value) === best) + .map(exact) + expect(allowed).toContain(exact(actual[alias])) + } + if (`low` in select) expectOne(`low`, select.low, 1) + if (`high` in select) expectOne(`high`, select.high, -1) + } + check(rows) + // Delete each member but the last, one at a time. + for (let i = 0; i < rows.length - 1; i++) { + source.utils.begin() + source.utils.write({ type: `delete`, value: rows[i]! }) + source.utils.commit() + check(rows.slice(i + 1)) + } + }, + ) + test.each([ `__group_value_0`, `__key_0`, From 7151e077bc4e7048de71aaeaeea1354fd5ff03ff Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 8 Oct 2026 15:19:23 -0600 Subject: [PATCH 11/12] fix(db): key object min and max inputs by row A rebuilt Date is a new instance at each evaluation, so keying a min or max contribution by instance kept a retraction from cancelling its insert and left a deleted row's value. Key object inputs by row key, as group values are, and correct the review record's exact-identity premise. Co-authored-by: Isaac --- .../group-representative-exact-value.md | 2 +- .../group-representative-exact-value.md | 52 +++++++++++++++++-- packages/db/src/query/compiler/group-by.ts | 16 +++--- packages/db/src/query/live/ARCHITECTURE.md | 7 ++- 4 files changed, 64 insertions(+), 13 deletions(-) diff --git a/.changeset/group-representative-exact-value.md b/.changeset/group-representative-exact-value.md index 2046c971fc..f12f181bd6 100644 --- a/.changeset/group-representative-exact-value.md +++ b/.changeset/group-representative-exact-value.md @@ -2,6 +2,6 @@ '@tanstack/db': patch --- -Update a grouped count without re-reading the group when the group's members contribute identical inputs. Before, each change to a `groupBy` group or to a count inside an include re-read every member of the group, because each member's contribution carried its row key. For example, an include that counts the comments of an issue re-read all of that issue's comments on each new comment. A `sum` or `avg` consolidates equal inputs the same way. A `min` or `max` over distinct values still visits one contribution for each distinct exact input, and an include correlated on an object key, such as a Date or a binary array, still visits each member. +Update a grouped count without re-reading the group when the group's members contribute identical inputs. Before, each change to a `groupBy` group or to a count inside an include re-read every member of the group, because each member's contribution carried its row key. For example, an include that counts the comments of an issue re-read all of that issue's comments on each new comment. A `sum` or `avg` consolidates equal inputs the same way. A `min` or `max` over distinct values still visits one contribution for each distinct exact input, a `min` or `max` over Dates visits one contribution for each member, and an include correlated on an object key, such as a Date or a binary array, still visits each member. A `groupBy` value now comes from the member with the smallest exact value, when several members are equal under query equality but differ exactly. A primitive comes before a Date, a binary array, or a Temporal value; another number comes before `-0`; a `Buffer` comes before a `Uint8Array` with the same bytes. Before, the member with the smallest row key supplied the value. For example, a group that holds `new Date(0)` and `0` now projects `0`, and a group that holds `-0` and `0` now projects `0`. When members hold content-equal instances, the member with the smallest row key supplies the instance, so the choice does not depend on the order in which rows arrived. The projected value is always an instance that a current member holds. Values that are not equal under query equality are in different groups and do not change. diff --git a/docs/contributing/oracle-reviews/group-representative-exact-value.md b/docs/contributing/oracle-reviews/group-representative-exact-value.md index 028dc30401..8ca0b50386 100644 --- a/docs/contributing/oracle-reviews/group-representative-exact-value.md +++ b/docs/contributing/oracle-reviews/group-representative-exact-value.md @@ -28,7 +28,9 @@ Evidence by revision, on `perf-aggregate-representatives`: (`ARCHITECTURE.md`, "Value identity"). D2 consolidates contributions whose hashes match, its hash treats `-0` as `0` and equal Dates as one value, and a consolidated entry keeps the record of its latest change. Thus a - contribution must carry the exact identity of every value it can supply. + contribution must carry a key that separates every value it can supply. A + primitive's key is its exact value. An object's key holds its row key, + because a rebuilt argument is a new instance each time. 3. **Group-value law (revised by a product decision).** When several members are equal under query equality but differ exactly, the projected value comes from the member with the smallest exact value: @@ -98,7 +100,7 @@ in the original order. In the reversed order, the old code projected the | ORC-006 | Applicable. The four mutants above fail at the intended checkpoints. | | ORC-007 | Inapplicable. These tests are finite, not generated properties. | | ORC-008 | Inapplicable. No stateful reference model changes. | -| ORC-009 | Applicable. "Iterator step" is a test observation, not a production concept. "Exact value" is the value relation that `ValueIdentity.exact` keeps, with Dates, binary arrays and Temporal values compared by type and content. | +| ORC-009 | Applicable. "Iterator step" is a test observation, not a production concept. "Exact value" is the value relation that `ValueIdentity.exact` keeps: primitives by value, with `-0` and `NaN` kept apart, and objects, including Dates, binary arrays and Temporal values, by instance. The group-value order compares objects by type tag only; the earlier text of this row said that exact identity compares them by content, which was false. | | ORC-010 | Applicable, with a gap. Cleanup runs in `finally` blocks, so a cleanup error can replace an assertion error. | | ORC-011 | Inapplicable. No shared fault was named. | | ORC-013 | Applicable. The work law is a scaling law. Four sizes over two orders of magnitude separate a constant cost from a cost per member. | @@ -111,7 +113,7 @@ extended oracle confirmed the product defects: | Finding | Verdict | Evidence on `9b021bbdc` | Disposition | | --- | --- | --- | --- | -| F1 `min`/`max` returns a deleted row's value | Confirmed | Rows `0` and `-0`, delete `0`: `min` and `max` return `0`. On `main`: `-0`. Two equal Dates: the deleted row's instance. | Fixed: contributions carry the exact identity of each sum, avg, min, or max input | +| F1 `min`/`max` returns a deleted row's value | Confirmed | Rows `0` and `-0`, delete `0`: `min` and `max` return `0`. On `main`: `-0`. Two equal Dates: the deleted row's instance. | Fixed: contributions carry the exact identity of each min or max input. This row first named sum and avg too; the medium follow-up removed them, and the targeted follow-up keys object inputs by row | | F2 projected value is a deleted row's instance | Confirmed | Two `Date(0)` instances, delete row 1: the deleted instance is projected | Fixed: the representative key holds the instance identity | | F3 oracle compares only a type label | Confirmed | The F2 defect passed the old oracle | Fixed: the oracle requires a positive member's instance | | F4 route identity uses parent equality | Evidence gap | An include aggregate cannot project a parent field, so no public observation was found. The mutant that restores the equality identity survives | Uses the parent context instance; recorded as unobserved | @@ -149,7 +151,8 @@ Review mutants on `e5449edc2`: - The work counter observes Map, Set and array iteration and array callbacks. A walk through an indexed `for` loop would not count. - Contributions consolidate only when their representative keys and min or - max inputs are identical. A min or max over distinct values, and an include + max inputs are identical. A min or max over objects keeps one contribution + per member. A min or max over distinct values, and an include correlated on Date, binary, or Temporal key instances, keep one contribution per distinct input or instance. - The group key serializes a large binary group value's contents once per @@ -202,3 +205,44 @@ Not changed: - db-ivm merges hash-equal values by design. Choosing which instance remains needs per-instance state, which the compiler identity supplies, so the fix stays in the compiler. + +## Targeted review follow-up (2026-10-08) + +Reviewed head `28a2f8c79`. Ledger: `review-targeted-ledger.md` in the task +scratch notes. + +- **Rebuilt Date min/max (regression).** The medium follow-up keyed a min or + max contribution by the exact identity of the compared value. A rebuilt Date + is a new instance at each evaluation, so a retraction did not cancel its + insert, and the Index kept the deleted row's instance. RED on `28a2f8c79`: + 6 cells, for example `expected [ 'Date(2000)' ] to include 'Date(5000)'`. + The base before this pull request passes all cells. An object input is now + keyed by its row key, as a group value is. +- **Law.** `min and max equal the remaining members' values` deletes members + one at a time. After each delete, the published `min` and `max` must be + exactly one of the remaining members' values that ties for the extreme. + The matrix covers `max` alone, `min` and `max` of one argument, and of two + arguments; signed zero and Dates; stored and rebuilt arguments. +- **Realm.** `group-by.test.ts` runs in the `node` environment. Under jsdom a + Node `Buffer` is not an instance of jsdom's `Uint8Array`, so the binary case + could not see the `Buffer` tag. + +| Mutant | Outcome | +| --- | --- | +| Production of `28a2f8c79` | Assertion failure, 6 cells (rebuilt Dates) | +| `max` without exact inputs | Assertion failure, 8 cells (signed zero) | +| No `Buffer` tag | Assertion failure, 2 tests (`expected 'uint8array' to be 'buffer'`) | +| Object input keyed by instance, not row | Assertion failure, 6 cells | + +Not changed, and recorded as proposals: + +- An include whose parents are equal under query equality but differ exactly + shares one route, so a parent field can come from another parent. This + predates this pull request. +- A single-group aggregate drops every non-aggregate select field, also in an + include, while the same select with `groupBy` throws + `NonAggregateExpressionNotInGroupByError`. This predates this pull request. +- db-ivm `min` and `max` ignore multiplicity, and the Index keeps the latest + record. The compiler's exact inputs work around both. +- The route representative is not covered by a deterministic-choice law. +- The work counter does not count indexed loops. diff --git a/packages/db/src/query/compiler/group-by.ts b/packages/db/src/query/compiler/group-by.ts index 0fe2778f5a..1d672acd8f 100644 --- a/packages/db/src/query/compiler/group-by.ts +++ b/packages/db/src/query/compiler/group-by.ts @@ -432,11 +432,13 @@ export function processGroupBy( // D2 consolidates contributions whose hashes match, and the hash treats -0 // as 0 and equal Dates as one value. A min or max returns one of its - // inputs, so contributions carry the exact identity of the value it - // compares. A sum, avg, or count only adds its coerced numbers, so merging - // equal inputs cannot change its result. The identity is taken from the - // aggregate's own input, so a retraction that rebuilds its argument still - // cancels the insert. + // inputs, so each contribution carries the exact key of the value it + // compares, as a group value does. A primitive's key is its exact value, so + // equal primitives still consolidate. An object's key holds its row key, so + // a retraction cancels its insert even when the argument builds a new + // instance each time, and a merged contribution never keeps a deleted + // row's instance. A sum, avg, or count only adds its coerced numbers, so + // merging equal inputs cannot change its result. const exactInputs: Array<(entry: [string, NamespacedRow]) => unknown> = [] const addAggregate = (alias: string, aggExpr: Aggregate) => { aggregates[alias] = getAggregateFunction(aggExpr) @@ -476,7 +478,9 @@ export function processGroupBy( if (exactInputs.length > 0) { aggregates[fields.exactInputs] = { preMap: (entry: [string, NamespacedRow]) => - exactInputs.map((input) => valueIdentity.exact(input(entry))), + exactInputs.map((input) => + exactValueKey(input(entry), entry[0], valueIdentity), + ), reduce: () => undefined, } } diff --git a/packages/db/src/query/live/ARCHITECTURE.md b/packages/db/src/query/live/ARCHITECTURE.md index a36fba4070..092125d023 100644 --- a/packages/db/src/query/live/ARCHITECTURE.md +++ b/packages/db/src/query/live/ARCHITECTURE.md @@ -236,7 +236,9 @@ consolidates contributions whose hashes match, and its hash treats -0 as 0 and equal Dates as one value. Each contribution therefore carries the exact identity of every value a contributor can supply: the representative key, and the exact input of every min or max, taken from the value that the min or max compares. -A sum, avg, or count adds coerced numbers, so merging its equal inputs cannot +A primitive input is keyed by its exact value and an object input by its +contributor's row key, because a rebuilt argument is a new instance at each +evaluation and its retraction must still cancel its insert. A sum, avg, or count adds coerced numbers, so merging its equal inputs cannot change its result. Contributions merge only when those identities are equal, so a merged contribution supplies only a value that a positive contributor holds. A correlated include's route representative carries the correlation key and @@ -244,7 +246,8 @@ parent context instances. Members that read the correlation key from their own row hold distinct instances when the key is an object, such as a Date or a binary array, so those members do not consolidate. A count, sum, or avg over consolidated contributions updates without re-reading the group; a min or max -over distinct values keeps one contribution per distinct exact input. D2 sees only +over distinct primitive values keeps one contribution per distinct exact input, +and a min or max over objects keeps one contribution per contributor. D2 sees only safe exact-value identity for a representative, not the raw value itself. A separate public group key preserves primitive keys and serializes opaque equality identity; graph-local identity tokens never cross the Collection boundary. Compiler group fields use a query-local namespace From 6912a48dabec4b0546c84f48845b12d0692e47e7 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 8 Oct 2026 15:36:13 -0600 Subject: [PATCH 12/12] test(db): type the min and max law's values Co-authored-by: Isaac --- packages/db/tests/query/group-by.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/db/tests/query/group-by.test.ts b/packages/db/tests/query/group-by.test.ts index 06bcc56a43..b9f17d9cbe 100644 --- a/packages/db/tests/query/group-by.test.ts +++ b/packages/db/tests/query/group-by.test.ts @@ -545,7 +545,12 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { test.each(minMaxCases)( `min and max equal the remaining members' values: %s`, (_name, select, aValues, bValues, toValue, rebuilt) => { - type Row = { id: number; group: number; a: unknown; b: unknown } + type Row = { + id: number + group: number + a: number | Date + b: number | Date + } // A stored argument holds the final values in the source rows. A // rebuilt argument holds numbers, and the subquery builds the value. const rows: Array = aValues.map((a, i) => ({ @@ -554,7 +559,7 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { a: rebuilt ? a : toValue(a), b: rebuilt ? bValues[i]! : toValue(bValues[i]!), })) - const valueOf = (raw: unknown) => + const valueOf = (raw: number | Date): number | Date => rebuilt ? toValue(raw as number) : raw const source = createCollection( mockSyncCollectionOptions({ @@ -595,7 +600,10 @@ function createGroupByTests(autoIndex: `off` | `eager`): void { ? `-0` : String(value) const check = (remaining: Array) => { - const actual = summary.toArray[0] as Record + const actual = summary.toArray[0] as unknown as Record< + string, + unknown + > const expectOne = ( alias: `low` | `high`, field: `a` | `b`,