From 9bfaa49b686d15ec72b775a55a2337a473bcfbfd Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 17:44:55 -0600 Subject: [PATCH 01/14] Apply canonical demand identity review fixes --- packages/db/src/query/index.ts | 5 + packages/db/src/query/ir-stable-identity.ts | 573 +++++++++++- packages/db/src/query/predicate-utils.ts | 71 +- .../src/query/runtime-reference-identity.ts | 41 + packages/db/src/query/subset-dedupe.ts | 19 +- .../db/tests/query/ir-stable-identity.test.ts | 867 ++++++++++++++++-- .../query/load-subset-oracle.property.test.ts | 136 ++- .../db/tests/query/predicate-utils.test.ts | 17 + .../query-db-collection/e2e/query-filter.ts | 118 +-- packages/query-db-collection/src/query.ts | 15 +- .../query-db-collection/src/serialization.ts | 135 --- .../load-subset-lifecycle-oracle.test.ts | 90 +- 12 files changed, 1662 insertions(+), 425 deletions(-) create mode 100644 packages/db/src/query/runtime-reference-identity.ts delete mode 100644 packages/query-db-collection/src/serialization.ts diff --git a/packages/db/src/query/index.ts b/packages/db/src/query/index.ts index c330b56b95..2c587b589b 100644 --- a/packages/db/src/query/index.ts +++ b/packages/db/src/query/index.ts @@ -98,9 +98,13 @@ export { type LiveQueryCollectionUtils } from './live/collection-config-builder. export { UnhashableQueryIRError, canonicalizeQueryIR, + getLoadSubsetDemandKey, + getQueryIdentity, getStableQueryBuilderHash, getStableQueryIRHash, getStableValueHash, + type DemandKey, + type QueryIdentity, } from './ir-stable-identity.js' // Predicate utilities for predicate push-down @@ -112,6 +116,7 @@ export { isLimitSubset, isOffsetLimitSubset, isPredicateSubset, + isLoadSubsetCoveredBy, } from './predicate-utils.js' export { DeduplicatedLoadSubset } from './subset-dedupe.js' diff --git a/packages/db/src/query/ir-stable-identity.ts b/packages/db/src/query/ir-stable-identity.ts index a479536fb2..871321601f 100644 --- a/packages/db/src/query/ir-stable-identity.ts +++ b/packages/db/src/query/ir-stable-identity.ts @@ -1,5 +1,7 @@ +import { normalizeValue } from '../utils/comparison.js' import { isRefProxy, toExpression } from './builder/ref-proxy.js' import { getQueryIR } from './builder/index.js' +import { getRuntimeReferenceIdentity } from './runtime-reference-identity.js' import type { Aggregate, BasicExpression, @@ -14,6 +16,7 @@ import type { Where, } from './ir.js' import type { InitialQueryBuilder, QueryBuilder } from './builder/index.js' +import type { LoadSubsetOptions } from '../types.js' type StableIdentityValue = | null @@ -23,6 +26,27 @@ type StableIdentityValue = | Array | { [key: string]: StableIdentityValue } +type RuntimeValueIdentity = `structural` | `semantic` + +type AliasScope = { + bindings: ReadonlyMap + hasUnqualifiedOutput: boolean + parent: AliasScope | undefined +} + +declare const queryIdentityBrand: unique symbol +declare const demandKeyBrand: unique symbol + +/** Semantic identity for a query plan, independent of its runtime owners. */ +export type QueryIdentity = string & { + readonly [queryIdentityBrand]: true +} + +/** Exact identity for one loadSubset demand, including its requested window. */ +export type DemandKey = string & { + readonly [demandKeyBrand]: true +} + export class UnhashableQueryIRError extends Error { constructor( public readonly path: string, @@ -34,7 +58,7 @@ export class UnhashableQueryIRError extends Error { } export function getStableQueryIRHash(query: QueryIR): string { - return JSON.stringify(canonicalizeQueryIR(query)) + return getQueryIdentity(query) } export function getStableQueryBuilderHash( @@ -47,14 +71,168 @@ export function getStableValueHash(value: unknown, path = `value`): string { return JSON.stringify(canonicalizeRuntimeValue(value, path, new WeakSet())) } +/** + * Returns the semantic identity of a structured query. + * + * Logical conjunctions and disjunctions are associative, commutative, and + * idempotent. Equality operands are commutative, while reversed inequalities + * are normalized by inverting their operator. Order-sensitive clauses and + * function arguments retain their original order. + */ +export function getQueryIdentity(query: QueryIR): QueryIdentity { + return JSON.stringify(canonicalizeQueryIR(query)) as QueryIdentity +} + +/** Returns the semantic identity of one structured expression. */ +export function getStableExpressionHash(expression: BasicExpression): string { + return JSON.stringify( + canonicalizeExpression(expression, `expression`, new WeakSet(), `semantic`), + ) +} + +/** + * Returns the exact semantic identity of a loadSubset request. + * + * Abort signals and subscriptions are owners of a request, not part of the + * requested data, and therefore do not affect the key. A demand generation + * scopes one asynchronous attempt rather than the data it requests. Code that + * rejects stale work compares this key alongside its generation; query-db uses + * the key alone so equivalent data demands can reuse one cache entry across + * generations. + */ +export function getLoadSubsetDemandKey( + options: LoadSubsetOptions, +): DemandKey | undefined { + if ( + options.where === undefined && + !options.orderBy?.length && + options.limit === undefined && + (options.offset === undefined || options.offset === 0) && + options.cursor === undefined + ) { + // Query-db uses its base query key for the one unconstrained demand. An + // owner-only option must not create another cache entry for the same data. + return undefined + } + + const seen = new WeakSet() + const result: Record = { + type: `loadSubsetDemand`, + query: canonicalizeLoadSubsetQuery(options, `loadSubset`, seen), + } + + if (options.limit !== undefined) { + result.limit = canonicalizeRuntimeValue( + options.limit, + `loadSubset.limit`, + seen, + ) + } + + if (options.offset !== undefined && options.offset !== 0) { + result.offset = canonicalizeRuntimeValue( + options.offset, + `loadSubset.offset`, + seen, + ) + } + + if (options.cursor !== undefined) { + const cursor: Record = { + whereFrom: canonicalizeExpression( + options.cursor.whereFrom, + `loadSubset.cursor.whereFrom`, + seen, + `semantic`, + ), + whereCurrent: canonicalizeExpression( + options.cursor.whereCurrent, + `loadSubset.cursor.whereCurrent`, + seen, + `semantic`, + ), + } + if (options.cursor.lastKey !== undefined) { + cursor.lastKey = canonicalizeRuntimeValue( + options.cursor.lastKey, + `loadSubset.cursor.lastKey`, + seen, + ) + } + result.cursor = cursor + } + + return JSON.stringify(result) as DemandKey +} + export function canonicalizeQueryIR(query: QueryIR): StableIdentityValue { return canonicalizeQuery(query, `query`, new WeakSet()) } +function createAliasScope( + query: QueryIR, + parent: AliasScope | undefined, +): AliasScope { + const bindings = new Map() + + const bindSource = (source: From): void => { + if (source.type === `unionFrom`) { + source.sources.forEach(bindSource) + return + } + if (source.type === `unionAll`) return + if (!bindings.has(source.alias)) { + bindings.set(source.alias, bindings.size) + } + } + + bindSource(query.from) + query.join?.forEach(({ from }) => bindSource(from)) + return { + bindings, + hasUnqualifiedOutput: query.from.type === `unionAll`, + parent, + } +} + +function resolveAliasBinding( + scope: AliasScope | undefined, + alias: string, +): readonly [number, number] | undefined { + let current = scope + let parentDistance = 0 + while (current) { + const binding = current.bindings.get(alias) + if (binding !== undefined) return [parentDistance, binding] + // A result-level union has no source alias. Every downstream ref starts at + // an output field, including nested paths such as profile.id, so it must + // not fall through and bind that field name to an enclosing query alias. + if (current.hasUnqualifiedOutput) return undefined + current = current.parent + parentDistance++ + } + return undefined +} + function canonicalizeQuery( query: QueryIR, path: string, seen: WeakSet, + parentScope?: AliasScope, +): StableIdentityValue { + return canonicalizeQueryInScope( + query, + path, + seen, + createAliasScope(query, parentScope), + ) +} + +function canonicalizeQueryInScope( + query: QueryIR, + path: string, + seen: WeakSet, + scope: AliasScope, ): StableIdentityValue { if (query.fnSelect) { throw new UnhashableQueryIRError(`${path}.fnSelect`, `function select`) @@ -70,40 +248,63 @@ function canonicalizeQuery( const result: Record = { type: `query`, - from: canonicalizeSource(query.from, `${path}.from`, seen), + from: canonicalizeSource(query.from, `${path}.from`, seen, scope), } if (query.select) { - result.select = canonicalizeSelect(query.select, `${path}.select`, seen) + result.select = canonicalizeSelect( + query.select, + `${path}.select`, + seen, + scope, + ) } if (query.join) { result.join = query.join.map((join, index) => - canonicalizeJoin(join, `${path}.join[${index}]`, seen), + canonicalizeJoin(join, `${path}.join[${index}]`, seen, scope), ) } if (query.where) { - result.where = query.where.map((where, index) => - canonicalizeWhere(where, `${path}.where[${index}]`, seen), + result.where = canonicalizeImplicitConjunction( + query.where, + `${path}.where`, + seen, + scope, ) } if (query.groupBy) { result.groupBy = query.groupBy.map((expression, index) => - canonicalizeExpression(expression, `${path}.groupBy[${index}]`, seen), + canonicalizeExpression( + expression, + `${path}.groupBy[${index}]`, + seen, + `semantic`, + scope, + ), ) } if (query.having) { - result.having = query.having.map((having, index) => - canonicalizeWhere(having, `${path}.having[${index}]`, seen), + result.having = canonicalizeImplicitConjunction( + query.having, + `${path}.having`, + seen, + scope, ) } if (query.orderBy) { result.orderBy = query.orderBy.map((orderBy, index) => - canonicalizeOrderBy(orderBy, `${path}.orderBy[${index}]`, seen), + canonicalizeOrderBy( + orderBy, + `${path}.orderBy[${index}]`, + seen, + `semantic`, + scope, + ), ) } @@ -111,7 +312,7 @@ function canonicalizeQuery( result.limit = canonicalizeRuntimeValue(query.limit, `${path}.limit`, seen) } - if (query.offset !== undefined) { + if (query.offset !== undefined && query.offset !== 0) { result.offset = canonicalizeRuntimeValue( query.offset, `${path}.offset`, @@ -130,16 +331,79 @@ function canonicalizeQuery( return result } +function canonicalizeImplicitConjunction( + clauses: ReadonlyArray, + path: string, + seen: WeakSet, + scope: AliasScope, +): Array { + const canonical = clauses.map((clause, index) => + canonicalizeWhere(clause, `${path}[${index}]`, seen, scope), + ) + canonical.sort(compareStableIdentityValues) + + return canonical.filter( + (clause, index) => + index === 0 || + compareStableIdentityValues(clause, canonical[index - 1]!) !== 0, + ) +} + +function canonicalizeLoadSubsetQuery( + options: LoadSubsetOptions, + path: string, + seen: WeakSet, +): StableIdentityValue { + const result: Record = { + type: `loadSubsetQuery`, + } + + if (options.where !== undefined) { + result.where = canonicalizeExpression( + options.where, + `${path}.where`, + seen, + `semantic`, + ) + } + + if (options.orderBy?.length) { + result.orderBy = options.orderBy.map((orderBy, index) => + canonicalizeOrderBy( + orderBy, + `${path}.orderBy[${index}]`, + seen, + `semantic`, + ), + ) + } + + return result +} + function canonicalizeJoin( join: JoinClause, path: string, seen: WeakSet, + scope: AliasScope, ): StableIdentityValue { return { type: join.type, - from: canonicalizeSource(join.from, `${path}.from`, seen), - left: canonicalizeExpression(join.left, `${path}.left`, seen), - right: canonicalizeExpression(join.right, `${path}.right`, seen), + from: canonicalizeSource(join.from, `${path}.from`, seen, scope), + left: canonicalizeExpression( + join.left, + `${path}.left`, + seen, + `semantic`, + scope, + ), + right: canonicalizeExpression( + join.right, + `${path}.right`, + seen, + `semantic`, + scope, + ), } } @@ -147,11 +411,11 @@ function canonicalizeSource( source: From, path: string, seen: WeakSet, + scope: AliasScope, ): StableIdentityValue { if (source.type === `collectionRef`) { return { type: `collectionRef`, - alias: source.alias, collectionId: canonicalizeRuntimeValue( source.collection.id, `${path}.collection.id`, @@ -164,7 +428,12 @@ function canonicalizeSource( return { type: `unionFrom`, sources: source.sources.map((unionSource, index) => - canonicalizeSource(unionSource, `${path}.sources[${index}]`, seen), + canonicalizeSource( + unionSource, + `${path}.sources[${index}]`, + seen, + scope, + ), ), } } @@ -173,15 +442,21 @@ function canonicalizeSource( return { type: `unionAll`, queries: source.queries.map((query, index) => - canonicalizeQuery(query, `${path}.queries[${index}]`, seen), + // Branches are peers that may capture the union query's outer scope; + // they are not children of the union result row itself. + canonicalizeQuery( + query, + `${path}.queries[${index}]`, + seen, + scope.parent, + ), ), } } return { type: `queryRef`, - alias: source.alias, - query: canonicalizeQuery(source.query, `${path}.query`, seen), + query: canonicalizeQuery(source.query, `${path}.query`, seen, scope), } } @@ -189,6 +464,7 @@ function canonicalizeSelect( select: Select, path: string, seen: WeakSet, + scope?: AliasScope, ): StableIdentityValue { return { type: `select`, @@ -196,7 +472,7 @@ function canonicalizeSelect( .sort() .map((key) => [ key, - canonicalizeSelectValue(select[key]!, `${path}.${key}`, seen), + canonicalizeSelectValue(select[key]!, `${path}.${key}`, seen, scope), ]), } } @@ -205,26 +481,34 @@ function canonicalizeSelectValue( value: unknown, path: string, seen: WeakSet, + scope?: AliasScope, ): StableIdentityValue { if (isRefProxy(value)) { - return canonicalizeExpression(toExpression(value), path, seen) + return canonicalizeExpression( + toExpression(value), + path, + seen, + `semantic`, + scope, + ) } if (isExpression(value)) { - return canonicalizeExpression(value, path, seen) + return canonicalizeExpression(value, path, seen, `semantic`, scope) } if (isPlainObject(value)) { - return canonicalizeSelect(value as Select, path, seen) + return canonicalizeSelect(value as Select, path, seen, scope) } - return canonicalizeRuntimeValue(value, path, seen) + return canonicalizeSemanticRuntimeValue(value, path, seen, scope) } function canonicalizeWhere( where: Where | Having, path: string, seen: WeakSet, + scope?: AliasScope, ): StableIdentityValue { if (isWhereObject(where)) { const result: Record = { @@ -233,6 +517,8 @@ function canonicalizeWhere( where.expression, `${path}.expression`, seen, + `semantic`, + scope, ), } @@ -243,19 +529,23 @@ function canonicalizeWhere( return result } - return canonicalizeExpression(where, path, seen) + return canonicalizeExpression(where, path, seen, `semantic`, scope) } function canonicalizeOrderBy( orderBy: OrderByClause, path: string, seen: WeakSet, + runtimeValueIdentity: RuntimeValueIdentity = `structural`, + scope?: AliasScope, ): StableIdentityValue { return { expression: canonicalizeExpression( orderBy.expression, `${path}.expression`, seen, + runtimeValueIdentity, + scope, ), compareOptions: canonicalizeRuntimeValue( orderBy.compareOptions, @@ -273,31 +563,96 @@ function canonicalizeExpression( | ConditionalSelect, path: string, seen: WeakSet, + runtimeValueIdentity: RuntimeValueIdentity = `structural`, + scope?: AliasScope, ): StableIdentityValue { if (expression.type === `ref`) { + const binding = resolveAliasBinding(scope, expression.path[0] ?? ``) return { type: `ref`, - path: expression.path.map((segment, index) => - canonicalizeRuntimeValue(segment, `${path}.path[${index}]`, seen), - ), + path: + binding === undefined + ? expression.path.map((segment, index) => + canonicalizeRuntimeValue(segment, `${path}.path[${index}]`, seen), + ) + : [ + [`binding`, ...binding], + ...expression.path + .slice(1) + .map((segment, index) => + canonicalizeRuntimeValue( + segment, + `${path}.path[${index + 1}]`, + seen, + ), + ), + ], } } if (expression.type === `val`) { return { type: `val`, - value: canonicalizeRuntimeValue(expression.value, `${path}.value`, seen), + value: + runtimeValueIdentity === `semantic` + ? canonicalizeSemanticRuntimeValue( + expression.value, + `${path}.value`, + seen, + scope, + ) + : canonicalizeRuntimeValue(expression.value, `${path}.value`, seen), } } if (expression.type === `func`) { - return { - type: `func`, - name: expression.name, - args: expression.args.map((arg, index) => - canonicalizeExpression(arg, `${path}.args[${index}]`, seen), - ), + if ( + expression.name === `in` && + expression.args.length === 2 && + expression.args[1]?.type === `val` && + Array.isArray(expression.args[1].value) + ) { + const candidates = expression.args[1].value.map((value, index) => + runtimeValueIdentity === `semantic` + ? canonicalizeSemanticRuntimeValue( + value, + `${path}.args[1].value[${index}]`, + seen, + scope, + ) + : canonicalizeRuntimeValue( + value, + `${path}.args[1].value[${index}]`, + seen, + ), + ) + return canonicalizeFunction(expression.name, [ + canonicalizeExpression( + expression.args[0]!, + `${path}.args[0]`, + seen, + runtimeValueIdentity, + scope, + ), + { + type: `val`, + // IN tests membership. Candidate order and duplicates do not change + // its result, but each candidate keeps its own equality semantics. + value: [`set`, sortUniqueStableIdentityValues(candidates)], + }, + ]) } + + const args = expression.args.map((arg, index) => + canonicalizeExpression( + arg, + `${path}.args[${index}]`, + seen, + runtimeValueIdentity, + scope, + ), + ) + return canonicalizeFunction(expression.name, args) } if (expression.type === `agg`) { @@ -305,7 +660,13 @@ function canonicalizeExpression( type: `agg`, name: expression.name, args: expression.args.map((arg, index) => - canonicalizeExpression(arg, `${path}.args[${index}]`, seen), + canonicalizeExpression( + arg, + `${path}.args[${index}]`, + seen, + runtimeValueIdentity, + scope, + ), ), } } @@ -318,11 +679,14 @@ function canonicalizeExpression( branch.condition, `${path}.branches[${index}].condition`, seen, + `semantic`, + scope, ), value: canonicalizeSelectValue( branch.value, `${path}.branches[${index}].value`, seen, + scope, ), })), } @@ -332,24 +696,35 @@ function canonicalizeExpression( expression.defaultValue, `${path}.defaultValue`, seen, + scope, ) } return result } + const childScope = createAliasScope(expression.query, scope) const result: Record = { type: `includesSubquery`, - query: canonicalizeQuery(expression.query, `${path}.query`, seen), + query: canonicalizeQueryInScope( + expression.query, + `${path}.query`, + seen, + childScope, + ), correlationField: canonicalizeExpression( expression.correlationField, `${path}.correlationField`, seen, + `semantic`, + scope, ), childCorrelationField: canonicalizeExpression( expression.childCorrelationField, `${path}.childCorrelationField`, seen, + `semantic`, + childScope, ), fieldName: expression.fieldName, materialization: expression.materialization, @@ -357,7 +732,7 @@ function canonicalizeExpression( if (expression.parentFilters) { result.parentFilters = expression.parentFilters.map((where, index) => - canonicalizeWhere(where, `${path}.parentFilters[${index}]`, seen), + canonicalizeWhere(where, `${path}.parentFilters[${index}]`, seen, scope), ) } @@ -368,6 +743,8 @@ function canonicalizeExpression( projection, `${path}.parentProjection[${index}]`, seen, + `semantic`, + scope, ), ) } @@ -379,6 +756,85 @@ function canonicalizeExpression( return result } +function canonicalizeFunction( + name: string, + args: Array, +): StableIdentityValue { + if ((name === `and` || name === `or`) && args.length > 0) { + const flattened = args.flatMap((arg) => + isCanonicalFunction(arg, name) ? arg.args : [arg], + ) + const unique = sortUniqueStableIdentityValues(flattened) + + return unique.length === 1 + ? unique[0]! + : { type: `func`, name, args: unique } + } + + if (name === `eq` && args.length === 2) { + args.sort(compareStableIdentityValues) + return { type: `func`, name, args } + } + + if ( + (name === `gt` || name === `gte` || name === `lt` || name === `lte`) && + args.length === 2 && + compareStableIdentityValues(args[0]!, args[1]!) > 0 + ) { + return { + type: `func`, + name: invertComparison(name), + args: [args[1]!, args[0]!], + } + } + + return { type: `func`, name, args } +} + +function sortUniqueStableIdentityValues( + values: Array, +): Array { + values.sort(compareStableIdentityValues) + return values.filter( + (value, index) => + index === 0 || + compareStableIdentityValues(value, values[index - 1]!) !== 0, + ) +} + +function isCanonicalFunction( + value: StableIdentityValue, + name: string, +): value is { + type: string + name: string + args: Array +} { + return ( + value !== null && + typeof value === `object` && + !Array.isArray(value) && + value.type === `func` && + value.name === name && + Array.isArray(value.args) + ) +} + +function invertComparison( + name: `gt` | `gte` | `lt` | `lte`, +): `gt` | `gte` | `lt` | `lte` { + switch (name) { + case `gt`: + return `lt` + case `gte`: + return `lte` + case `lt`: + return `gt` + case `lte`: + return `gte` + } +} + function canonicalizeRuntimeValue( value: unknown, path: string, @@ -497,6 +953,47 @@ function canonicalizeRuntimeValue( throw new UnhashableQueryIRError(path, `non-plain object value`) } +function canonicalizeSemanticRuntimeValue( + value: unknown, + path: string, + seen: WeakSet, + scope?: AliasScope, +): StableIdentityValue { + if (isRefProxy(value)) { + return canonicalizeExpression( + toExpression(value), + path, + seen, + `semantic`, + scope, + ) + } + + if (typeof value === `number` && Object.is(value, -0)) { + return canonicalizeRuntimeValue(0, path, seen) + } + + // Equality compares Uint8Array and Buffer values by content, independent of + // their concrete constructor and size. + const isUint8Array = + (typeof Buffer !== `undefined` && value instanceof Buffer) || + value instanceof Uint8Array + if (isUint8Array) { + return [`binary`, `Uint8Array`, Array.from(value as Uint8Array)] + } + + const normalized = normalizeValue(value) + if (normalized !== value) { + return canonicalizeRuntimeValue(normalized, path, seen) + } + + if (typeof value === `object` && value !== null) { + return getRuntimeReferenceIdentity(value) + } + + return canonicalizeRuntimeValue(value, path, seen) +} + function compareStableIdentityValues( left: StableIdentityValue, right: StableIdentityValue, diff --git a/packages/db/src/query/predicate-utils.ts b/packages/db/src/query/predicate-utils.ts index 4483d44ae2..2808a19937 100644 --- a/packages/db/src/query/predicate-utils.ts +++ b/packages/db/src/query/predicate-utils.ts @@ -1,4 +1,8 @@ import { Func, Value } from './ir.js' +import { + UnhashableQueryIRError, + getStableExpressionHash, +} from './ir-stable-identity.js' import type { BasicExpression, OrderBy, PropRef } from './ir.js' import type { LoadSubsetOptions } from '../types.js' @@ -872,6 +876,16 @@ export function isPredicateSubset( // Example: superset = {where: status='active', limit: 10, offset: 0, orderBy: desc} // subset = {where: status='active', limit: 5, offset: 0, orderBy: desc} // The top 5 active items ARE contained in the top 10 active items. + if (superset.limit !== undefined || superset.cursor !== undefined) { + // A cursor page only covers another request for the same page, whether or + // not the adapter also uses a numeric limit. + // Adapters may use the cursor expressions instead of offset, so matching + // offsets alone do not prove that two requests load the same rows. + if (!areCursorExpressionsEqual(subset.cursor, superset.cursor)) { + return false + } + } + if (superset.limit !== undefined) { // For limited supersets, where clauses must be equal if (!areWhereClausesEqual(subset.where, superset.where)) { @@ -893,6 +907,31 @@ export function isPredicateSubset( ) } +/** + * Returns whether established coverage satisfies a requested demand. + * + * Coverage is a directional relation. It must not be replaced with DemandKey + * equality, which answers whether two exact requests are the same. + */ +export function isLoadSubsetCoveredBy( + demand: LoadSubsetOptions, + coverage: LoadSubsetOptions, +): boolean { + return isPredicateSubset(demand, coverage) +} + +function areCursorExpressionsEqual( + a: LoadSubsetOptions[`cursor`], + b: LoadSubsetOptions[`cursor`], +): boolean { + if (a === undefined || b === undefined) return a === b + return ( + Object.is(a.lastKey, b.lastKey) && + areExpressionsEqual(a.whereFrom, b.whereFrom) && + areExpressionsEqual(a.whereCurrent, b.whereCurrent) + ) +} + /** * Check if two where clauses are structurally equal. * Used for limited query subset checks where subset relationship isn't sufficient. @@ -1047,32 +1086,34 @@ function findPredicateWithOperator( } function areExpressionsEqual(a: BasicExpression, b: BasicExpression): boolean { - if (a.type !== b.type) { - return false + try { + return getStableExpressionHash(a) === getStableExpressionHash(b) + } catch (error) { + if (!(error instanceof UnhashableQueryIRError)) throw error + return areExpressionsStructurallyEqual(a, b) } +} +function areExpressionsStructurallyEqual( + a: BasicExpression, + b: BasicExpression, +): boolean { + if (a.type !== b.type) return false if (a.type === `val` && b.type === `val`) { return areValuesEqual(a.value, b.value) } - if (a.type === `ref` && b.type === `ref`) { return areRefsEqual(a, b) } - if (a.type === `func` && b.type === `func`) { - const aFunc = a - const bFunc = b - if (aFunc.name !== bFunc.name) { - return false - } - if (aFunc.args.length !== bFunc.args.length) { - return false - } - return aFunc.args.every((arg, i) => - areExpressionsEqual(arg, bFunc.args[i]!), + return ( + a.name === b.name && + a.args.length === b.args.length && + a.args.every((arg, index) => + areExpressionsStructurallyEqual(arg, b.args[index]!), + ) ) } - return false } diff --git a/packages/db/src/query/runtime-reference-identity.ts b/packages/db/src/query/runtime-reference-identity.ts new file mode 100644 index 0000000000..d903060522 --- /dev/null +++ b/packages/db/src/query/runtime-reference-identity.ts @@ -0,0 +1,41 @@ +export type RuntimeReferenceIdentity = [ + `runtimeReference`, + namespace: string, + sequence: number, +] + +export function createRuntimeReferenceIdentityFactory(): ( + value: object, +) => RuntimeReferenceIdentity { + const namespace = createRuntimeReferenceNamespace() + const referenceIds = new WeakMap() + let sequence = 0 + + return (value) => { + let referenceId = referenceIds.get(value) + if (referenceId === undefined) { + referenceId = ++sequence + referenceIds.set(value, referenceId) + } + return [`runtimeReference`, namespace, referenceId] + } +} + +export const getRuntimeReferenceIdentity = + createRuntimeReferenceIdentityFactory() + +function createRuntimeReferenceNamespace(): string { + const randomValues = new Uint32Array(4) + const runtimeCrypto = Reflect.get(globalThis, `crypto`) as + | { getRandomValues: (values: Uint32Array) => Uint32Array } + | undefined + if (runtimeCrypto !== undefined) { + runtimeCrypto.getRandomValues(randomValues) + return Array.from(randomValues, (value) => value.toString(36)).join(`-`) + } + + // Reference equality cannot survive a runtime boundary. A per-runtime nonce + // prevents a persisted key from matching an unrelated reference after a + // reload, even on platforms without Web Crypto. + return `${Date.now().toString(36)}-${Math.random().toString(36).slice(2)}-${Math.random().toString(36).slice(2)}` +} diff --git a/packages/db/src/query/subset-dedupe.ts b/packages/db/src/query/subset-dedupe.ts index c0381f78d5..f8e49d0f08 100644 --- a/packages/db/src/query/subset-dedupe.ts +++ b/packages/db/src/query/subset-dedupe.ts @@ -1,5 +1,5 @@ import { - isPredicateSubset, + isLoadSubsetCoveredBy, isWhereSubset, minusWherePredicates, unionWherePredicates, @@ -61,7 +61,7 @@ export class DeduplicatedLoadSubset { // Flag to track if we've loaded all data (unlimited call with no where clause) private hasLoadedAllData = false - // List of all limited calls (with limit, possibly with orderBy) + // List of calls with a finite or cursor-relative result window. // We clone options before storing to prevent mutation of stored predicates private limitedCalls: Array = [] @@ -109,9 +109,9 @@ export class DeduplicatedLoadSubset { } // Check against limited calls - if (options.limit !== undefined) { + if (options.limit !== undefined || options.cursor !== undefined) { const alreadyLoaded = this.limitedCalls.some((loaded) => - isPredicateSubset(options, loaded), + isLoadSubsetCoveredBy(options, loaded), ) if (alreadyLoaded) { @@ -124,7 +124,8 @@ export class DeduplicatedLoadSubset { // This prevents duplicate requests when concurrent calls have subset relationships const matchingInflight = this.inflightCalls.find( (inflight) => - !inflight.lease.aborted && isPredicateSubset(options, inflight.options), + !inflight.lease.aborted && + isLoadSubsetCoveredBy(options, inflight.options), ) if (matchingInflight !== undefined) { @@ -148,7 +149,11 @@ export class DeduplicatedLoadSubset { const lease = createSharedAbortLease(options.signal) const trackingOptions = cloneOptions({ ...options, signal: lease.signal }) const loadOptions = cloneOptions({ ...options, signal: lease.signal }) - if (this.unlimitedWhere !== undefined && options.limit === undefined) { + if ( + this.unlimitedWhere !== undefined && + options.limit === undefined && + options.cursor === undefined + ) { // Compute difference to get only the missing data // We can only do this for unlimited queries // and we can only remove data that was loaded from unlimited queries @@ -230,7 +235,7 @@ export class DeduplicatedLoadSubset { private updateTracking(options: LoadSubsetOptions): void { // Update tracking based on whether this was a limited or unlimited call - if (options.limit === undefined) { + if (options.limit === undefined && options.cursor === undefined) { // Unlimited call - update combined where predicate // We ignore orderBy for unlimited calls as mentioned in requirements if (options.where === undefined) { diff --git a/packages/db/tests/query/ir-stable-identity.test.ts b/packages/db/tests/query/ir-stable-identity.test.ts index 145b3232af..3d9bd86053 100644 --- a/packages/db/tests/query/ir-stable-identity.test.ts +++ b/packages/db/tests/query/ir-stable-identity.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from 'vitest' +import { fc, test as fcTest } from '@fast-check/vitest' +import { Temporal } from 'temporal-polyfill' import { CollectionImpl } from '../../src/collection/index.js' import { Query, getQueryIR } from '../../src/query/builder/index.js' import { @@ -18,18 +20,36 @@ import { length, like, lower, + lt, max, not, or, + subtract, sum, upper, } from '../../src/query/builder/functions.js' import { UnhashableQueryIRError, + getLoadSubsetDemandKey, + getQueryIdentity, + getStableExpressionHash, getStableQueryIRHash, getStableValueHash, } from '../../src/query/ir-stable-identity.js' -import type { QueryIR } from '../../src/query/ir.js' +import { + CollectionRef, + Func, + IncludesSubquery, + PropRef, + QueryRef, + UnionAll, + Value, +} from '../../src/query/ir.js' +import { compileExpression } from '../../src/query/compiler/evaluators.js' +import { isLoadSubsetCoveredBy } from '../../src/query/predicate-utils.js' +import { createRuntimeReferenceIdentityFactory } from '../../src/query/runtime-reference-identity.js' +import type { BasicExpression, QueryIR } from '../../src/query/ir.js' +import type { LoadSubsetOptions } from '../../src/types.js' interface User { id: number @@ -52,6 +72,27 @@ interface User { largeViewCount?: bigint } +const referenceSemanticPairArbitrary = fc.oneof( + fc + .array(fc.integer()) + .map((values): [unknown, unknown] => [[...values], [...values]]), + fc + .dictionary(fc.string(), fc.integer()) + .map((value): [unknown, unknown] => [{ ...value }, { ...value }]), + fc + .array(fc.tuple(fc.string(), fc.integer())) + .map((entries): [unknown, unknown] => [new Map(entries), new Map(entries)]), + fc + .array(fc.integer()) + .map((values): [unknown, unknown] => [new Set(values), new Set(values)]), + fc + .int16Array() + .map((value): [unknown, unknown] => [ + new Int16Array(value), + new Int16Array(value), + ]), +) + interface Post { id: number userId: number @@ -81,12 +122,557 @@ describe(`stable runtime value hashing`, () => { }) }) +describe(`semantic expression identity`, () => { + const age = new PropRef([`user`, `age`]) + const active = new PropRef([`user`, `active`]) + type EquivalentExpressionPair = { + original: BasicExpression + equivalent: BasicExpression + } + + const comparisonPairArbitrary: fc.Arbitrary = fc + .record({ + operator: fc.constantFrom<`gt` | `gte` | `lt` | `lte`>( + `gt`, + `gte`, + `lt`, + `lte`, + ), + threshold: fc.integer(), + }) + .map(({ operator, threshold }) => { + const inverse: Record<`gt` | `gte` | `lt` | `lte`, string> = { + gt: `lt`, + gte: `lte`, + lt: `gt`, + lte: `gte`, + } + return { + original: new Func(operator, [age, new Value(threshold)]), + equivalent: new Func(inverse[operator], [ + new Value(threshold), + age, + ]), + } + }) + const equalityPairArbitrary: fc.Arbitrary = fc + .boolean() + .map((value) => ({ + original: new Func(`eq`, [active, new Value(value)]), + equivalent: new Func(`eq`, [new Value(value), active]), + })) + const membershipPairArbitrary: fc.Arbitrary = fc + .uniqueArray(fc.integer(), { minLength: 1, maxLength: 8 }) + .map((values) => ({ + original: new Func(`in`, [age, new Value(values)]), + equivalent: new Func(`in`, [ + age, + new Value([...values].reverse().concat(values[0]!)), + ]), + })) + const atomicExpressionPairArbitrary = fc.oneof( + comparisonPairArbitrary, + equalityPairArbitrary, + membershipPairArbitrary, + ) + const equivalentExpressionPairArbitrary = fc.oneof( + { weight: 3, arbitrary: atomicExpressionPairArbitrary }, + { + weight: 2, + arbitrary: fc + .tuple( + fc.constantFrom(`and`, `or`), + atomicExpressionPairArbitrary, + atomicExpressionPairArbitrary, + ) + .map(([operator, left, right]) => ({ + original: new Func(operator, [ + left.original, + new Func(operator, [right.original, left.original]), + ]), + equivalent: new Func(operator, [ + right.equivalent, + left.equivalent, + ]), + })), + }, + ) + + it(`normalizes associative, commutative, and idempotent boolean forms`, () => { + const adult = new Func(`gte`, [age, new Value(18)]) + const enabled = new Func(`eq`, [active, new Value(true)]) + const nested = new Func(`and`, [ + enabled, + new Func(`and`, [adult, enabled]), + ]) + const flat = new Func(`and`, [adult, enabled]) + + expect(getStableExpressionHash(nested)).toBe(getStableExpressionHash(flat)) + expect(getStableExpressionHash(new Func(`or`, [adult, adult]))).toBe( + getStableExpressionHash(adult), + ) + }) + + it(`normalizes equality and reversed inequalities`, () => { + expect(getStableExpressionHash(new Func(`eq`, [age, new Value(18)]))).toBe( + getStableExpressionHash(new Func(`eq`, [new Value(18), age])), + ) + expect(getStableExpressionHash(new Func(`gt`, [age, new Value(18)]))).toBe( + getStableExpressionHash(new Func(`lt`, [new Value(18), age])), + ) + }) + + it(`preserves order-sensitive function arguments`, () => { + expect( + getStableExpressionHash(new Func(`subtract`, [age, new Value(1)])), + ).not.toBe( + getStableExpressionHash(new Func(`subtract`, [new Value(1), age])), + ) + }) + + fcTest.prop([ + equivalentExpressionPairArbitrary, + fc.record({ age: fc.integer(), active: fc.boolean() }), + ])(`canonical expression grammar preserves semantics`, (pair, sample) => { + const row = { user: sample } + + expect(compileExpression(pair.original)(row)).toBe( + compileExpression(pair.equivalent)(row), + ) + expect(getStableExpressionHash(pair.original)).toBe( + getStableExpressionHash(pair.equivalent), + ) + }) + + fcTest.prop([referenceSemanticPairArbitrary])( + `keeps reference-semantic values distinct across identity and coverage`, + ([first, second]) => { + const value = new PropRef([`row`, `value`]) + const firstPredicate = new Func(`eq`, [value, new Value(first)]) + const secondPredicate = new Func(`eq`, [ + value, + new Value(second), + ]) + const row = { row: { value: first } } + + expect(compileExpression(firstPredicate)(row)).toBe(true) + expect(compileExpression(secondPredicate)(row)).toBe(false) + expect(getStableExpressionHash(firstPredicate)).not.toBe( + getStableExpressionHash(secondPredicate), + ) + expect( + getLoadSubsetDemandKey({ where: firstPredicate, limit: 1 }), + ).not.toBe(getLoadSubsetDemandKey({ where: secondPredicate, limit: 1 })) + expect( + isLoadSubsetCoveredBy( + { where: firstPredicate, limit: 1 }, + { where: secondPredicate, limit: 1 }, + ), + ).toBe(false) + }, + ) + + it(`does not reuse reference identities across runtimes`, () => { + const firstRuntime = createRuntimeReferenceIdentityFactory() + const secondRuntime = createRuntimeReferenceIdentityFactory() + + expect(firstRuntime({ a: 1 })).not.toEqual(secondRuntime({ b: 2 })) + }) + + fcTest.prop([ + fc.uniqueArray(fc.oneof(fc.integer(), fc.string(), fc.boolean()), { + minLength: 1, + maxLength: 8, + }), + ])(`treats IN candidates as a set`, (candidates) => { + const value = new PropRef([`row`, `value`]) + const ordered = new Func(`in`, [value, new Value(candidates)]) + const reordered = new Func(`in`, [ + value, + new Value([...candidates].reverse().concat(candidates[0]!)), + ]) + + for (const candidate of candidates) { + const row = { row: { value: candidate } } + expect(compileExpression(ordered)(row)).toBe( + compileExpression(reordered)(row), + ) + } + expect(getStableExpressionHash(ordered)).toBe( + getStableExpressionHash(reordered), + ) + expect(getLoadSubsetDemandKey({ where: ordered })).toBe( + getLoadSubsetDemandKey({ where: reordered }), + ) + }) +}) + +describe(`loadSubset demand identity`, () => { + const id = new PropRef([`id`]) + const group = new PropRef([`group`]) + const first = new Func(`eq`, [id, new Value(`a`)]) + const second = new Func(`eq`, [group, new Value(`x`)]) + const orderBy: NonNullable = [ + { + expression: id, + compareOptions: { direction: `asc`, nulls: `first` }, + }, + { + expression: group, + compareOptions: { direction: `desc`, nulls: `last` }, + }, + ] + + it(`includes the exact requested window`, () => { + const narrow = { where: first, orderBy, limit: 10, offset: 5 } + const wide = { where: first, orderBy, limit: 20, offset: 0 } + + expect(getLoadSubsetDemandKey(narrow)).not.toBe( + getLoadSubsetDemandKey(wide), + ) + }) + + it(`normalizes predicates but preserves orderBy sequence`, () => { + const left = new Func(`and`, [first, second]) + const right = new Func(`and`, [second, first]) + + expect(getLoadSubsetDemandKey({ where: left, orderBy })).toBe( + getLoadSubsetDemandKey({ where: right, orderBy }), + ) + expect(getLoadSubsetDemandKey({ where: left, orderBy })).not.toBe( + getLoadSubsetDemandKey({ where: right, orderBy: [...orderBy].reverse() }), + ) + }) + + it(`includes cursor shape and excludes runtime owners`, () => { + const cursor = { + whereFrom: new Func(`gt`, [id, new Value(`a`)]), + whereCurrent: first, + lastKey: `a`, + } + const firstOwner = new AbortController() + const secondOwner = new AbortController() + const subscription = {} as NonNullable + + expect( + getLoadSubsetDemandKey({ + where: first, + cursor, + signal: firstOwner.signal, + subscription, + }), + ).toBe( + getLoadSubsetDemandKey({ + where: first, + cursor, + signal: secondOwner.signal, + }), + ) + expect(getLoadSubsetDemandKey({ where: first, cursor })).not.toBe( + getLoadSubsetDemandKey({ + where: first, + cursor: { ...cursor, lastKey: `b` }, + }), + ) + }) + + it(`uses the base query key for an unconstrained owner-only demand`, () => { + expect(getLoadSubsetDemandKey({})).toBeUndefined() + expect(getLoadSubsetDemandKey({ offset: 0 })).toBeUndefined() + expect( + getLoadSubsetDemandKey({ signal: new AbortController().signal }), + ).toBeUndefined() + expect(getLoadSubsetDemandKey({ where: first, offset: 0 })).toBe( + getLoadSubsetDemandKey({ where: first }), + ) + }) + + it.each([ + [`signed zero`, -0, 0], + [`invalid Date`, new Date(Number.NaN), new Date(Number.NaN)], + [ + `Temporal.PlainDate`, + Temporal.PlainDate.from(`2024-01-15`), + Temporal.PlainDate.from(`2024-01-15`), + ], + [ + `Temporal.Duration`, + Temporal.Duration.from(`PT1H`), + Temporal.Duration.from(`PT1H`), + ], + [ + `large cross-constructor binary`, + new Uint8Array(129).fill(7), + Buffer.from(new Uint8Array(129).fill(7)), + ], + ])( + `uses comparison semantics for equivalent %s values`, + (_label, firstValue, secondValue) => { + const value = new PropRef([`row`, `value`]) + const firstPredicate = new Func(`eq`, [ + value, + new Value(firstValue), + ]) + const secondPredicate = new Func(`eq`, [ + value, + new Value(secondValue), + ]) + + expect( + compileExpression(firstPredicate)({ row: { value: secondValue } }), + ).toBe(true) + expect(getStableExpressionHash(firstPredicate)).toBe( + getStableExpressionHash(secondPredicate), + ) + expect(getLoadSubsetDemandKey({ where: firstPredicate })).toBe( + getLoadSubsetDemandKey({ where: secondPredicate }), + ) + expect(getQueryIdentity(createProfileValueQuery(firstValue))).toBe( + getQueryIdentity(createProfileValueQuery(secondValue)), + ) + }, + ) +}) + const postsCollection = new CollectionImpl({ id: `posts`, getKey: (item) => item.id, sync: { sync: () => {} }, }) +function createProfileValueQuery(value: unknown): QueryIR { + return { + ...getQueryIR(new Query().from({ user: usersCollection })), + where: [ + new Func(`eq`, [ + new PropRef([`user`, `profile`]), + new Value(value), + ]), + ], + } +} + +function createAlphaRenamedJoinQuery( + userAlias: string, + postAlias: string, +): QueryIR { + return { + from: new CollectionRef( + usersCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + userAlias, + ), + join: [ + { + type: `inner`, + from: new CollectionRef( + postsCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + postAlias, + ), + left: new PropRef([userAlias, `id`]), + right: new PropRef([postAlias, `userId`]), + }, + ], + where: [ + new Func(`eq`, [new PropRef([postAlias, `published`]), new Value(true)]), + ], + select: { + userId: new PropRef([userAlias, `id`]), + postTitle: new PropRef([postAlias, `title`]), + }, + orderBy: [ + { + expression: new PropRef([postAlias, `createdAt`]), + compareOptions: { direction: `desc`, nulls: `last` }, + }, + ], + } +} + +function createAlphaRenamedNestedQuery( + innerAlias: string, + outerAlias: string, +): QueryIR { + const inner: QueryIR = { + from: new CollectionRef( + usersCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + innerAlias, + ), + select: { + id: new PropRef([innerAlias, `id`]), + status: new PropRef([innerAlias, `status`]), + }, + } + return { + from: new QueryRef(inner, outerAlias), + where: [ + new Func(`eq`, [ + new PropRef([outerAlias, `status`]), + new Value(`active`), + ]), + ], + select: { id: new PropRef([outerAlias, `id`]) }, + } +} + +function createUnionDerivedNestedQuery(outerAlias: string): QueryIR { + const users = new Query() + .from({ user: usersCollection }) + .select(({ user }) => ({ id: user.id, kind: user.status })) + const posts = new Query() + .from({ post: postsCollection }) + .select(({ post }) => ({ id: post.id, kind: post.title })) + const union = new Query() + .unionAll(users, posts) + .where(({ kind }) => eq(kind, `active`)) + + return getQueryIR( + new Query().from({ [outerAlias]: union } as Record), + ) +} + +function createUnionDerivedNestedOutputQuery(outerAlias: string): QueryIR { + const users = new Query() + .from({ user: usersCollection }) + .select(({ user }) => ({ profile: { id: user.id } })) + const posts = new Query() + .from({ post: postsCollection }) + .select(({ post }) => ({ profile: { id: post.id } })) + const union = new Query() + .unionAll(users, posts) + .where(({ profile }) => eq(profile.id, 1)) + + return getQueryIR( + new Query().from({ [outerAlias]: union } as Record), + ) +} + +function createUnionDerivedIncludesQuery(parentAlias: string): QueryIR { + const firstPosts = new Query() + .from({ post: postsCollection }) + .select(({ post }) => ({ id: post.id, userId: post.userId })) + const secondPosts = new Query() + .from({ otherPost: postsCollection }) + .select(({ otherPost }) => ({ + id: otherPost.id, + userId: otherPost.userId, + })) + const childQuery = getQueryIR(new Query().unionAll(firstPosts, secondPosts)) + const posts = new IncludesSubquery( + childQuery, + new PropRef([parentAlias, `id`]), + new PropRef([`userId`]), + `posts`, + undefined, + undefined, + `array`, + ) + + return { + from: new CollectionRef( + usersCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + parentAlias, + ), + select: { + id: new PropRef([parentAlias, `id`]), + posts, + }, + } +} + +function createCorrelatedUnionIncludesQuery(parentAlias: string): QueryIR { + const createBranch = (childAlias: string): QueryIR => ({ + from: new CollectionRef( + postsCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + childAlias, + ), + select: { + profile: { id: new PropRef([childAlias, `id`]) }, + userId: new PropRef([childAlias, `userId`]), + parentAge: new PropRef([parentAlias, `age`]), + }, + }) + const childQuery: QueryIR = { + from: new UnionAll([createBranch(`firstPost`), createBranch(`secondPost`)]), + where: [new Func(`eq`, [new PropRef([`profile`, `id`]), new Value(1)])], + } + const posts = new IncludesSubquery( + childQuery, + new PropRef([parentAlias, `id`]), + new PropRef([`userId`]), + `posts`, + undefined, + [new PropRef([parentAlias, `age`])], + `array`, + ) + + return { + from: new CollectionRef( + usersCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + parentAlias, + ), + select: { + id: new PropRef([parentAlias, `id`]), + posts, + }, + } +} + +function createAlphaRenamedIncludesQuery( + parentAlias: string, + childAlias: string, +): QueryIR { + const childQuery: QueryIR = { + from: new CollectionRef( + postsCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + childAlias, + ), + select: { + id: new PropRef([childAlias, `id`]), + title: new PropRef([childAlias, `title`]), + }, + } + const posts = new IncludesSubquery( + childQuery, + new PropRef([parentAlias, `id`]), + new PropRef([childAlias, `userId`]), + `posts`, + [ + new Func(`eq`, [ + new PropRef([parentAlias, `status`]), + new Value(`active`), + ]), + ], + [new PropRef([parentAlias, `id`])], + `array`, + ) + return { + from: new CollectionRef( + usersCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + parentAlias, + ), + select: { + id: new PropRef([parentAlias, `id`]), + posts, + }, + } +} + const structuredQueries: Array<[string, () => QueryIR]> = [ [ `basic collection source`, @@ -359,6 +945,175 @@ describe(`stable QueryIR identity smoke test`, () => { ) }) + fcTest.prop([ + fc.uniqueArray(fc.stringMatching(/^[a-z][a-z0-9]{0,8}$/), { + minLength: 4, + maxLength: 4, + }), + ])(`does not depend on lexical source aliases`, (aliases) => { + const [firstUser, firstPost, secondUser, secondPost] = aliases + + expect( + getQueryIdentity(createAlphaRenamedJoinQuery(firstUser!, firstPost!)), + ).toBe( + getQueryIdentity(createAlphaRenamedJoinQuery(secondUser!, secondPost!)), + ) + expect( + getQueryIdentity(createAlphaRenamedNestedQuery(firstUser!, firstPost!)), + ).toBe( + getQueryIdentity(createAlphaRenamedNestedQuery(secondUser!, secondPost!)), + ) + expect( + getQueryIdentity(createAlphaRenamedIncludesQuery(firstUser!, firstPost!)), + ).toBe( + getQueryIdentity( + createAlphaRenamedIncludesQuery(secondUser!, secondPost!), + ), + ) + }) + + fcTest.prop([ + fc + .stringMatching(/^[a-z][a-z0-9]{0,8}$/) + .filter( + (alias) => + alias !== `kind` && alias !== `profile` && alias !== `userId`, + ), + ])(`does not bind union-derived output fields to outer aliases`, (alias) => { + expect(getQueryIdentity(createUnionDerivedNestedQuery(`kind`))).toBe( + getQueryIdentity(createUnionDerivedNestedQuery(alias)), + ) + expect( + getQueryIdentity(createUnionDerivedNestedOutputQuery(`profile`)), + ).toBe(getQueryIdentity(createUnionDerivedNestedOutputQuery(alias))) + expect(getQueryIdentity(createUnionDerivedIncludesQuery(`userId`))).toBe( + getQueryIdentity(createUnionDerivedIncludesQuery(alias)), + ) + expect( + getQueryIdentity(createCorrelatedUnionIncludesQuery(`profile`)), + ).toBe(getQueryIdentity(createCorrelatedUnionIncludesQuery(alias))) + }) + + it(`shares identity across equivalent predicate formulations`, () => { + const left = getQueryIR( + new Query() + .from({ user: usersCollection }) + .where(({ user }) => and(eq(user.status, `active`), gt(user.age, 18))), + ) + const right = getQueryIR( + new Query() + .from({ user: usersCollection }) + .where(({ user }) => and(lt(18, user.age), eq(`active`, user.status))), + ) + + expect(getQueryIdentity(left)).toBe(getQueryIdentity(right)) + }) + + it(`normalizes the implicit conjunction order of repeated where clauses`, () => { + const left = getQueryIR( + new Query() + .from({ user: usersCollection }) + .where(({ user }) => eq(user.status, `active`)) + .where(({ user }) => gt(user.age, 18)), + ) + const right = getQueryIR( + new Query() + .from({ user: usersCollection }) + .where(({ user }) => gt(user.age, 18)) + .where(({ user }) => eq(user.status, `active`)), + ) + const duplicate = getQueryIR( + new Query() + .from({ user: usersCollection }) + .where(({ user }) => eq(user.status, `active`)) + .where(({ user }) => gt(user.age, 18)) + .where(({ user }) => eq(user.status, `active`)), + ) + + expect(getQueryIdentity(left)).toBe(getQueryIdentity(right)) + expect(getQueryIdentity(left)).toBe(getQueryIdentity(duplicate)) + }) + + it(`normalizes the implicit conjunction order of repeated having clauses`, () => { + const left = getQueryIR( + new Query() + .from({ user: usersCollection }) + .groupBy(({ user }) => user.teamId) + .select(({ user }) => ({ + teamId: user.teamId, + userCount: count(user.id), + averageAge: avg(user.age), + })) + .having(({ $selected }) => gt($selected.userCount, 1)) + .having(({ $selected }) => gt($selected.averageAge, 18)), + ) + const right = getQueryIR( + new Query() + .from({ user: usersCollection }) + .groupBy(({ user }) => user.teamId) + .select(({ user }) => ({ + teamId: user.teamId, + userCount: count(user.id), + averageAge: avg(user.age), + })) + .having(({ $selected }) => gt($selected.averageAge, 18)) + .having(({ $selected }) => gt($selected.userCount, 1)), + ) + + expect(getQueryIdentity(left)).toBe(getQueryIdentity(right)) + }) + + it(`includes a query plan's result window`, () => { + const createQuery = (limit: number) => + getQueryIR( + new Query() + .from({ user: usersCollection }) + .orderBy(({ user }) => user.age) + .limit(limit), + ) + + expect(getQueryIdentity(createQuery(10))).not.toBe( + getQueryIdentity(createQuery(20)), + ) + }) + + it(`elides the default query offset`, () => { + const base = getQueryIR(new Query().from({ user: usersCollection })) + const offsetZero = getQueryIR( + new Query().from({ user: usersCollection }).offset(0), + ) + + expect(getQueryIdentity(base)).toBe(getQueryIdentity(offsetZero)) + }) + + it(`preserves function-argument and orderBy-clause order`, () => { + const subtractAge = getQueryIR( + new Query() + .from({ user: usersCollection }) + .orderBy(({ user }) => subtract(user.age, 1)) + .orderBy(({ user }) => user.name), + ) + const subtractFromOne = getQueryIR( + new Query() + .from({ user: usersCollection }) + .orderBy(({ user }) => subtract(1, user.age)) + .orderBy(({ user }) => user.name), + ) + const reversedClauses = getQueryIR( + new Query() + .from({ user: usersCollection }) + .orderBy(({ user }) => user.name) + .orderBy(({ user }) => subtract(user.age, 1)), + ) + + expect(getQueryIdentity(subtractAge)).not.toBe( + getQueryIdentity(subtractFromOne), + ) + expect(getQueryIdentity(subtractAge)).not.toBe( + getQueryIdentity(reversedClauses), + ) + }) + it(`preserves semantically significant union source ordering`, () => { const usersThenPosts = getQueryIR( new Query().unionAll({ user: usersCollection, post: postsCollection }), @@ -385,26 +1140,50 @@ describe(`stable QueryIR identity smoke test`, () => { ) }) - it(`normalizes object property ordering inside values`, () => { - const left = getQueryIR( - new Query().from({ user: usersCollection }).where(({ user }) => - eq(user.profile, { - skills: [`ts`, `db`], - experience: { years: 5 }, - }), - ), - ) + fcTest.prop([referenceSemanticPairArbitrary])( + `keeps queries distinct when captured values compare by reference`, + ([first, second]) => { + const firstQuery = createProfileValueQuery(first) + const secondQuery = createProfileValueQuery(second) + const firstPredicate = firstQuery.where![0] as BasicExpression + const secondPredicate = secondQuery.where![0] as BasicExpression + const row = { user: { profile: first } } - const right = getQueryIR( - new Query().from({ user: usersCollection }).where(({ user }) => - eq(user.profile, { - experience: { years: 5 }, - skills: [`ts`, `db`], - }), + expect(compileExpression(firstPredicate)(row)).toBe(true) + expect(compileExpression(secondPredicate)(row)).toBe(false) + expect(getQueryIdentity(firstQuery)).not.toBe( + getQueryIdentity(secondQuery), + ) + }, + ) + + it(`uses evaluator semantics for invalid Date and Temporal values`, () => { + expect(getQueryIdentity(createProfileValueQuery(new Date(`invalid`)))).toBe( + getQueryIdentity(createProfileValueQuery(new Date(`also invalid`))), + ) + expect( + getQueryIdentity( + createProfileValueQuery(Temporal.PlainDate.from(`2026-08-24`)), + ), + ).toBe( + getQueryIdentity( + createProfileValueQuery(Temporal.PlainDate.from(`2026-08-24`)), ), ) + }) - expect(getStableQueryIRHash(left)).toBe(getStableQueryIRHash(right)) + it(`normalizes object property ordering in structural value hashes`, () => { + expect( + getStableValueHash({ + skills: [`ts`, `db`], + experience: { years: 5 }, + }), + ).toBe( + getStableValueHash({ + experience: { years: 5 }, + skills: [`ts`, `db`], + }), + ) }) it(`keeps runtime values disjoint from internal identity tags`, () => { @@ -472,14 +1251,7 @@ describe(`stable QueryIR identity smoke test`, () => { } }) - it(`rejects opaque runtime values inside otherwise structured expressions`, () => { - const circularValue: Record = {} - circularValue.self = circularValue - - class OpaqueValue { - value = `Tanner` - } - + it(`rejects function and symbol values inside structured expressions`, () => { const queries = [ [ `function value`, @@ -499,35 +1271,6 @@ describe(`stable QueryIR identity smoke test`, () => { ), /symbol value/, ], - [ - `circular value`, - getQueryIR( - new Query() - .from({ user: usersCollection }) - .where(({ user }) => eq(user.profile, circularValue as never)), - ), - /circular value/, - ], - [ - `invalid date`, - getQueryIR( - new Query() - .from({ user: usersCollection }) - .where(({ user }) => - eq(user.createdAt, new Date(`invalid`) as never), - ), - ), - /invalid Date/, - ], - [ - `class instance`, - getQueryIR( - new Query() - .from({ user: usersCollection }) - .where(({ user }) => eq(user.name, new OpaqueValue() as never)), - ), - /non-plain object value/, - ], ] as const for (const [name, query, message] of queries) { @@ -537,4 +1280,20 @@ describe(`stable QueryIR identity smoke test`, () => { expect(() => getStableQueryIRHash(query), name).toThrow(message) } }) + + it(`accepts opaque object values by reference`, () => { + const circularValue: Record = {} + circularValue.self = circularValue + + class OpaqueValue { + value = `Tanner` + } + + expect(() => + getQueryIdentity(createProfileValueQuery(circularValue)), + ).not.toThrow() + expect(() => + getQueryIdentity(createProfileValueQuery(new OpaqueValue())), + ).not.toThrow() + }) }) diff --git a/packages/db/tests/query/load-subset-oracle.property.test.ts b/packages/db/tests/query/load-subset-oracle.property.test.ts index cf2645607c..fd6296e0b0 100644 --- a/packages/db/tests/query/load-subset-oracle.property.test.ts +++ b/packages/db/tests/query/load-subset-oracle.property.test.ts @@ -56,6 +56,7 @@ type WindowRequest = { direction: `asc` | `desc` nulls?: `first` | `last` stringSort?: `lexical` | `locale` + cursorBoundary?: number offset: number limit?: number } @@ -226,10 +227,23 @@ const rejectedWaiterScenarioArbitrary: fc.Arbitrary = const windowRequestArbitrary: fc.Arbitrary> = fc.record({ - orderField: fc.constantFrom(`none`, `rank`, `score`), - direction: fc.constantFrom(`asc`, `desc`), - nulls: fc.constantFrom(`first`, `last`), - stringSort: fc.constantFrom(`lexical`, `locale`), + orderField: fc.constantFrom>( + `none`, + `rank`, + `score`, + ), + direction: fc.constantFrom(`asc`, `desc`), + nulls: fc.constantFrom>( + `first`, + `last`, + ), + stringSort: fc.constantFrom>( + `lexical`, + `locale`, + ), + cursorBoundary: fc.option(fc.integer({ min: -3, max: 3 }), { + nil: undefined, + }), offset: fc.integer({ min: 0, max: 6 }), limit: fc.option(fc.integer({ min: 0, max: 6 }), { nil: undefined }), }) @@ -240,6 +254,9 @@ const finiteWindowRequestArbitrary: fc.Arbitrary> = direction: fc.constantFrom(`asc`, `desc`), nulls: fc.constantFrom(`first`, `last`), stringSort: fc.constantFrom(`lexical`, `locale`), + cursorBoundary: fc.option(fc.integer({ min: -3, max: 3 }), { + nil: undefined, + }), offset: fc.integer({ min: 0, max: 6 }), limit: fc.integer({ min: 0, max: 6 }), }) @@ -512,10 +529,25 @@ function readDedupeTrackingState(dedupe: DeduplicatedLoadSubset): { function toWindowOptions(request: WindowRequest): LoadSubsetOptions { const orderField = request.orderField ?? `rank` + const cursorRef = orderField === `score` ? scoreRef : rankRef return { where: request.where ? toWhere(request.where) : undefined, offset: request.offset, limit: request.limit, + cursor: + request.cursorBoundary === undefined + ? undefined + : { + whereFrom: new Func(request.direction === `asc` ? `gt` : `lt`, [ + cursorRef, + new Value(request.cursorBoundary), + ]), + whereCurrent: new Func(`eq`, [ + cursorRef, + new Value(request.cursorBoundary), + ]), + lastKey: request.cursorBoundary, + }, orderBy: orderField === `none` ? undefined @@ -552,6 +584,7 @@ type WindowCoverageDescriptor = { request: WindowRequest whereFingerprint: string orderFingerprint: string | undefined + cursorFingerprint: string | undefined matching: Set } @@ -565,6 +598,9 @@ function describeWindowCoverage( orderFingerprint: options.orderBy ? JSON.stringify(options.orderBy) : undefined, + cursorFingerprint: options.cursor + ? JSON.stringify(options.cursor) + : undefined, matching: matchingValues(options.where), } } @@ -576,10 +612,14 @@ function describedWindowCovers( if ( loaded.request.limit === undefined && loaded.request.offset === 0 && + loaded.cursorFingerprint === undefined && isSubset(requested.matching, loaded.matching) ) { return true } + if (requested.cursorFingerprint !== loaded.cursorFingerprint) { + return false + } if (requested.whereFingerprint !== loaded.whereFingerprint) return false if (requested.orderFingerprint === undefined) return true return requested.orderFingerprint === loaded.orderFingerprint @@ -1488,23 +1528,48 @@ describe(`loadSubset coverage oracle`, () => { ), ) - it( - `discovered trace: an identical filtered window reuses its load`, - expectAssertionFailure( - () => - Promise.resolve().then(() => { - const request: WindowRequest = { - where: { kind: `in`, values: [0] }, - orderField: `none`, - direction: `asc`, - offset: 0, - limit: 1, - } - expect(countWindowLoads([request, request])).toBe(1) - }), - { message: /expected 2 to be/ }, - ), - ) + it(`discovered trace: an identical filtered window reuses its load`, () => { + const request: WindowRequest = { + where: { kind: `in`, values: [0] }, + orderField: `none`, + direction: `asc`, + offset: 0, + limit: 1, + } + expect(countWindowLoads([request, request])).toBe(1) + }) + + it(`discovered trace: distinct cursor pages start distinct loads`, () => { + const request: WindowRequest = { + orderField: `rank`, + direction: `asc`, + offset: 0, + limit: 2, + cursorBoundary: 1, + } + + runWindowCoverageTrace([ + request, + { ...request, cursorBoundary: 2 }, + request, + ]) + }) + + it(`discovered trace: a cursor without a limit is not full coverage`, () => { + const request: WindowRequest = { + orderField: `rank`, + direction: `asc`, + offset: 0, + limit: undefined, + cursorBoundary: 1, + } + + runWindowCoverageTrace([ + request, + { ...request, cursorBoundary: 2 }, + { ...request, cursorBoundary: undefined }, + ]) + }) it(`rejects repeated transport work for one covered predicate`, () => { expect(() => @@ -2003,25 +2068,16 @@ describe(`loadSubset coverage oracle`, () => { ), ) - it( - `discovered trace: widening a window forgets an earlier covered window`, - expectAssertionFailure( - () => - Promise.resolve().then(() => { - const first: WindowRequest = { - orderField: `none`, - direction: `asc`, - offset: 0, - limit: 1, - where: { kind: `in`, values: [0] }, - } - expect(countWindowLoads([first, { ...first, limit: 2 }, first])).toBe( - 2, - ) - }), - { message: /expected 3 to be 2/ }, - ), - ) + it(`discovered trace: widening a window remembers an earlier covered window`, () => { + const first: WindowRequest = { + orderField: `none`, + direction: `asc`, + offset: 0, + limit: 1, + where: { kind: `in`, values: [0] }, + } + expect(countWindowLoads([first, { ...first, limit: 2 }, first])).toBe(2) + }) it( `discovered trace: complementary ranges redundantly reload an all-data request`, diff --git a/packages/db/tests/query/predicate-utils.test.ts b/packages/db/tests/query/predicate-utils.test.ts index 1f47eef23b..721c07f5ae 100644 --- a/packages/db/tests/query/predicate-utils.test.ts +++ b/packages/db/tests/query/predicate-utils.test.ts @@ -832,6 +832,23 @@ describe(`isPredicateSubset`, () => { expect(isPredicateSubset(subset, superset)).toBe(true) }) + it(`treats semantic predicate forms as equal coverage`, () => { + const age = ref(`age`) + const status = ref(`status`) + const ageCheck = gt(age, val(18)) + const statusCheck = eq(status, val(`active`)) + const subset: LoadSubsetOptions = { + where: func(`and`, ageCheck, statusCheck), + limit: 10, + } + const superset: LoadSubsetOptions = { + where: func(`and`, eq(val(`active`), status), func(`lt`, val(18), age)), + limit: 20, + } + + expect(isPredicateSubset(subset, superset)).toBe(true) + }) + it(`should return false for limited superset with different where clause`, () => { // Even if subset's where is more restrictive, it can't be a subset // of a limited superset with a different where clause. diff --git a/packages/query-db-collection/e2e/query-filter.ts b/packages/query-db-collection/e2e/query-filter.ts index aa3de76b15..cf9ac16508 100644 --- a/packages/query-db-collection/e2e/query-filter.ts +++ b/packages/query-db-collection/e2e/query-filter.ts @@ -3,7 +3,7 @@ * Uses expression helpers to implement proper predicate push-down */ -import { parseLoadSubsetOptions } from '@tanstack/db' +import { getLoadSubsetDemandKey, parseLoadSubsetOptions } from '@tanstack/db' import type { IR, LoadSubsetOptions, @@ -41,117 +41,11 @@ export function buildQueryKey( namespace: string, options: LoadSubsetOptions | undefined, ) { - return [`e2e`, namespace, serializeLoadSubsetOptions(options)] -} - -export function serializeLoadSubsetOptions( - options: LoadSubsetOptions | undefined, -): unknown { - if (!options) { - return null - } - - const result: Record = {} - - if (options.where) { - result.where = serializeExpression(options.where) - } - - if (options.orderBy?.length) { - result.orderBy = options.orderBy.map((clause) => ({ - expression: serializeExpression(clause.expression), - direction: clause.compareOptions.direction, - nulls: clause.compareOptions.nulls, - })) - } - - if (options.limit !== undefined) { - result.limit = options.limit - } - - // Include offset for pagination support - different offsets need different query keys - if (options.offset !== undefined) { - result.offset = options.offset - } - - return JSON.stringify(Object.keys(result).length === 0 ? null : result) -} - -function serializeExpression(expr: IR.BasicExpression | undefined): unknown { - if (!expr) { - return null - } - - switch (expr.type) { - case `val`: - return { - type: `val`, - value: serializeValue(expr.value), - } - case `ref`: - return { - type: `ref`, - path: [...expr.path], - } - case `func`: - return { - type: `func`, - name: expr.name, - args: expr.args.map((arg) => serializeExpression(arg)), - } - default: - return null - } -} - -function serializeValue(value: unknown): unknown { - if (value === undefined) { - return { __type: `undefined` } - } - - if (typeof value === `number`) { - if (Number.isNaN(value)) { - return { __type: `nan` } - } - if (value === Number.POSITIVE_INFINITY) { - return { __type: `infinity`, sign: 1 } - } - if (value === Number.NEGATIVE_INFINITY) { - return { __type: `infinity`, sign: -1 } - } - } - - if (typeof value === `bigint`) { - return { __type: `bigint`, value: value.toString() } - } - - if ( - value === null || - typeof value === `string` || - typeof value === `number` || - typeof value === `boolean` - ) { - return value - } - - if (value instanceof Date) { - return { __type: `date`, value: value.toJSON() } - } - - if (Array.isArray(value)) { - return value.map((item) => serializeValue(item)) - } - - if (typeof value === `object`) { - return Object.fromEntries( - Object.entries(value as Record).map(([key, val]) => [ - key, - serializeValue(val), - ]), - ) - } - - return value + return [ + `e2e`, + namespace, + options === undefined ? undefined : getLoadSubsetDemandKey(options), + ] } type Predicate = (item: T) => boolean diff --git a/packages/query-db-collection/src/query.ts b/packages/query-db-collection/src/query.ts index 5396bd49d6..7d611349fb 100644 --- a/packages/query-db-collection/src/query.ts +++ b/packages/query-db-collection/src/query.ts @@ -1,5 +1,9 @@ import { QueryObserver, hashKey } from '@tanstack/query-core' -import { deepEquals, withCollectionConfigFactory } from '@tanstack/db' +import { + deepEquals, + getLoadSubsetDemandKey, + withCollectionConfigFactory, +} from '@tanstack/db' import { GetKeyRequiredError, InitialDataInOnDemandModeError, @@ -8,7 +12,6 @@ import { QueryKeyRequiredError, } from './errors' import { createWriteUtils } from './manual-sync' -import { serializeLoadSubsetOptions } from './serialization' import type { BaseCollectionConfig, ChangeMessage, @@ -1215,10 +1218,10 @@ export function queryCollectionOptions( // Function-based queryKey: use it to build the key from opts return queryKey(opts) } else if (syncMode === `on-demand`) { - // Static queryKey in on-demand mode: automatically append serialized predicates - // to create separate cache entries for different predicate combinations - const serialized = serializeLoadSubsetOptions(opts) - return serialized !== undefined ? [...queryKey, serialized] : queryKey + // A static on-demand key is extended by exact semantic demand so + // equivalent predicates share one entry while distinct windows do not. + const demandKey = getLoadSubsetDemandKey(opts) + return demandKey !== undefined ? [...queryKey, demandKey] : queryKey } else { // Static queryKey in eager mode: use as-is return queryKey diff --git a/packages/query-db-collection/src/serialization.ts b/packages/query-db-collection/src/serialization.ts deleted file mode 100644 index 9849c4bd33..0000000000 --- a/packages/query-db-collection/src/serialization.ts +++ /dev/null @@ -1,135 +0,0 @@ -import type { IR, LoadSubsetOptions } from '@tanstack/db' - -/** - * Serializes LoadSubsetOptions into a stable, hashable format for query keys. - * Includes where, orderBy, limit, and offset for pagination support. - * Note: cursor expressions are not serialized as they are backend-specific. - * @internal - */ -export function serializeLoadSubsetOptions( - options: LoadSubsetOptions | undefined, -): string | undefined { - if (!options) { - return undefined - } - - const result: Record = {} - - if (options.where) { - result.where = serializeExpression(options.where) - } - - if (options.orderBy?.length) { - result.orderBy = options.orderBy.map((clause) => { - const baseOrderBy = { - expression: serializeExpression(clause.expression), - direction: clause.compareOptions.direction, - nulls: clause.compareOptions.nulls, - stringSort: clause.compareOptions.stringSort, - } - - // Handle locale-specific options when stringSort is 'locale' - if (clause.compareOptions.stringSort === `locale`) { - return { - ...baseOrderBy, - locale: clause.compareOptions.locale, - localeOptions: clause.compareOptions.localeOptions, - } - } - - return baseOrderBy - }) - } - - if (options.limit !== undefined) { - result.limit = options.limit - } - - // Include offset for pagination support - if (options.offset !== undefined) { - result.offset = options.offset - } - - return Object.keys(result).length === 0 ? undefined : JSON.stringify(result) -} - -/** - * Recursively serializes an IR expression for stable hashing - * @internal - */ -function serializeExpression(expr: IR.BasicExpression | undefined): unknown { - if (!expr) { - return null - } - - switch (expr.type) { - case `val`: - return { - type: `val`, - value: serializeValue(expr.value), - } - case `ref`: - return { - type: `ref`, - path: [...expr.path], - } - case `func`: - return { - type: `func`, - name: expr.name, - args: expr.args.map((arg) => serializeExpression(arg)), - } - default: - return null - } -} - -/** - * Serializes special JavaScript values (undefined, NaN, Infinity, Date) - * @internal - */ -function serializeValue(value: unknown): unknown { - if (value === undefined) { - return { __type: `undefined` } - } - - if (typeof value === `number`) { - if (Number.isNaN(value)) { - return { __type: `nan` } - } - if (value === Number.POSITIVE_INFINITY) { - return { __type: `infinity`, sign: 1 } - } - if (value === Number.NEGATIVE_INFINITY) { - return { __type: `infinity`, sign: -1 } - } - } - - if ( - value === null || - typeof value === `string` || - typeof value === `number` || - typeof value === `boolean` - ) { - return value - } - - if (value instanceof Date) { - return { __type: `date`, value: value.toJSON() } - } - - if (Array.isArray(value)) { - return value.map((item) => serializeValue(item)) - } - - if (typeof value === `object`) { - return Object.fromEntries( - Object.entries(value as Record).map(([key, val]) => [ - key, - serializeValue(val), - ]), - ) - } - - return value -} diff --git a/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts b/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts index 3e13faf653..38dc3e9a19 100644 --- a/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts +++ b/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts @@ -7,7 +7,6 @@ import { eq, } from '@tanstack/db' import { describe, expect, it, vi } from 'vitest' -import { expectAssertionFailure } from '../../db/tests/expected-failure.js' import { TraceAssertionError } from '../../db/tests/trace-runner.js' import { queryCollectionOptions } from '../src/query.js' import type { QueryFunctionContext } from '@tanstack/query-core' @@ -348,7 +347,7 @@ async function expectDeferredStartupReadyDoesNotOverrideError(): Promise { } async function expectEquivalentPredicatesShareOneLoad( - form: `commutative-and` | `reversed-equality`, + form: `commutative-and` | `commutative-or` | `reversed-equality`, ): Promise { const queryClient = createQueryClient() const id = `load-subset-canonical-predicate-${collectionSequence++}` @@ -373,14 +372,22 @@ async function expectEquivalentPredicatesShareOneLoad( new IR.PropRef([`group`]), new IR.Value(`x`), ]) - const first = - form === `commutative-and` - ? new IR.Func(`and`, [firstComparison, secondComparison]) - : firstComparison - const second = - form === `commutative-and` - ? new IR.Func(`and`, [secondComparison, firstComparison]) - : new IR.Func(`eq`, [new IR.Value(`a`), new IR.PropRef([`id`])]) + let first: IR.BasicExpression + let second: IR.BasicExpression + switch (form) { + case `commutative-and`: + first = new IR.Func(`and`, [firstComparison, secondComparison]) + second = new IR.Func(`and`, [secondComparison, firstComparison]) + break + case `commutative-or`: + first = new IR.Func(`or`, [firstComparison, secondComparison]) + second = new IR.Func(`or`, [secondComparison, firstComparison]) + break + case `reversed-equality`: + first = firstComparison + second = new IR.Func(`eq`, [new IR.Value(`a`), new IR.PropRef([`id`])]) + break + } try { await collection._sync.loadSubset({ where: first }) @@ -396,6 +403,41 @@ async function expectEquivalentPredicatesShareOneLoad( } } +async function expectEquivalentComparisonValuesShareOneLoad( + firstValue: unknown, + secondValue: unknown, +): Promise { + const queryClient = createQueryClient() + const id = `load-subset-comparison-value-${collectionSequence++}` + const queryFn = vi.fn().mockResolvedValue([{ id: `a` }]) + const collection = createCollection( + queryCollectionOptions({ + id, + queryClient, + queryKey: [id], + queryFn, + getKey: (row) => row.id, + startSync: true, + syncMode: `on-demand`, + retry: false, + }), + ) + const value = new IR.PropRef([`value`]) + + try { + await collection._sync.loadSubset({ + where: new IR.Func(`eq`, [value, new IR.Value(firstValue)]), + }) + await collection._sync.loadSubset({ + where: new IR.Func(`eq`, [value, new IR.Value(secondValue)]), + }) + expect(queryFn).toHaveBeenCalledOnce() + } finally { + await collection.cleanup() + queryClient.clear() + } +} + async function expectFinalOwnerCleanupAbortsQuery(): Promise { const queryClient = createQueryClient() const id = `load-subset-cancel-final-owner-${collectionSequence++}` @@ -526,19 +568,31 @@ describe(`loadSubset lifecycle oracle`, () => { }) it(`commutative predicate forms share one query-db transport load`, async () => { - await expectAssertionFailure(expectEquivalentPredicatesShareOneLoad, { - checkpoint: 0, - classify: ({ actual, expected }) => actual === 2 && expected === 1, - })(`commutative-and`) + await expectEquivalentPredicatesShareOneLoad(`commutative-and`) + await expectEquivalentPredicatesShareOneLoad(`commutative-or`) }) it(`reversed equality operands share one query-db transport load`, async () => { - await expectAssertionFailure(expectEquivalentPredicatesShareOneLoad, { - checkpoint: 0, - classify: ({ actual, expected }) => actual === 2 && expected === 1, - })(`reversed-equality`) + await expectEquivalentPredicatesShareOneLoad(`reversed-equality`) }) + it.each([ + [ + `valid Date`, + new Date(`2024-01-15T00:00:00Z`), + new Date(`2024-01-15T00:00:00Z`), + ], + [`invalid Date`, new Date(Number.NaN), new Date(Number.NaN)], + ])( + `shares one query-db transport load for equivalent %s values`, + async (_label, firstValue, secondValue) => { + await expectEquivalentComparisonValuesShareOneLoad( + firstValue, + secondValue, + ) + }, + ) + it(`aborts an in-flight query when its final live-query owner cleans up`, async () => { await expectFinalOwnerCleanupAbortsQuery() }) From 1d003a41688bf89a4dc752df8dfdc99c94cadb46 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 17:45:52 -0600 Subject: [PATCH 02/14] docs: add canonical demand identity changeset --- .changeset/canonical-demand-identity.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/canonical-demand-identity.md diff --git a/.changeset/canonical-demand-identity.md b/.changeset/canonical-demand-identity.md new file mode 100644 index 0000000000..dc4d0602d4 --- /dev/null +++ b/.changeset/canonical-demand-identity.md @@ -0,0 +1,6 @@ +--- +'@tanstack/db': patch +'@tanstack/query-db-collection': patch +--- + +Canonicalize equivalent loadSubset queries to one demand identity while preserving distinct runtime values and window requests. Query DB now reuses the same canonical identity for its on-demand cache keys. From 53c270c3af52871b0e0157ff66478a5814d8747d Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 18:23:24 -0600 Subject: [PATCH 03/14] fix(db): fall back when runtime crypto is incomplete --- .../db/src/query/runtime-reference-identity.ts | 4 ++-- .../db/tests/query/ir-stable-identity.test.ts | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/db/src/query/runtime-reference-identity.ts b/packages/db/src/query/runtime-reference-identity.ts index d903060522..15d7b82b6d 100644 --- a/packages/db/src/query/runtime-reference-identity.ts +++ b/packages/db/src/query/runtime-reference-identity.ts @@ -27,9 +27,9 @@ export const getRuntimeReferenceIdentity = function createRuntimeReferenceNamespace(): string { const randomValues = new Uint32Array(4) const runtimeCrypto = Reflect.get(globalThis, `crypto`) as - | { getRandomValues: (values: Uint32Array) => Uint32Array } + | { getRandomValues?: (values: Uint32Array) => Uint32Array } | undefined - if (runtimeCrypto !== undefined) { + if (typeof runtimeCrypto?.getRandomValues === `function`) { runtimeCrypto.getRandomValues(randomValues) return Array.from(randomValues, (value) => value.toString(36)).join(`-`) } diff --git a/packages/db/tests/query/ir-stable-identity.test.ts b/packages/db/tests/query/ir-stable-identity.test.ts index 3d9bd86053..8a5447557f 100644 --- a/packages/db/tests/query/ir-stable-identity.test.ts +++ b/packages/db/tests/query/ir-stable-identity.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { fc, test as fcTest } from '@fast-check/vitest' import { Temporal } from 'temporal-polyfill' import { CollectionImpl } from '../../src/collection/index.js' @@ -279,6 +279,21 @@ describe(`semantic expression identity`, () => { expect(firstRuntime({ a: 1 })).not.toEqual(secondRuntime({ b: 2 })) }) + it(`falls back when the runtime crypto object lacks getRandomValues`, () => { + vi.stubGlobal(`crypto`, {}) + try { + const runtime = createRuntimeReferenceIdentityFactory() + + expect(runtime({ a: 1 })).toEqual([ + `runtimeReference`, + expect.any(String), + 1, + ]) + } finally { + vi.unstubAllGlobals() + } + }) + fcTest.prop([ fc.uniqueArray(fc.oneof(fc.integer(), fc.string(), fc.boolean()), { minLength: 1, From 4eda84f874619a4f197d5036708cad8c6ed5f02d Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 18:39:45 -0600 Subject: [PATCH 04/14] fix(db): settle subset loads after publication --- AGENTS.md | 10 + .../src/persisted.ts | 114 ++- .../tests/persisted.test.ts | 73 ++ packages/db/src/client.ts | 2 + packages/db/src/collection/state.ts | 29 + packages/db/src/collection/sync.ts | 27 +- packages/db/src/query/live/ARCHITECTURE.md | 35 +- packages/db/src/types.ts | 20 +- .../tests/query/bucket-facade-adapter.test.ts | 3 +- .../query/load-subset-oracle.property.test.ts | 651 +++++++++++++++++- .../electric-db-collection/src/electric.ts | 48 +- .../tests/electric.test.ts | 40 ++ .../powersync-db-collection/src/powersync.ts | 139 ++-- .../tests/on-demand-sync.test.ts | 77 +++ .../query-db-collection/src/manual-sync.ts | 8 +- packages/query-db-collection/src/query.ts | 235 ++++--- .../query-db-collection/tests/query.test.ts | 139 ++++ packages/rxdb-db-collection/src/rxdb.ts | 34 +- .../rxdb-db-collection/tests/rxdb.test.ts | 82 ++- .../trailbase-db-collection/src/trailbase.ts | 4 +- .../tests/trailbase.test.ts | 111 ++- 21 files changed, 1658 insertions(+), 223 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ac209cf93d..1d6fbf182b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -379,6 +379,16 @@ test('ignores snapshot that resolves after up-to-date message', async () => { }) ``` +### Treat Every Review Bug as a Test Gap + +When a reviewer agent confirms a bug, it must also ask why the existing tests +did not catch it. The finding should name the missing test law, state +transition, generator dimension, adapter boundary, or assertion. If a test or +oracle should already have caught the bug, identify the false-green model, +classifier, fixture, or assertion that let it pass. Use that analysis to suggest +the smallest test or oracle improvement that would catch the same class of bug, +not only the reported example. + ### Name Tests After Behavior Test names should state the behavior they prove. Do not put issue or pull diff --git a/packages/db-sqlite-persistence-core/src/persisted.ts b/packages/db-sqlite-persistence-core/src/persisted.ts index be32f3eb00..fb02f35643 100644 --- a/packages/db-sqlite-persistence-core/src/persisted.ts +++ b/packages/db-sqlite-persistence-core/src/persisted.ts @@ -22,6 +22,7 @@ import type { InsertMutationFnParams, LoadSubsetOptions, PendingMutation, + SyncAppliedReceipt, SyncConfig, SyncConfigRes, SyncMetadataApi, @@ -433,7 +434,7 @@ type SyncControlFns = { | { type: `delete`; key: TKey }, ) => void) | null - commit: (() => void) | null + commit: (() => SyncAppliedReceipt) | null truncate: (() => void) | null metadata: SyncMetadataApi | null } @@ -586,6 +587,7 @@ type BufferedSyncTransaction = { > truncate: boolean internal: boolean + resolveApplied?: () => void } type OpenSyncTransaction< @@ -811,6 +813,8 @@ class PersistedCollectionRuntime< private startupMetadataPromise: Promise | null = null private startPromise: Promise | null = null private internalApplyDepth = 0 + private appliedReceiptSequence = 0 + private readonly pendingAppliedReceipts = new Map>() private isHydrating = false private coordinatorUnsubscribe: (() => void) | null = null private indexAddedUnsubscribe: (() => void) | null = null @@ -834,7 +838,29 @@ class PersistedCollectionRuntime< ) {} setSyncControls(syncControls: SyncControlFns): void { - this.syncControls = syncControls + const commit = syncControls.commit + this.syncControls = { + ...syncControls, + commit: commit ? () => this.trackAppliedReceipt(commit()) : null, + } + } + + private trackAppliedReceipt(receipt: SyncAppliedReceipt): SyncAppliedReceipt { + const sequence = ++this.appliedReceiptSequence + if (receipt === true) { + return true + } + this.pendingAppliedReceipts.set(sequence, receipt) + void receipt.then(() => this.pendingAppliedReceipts.delete(sequence)) + return receipt + } + + private async waitForAppliedReceiptsAfter(cursor: number): Promise { + await Promise.all( + Array.from(this.pendingAppliedReceipts, ([sequence, receipt]) => + sequence > cursor ? receipt : undefined, + ), + ) } clearSyncControls(): void { @@ -906,9 +932,11 @@ class PersistedCollectionRuntime< if (this.syncMode !== `on-demand`) { this.activeSubsets.set(this.getSubsetKey({}), {}) + const appliedCursor = this.appliedReceiptSequence await this.applyMutex.run(() => this.hydrateSubsetUnsafe({}, { requestRemoteEnsure: false }), ) + await this.waitForAppliedReceiptsAfter(appliedCursor) } } @@ -985,17 +1013,19 @@ class PersistedCollectionRuntime< ): Promise { this.activeSubsets.set(this.getSubsetKey(options), options) + const appliedCursor = this.appliedReceiptSequence await this.applyMutex.run(() => this.hydrateSubsetUnsafe(options, { requestRemoteEnsure: this.mode === `sync-present`, }), ) + await this.waitForAppliedReceiptsAfter(appliedCursor) if (upstreamLoadSubset) { try { const maybePromise = upstreamLoadSubset(options) if (maybePromise instanceof Promise) { - maybePromise.catch((error) => { + await maybePromise.catch((error) => { console.warn( `Failed to load remote subset in persisted wrapper:`, error, @@ -1156,15 +1186,18 @@ class PersistedCollectionRuntime< this.pendingRemoteSubsetEnsures.clear() this.activeSubsets.clear() + for (const transaction of this.queuedHydrationTransactions) { + transaction.resolveApplied?.() + } this.queuedHydrationTransactions.length = 0 this.queuedTxCommitted.length = 0 this.clearSyncControls() } - private withInternalApply(task: () => void): void { + private withInternalApply(task: () => TResult): TResult { this.internalApplyDepth++ try { - task() + return task() } finally { this.internalApplyDepth-- } @@ -1318,29 +1351,28 @@ class PersistedCollectionRuntime< private async applyBufferedSyncTransactionUnsafe( transaction: BufferedSyncTransaction, ): Promise { - if ( - !this.syncControls.begin || - !this.syncControls.write || - !this.syncControls.commit - ) { + const { begin, write, commit, truncate, metadata } = this.syncControls + if (!begin || !write || !commit) { + transaction.resolveApplied?.() return } - const applyToCollection = () => { - this.syncControls.begin?.() + let receiptLinkedToCoreApplication = false + const applyToCollection = (): boolean => { + begin() if (transaction.truncate) { - this.syncControls.truncate?.() + truncate?.() } for (const operation of transaction.operations) { if (operation.type === `delete`) { - this.syncControls.write?.({ + write({ type: `delete`, key: operation.key, }) } else { - this.syncControls.write?.({ + write({ type: `update`, value: operation.value, metadata: operation.metadata, @@ -1350,30 +1382,47 @@ class PersistedCollectionRuntime< for (const [key, metadataWrite] of transaction.rowMetadataWrites) { if (metadataWrite.type === `delete`) { - this.syncControls.metadata?.row.delete(key) + metadata?.row.delete(key) } else { - this.syncControls.metadata?.row.set(key, metadataWrite.value) + metadata?.row.set(key, metadataWrite.value) } } for (const [key, metadataWrite] of transaction.collectionMetadataWrites) { if (metadataWrite.type === `delete`) { - this.syncControls.metadata?.collection.delete(key) + metadata?.collection.delete(key) } else { - this.syncControls.metadata?.collection.set(key, metadataWrite.value) + metadata?.collection.set(key, metadataWrite.value) } } - this.syncControls.commit?.() + const applied = commit() + if (applied === true) { + transaction.resolveApplied?.() + return false + } else { + void applied.then(() => transaction.resolveApplied?.()) + return true + } } - if (transaction.internal) { - this.withInternalApply(applyToCollection) - return - } + try { + if (transaction.internal) { + receiptLinkedToCoreApplication = + this.withInternalApply(applyToCollection) + return + } - applyToCollection() - await this.persistAndBroadcastExternalSyncTransactionUnsafe(transaction) + receiptLinkedToCoreApplication = applyToCollection() + await this.persistAndBroadcastExternalSyncTransactionUnsafe(transaction) + } catch (error) { + // A replay failure before commit has no core receipt that cleanup can + // settle. Release the wrapper receipt so the source load cannot hang. + if (!receiptLinkedToCoreApplication) { + transaction.resolveApplied?.() + } + throw error + } } private async persistAndBroadcastExternalSyncTransactionUnsafe( @@ -2460,11 +2509,14 @@ function createWrappedSyncConfig< commit: () => { const openTransaction = transactionStack.pop() if (!openTransaction) { - params.commit() - return + return params.commit() } if (openTransaction.queuedBecauseHydrating) { + let resolveApplied!: () => void + const applied = new Promise((resolve) => { + resolveApplied = resolve + }) runtime.queueHydrationBufferedTransaction({ operations: openTransaction.operations, rowMetadataWrites: openTransaction.rowMetadataWrites, @@ -2472,11 +2524,12 @@ function createWrappedSyncConfig< openTransaction.collectionMetadataWrites, truncate: openTransaction.truncate, internal: openTransaction.internal, + resolveApplied, }) - return + return applied } - params.commit() + const applied = params.commit() if (!openTransaction.internal) { void runtime .persistAndBroadcastExternalSyncTransaction({ @@ -2494,6 +2547,7 @@ function createWrappedSyncConfig< ) }) } + return applied }, } diff --git a/packages/db-sqlite-persistence-core/tests/persisted.test.ts b/packages/db-sqlite-persistence-core/tests/persisted.test.ts index 78087419fa..abcc499f48 100644 --- a/packages/db-sqlite-persistence-core/tests/persisted.test.ts +++ b/packages/db-sqlite-persistence-core/tests/persisted.test.ts @@ -1099,6 +1099,79 @@ describe(`persistedCollectionOptions`, () => { }) }) + it(`settles a hydration-buffered receipt when replay fails`, async () => { + const adapter = createRecordingAdapter() + let resolveLoadSubset: (() => void) | undefined + adapter.loadSubset = async () => { + await new Promise((resolve) => { + resolveLoadSubset = resolve + }) + return [] + } + + const replayError = new Error(`replay key failed`) + let bufferedRowKeyReads = 0 + let remoteBegin: (() => void) | undefined + let remoteWrite: + | ((message: { type: `insert`; value: Todo }) => void) + | undefined + let remoteCommit: (() => true | Promise) | undefined + + const collection = createCollection( + persistedCollectionOptions({ + id: `sync-present-replay-failure-receipt`, + getKey: (item) => { + if (item.id === `during-hydrate`) { + bufferedRowKeyReads++ + if (bufferedRowKeyReads === 2) { + throw replayError + } + } + return item.id + }, + sync: { + sync: ({ begin, write, commit, markReady }) => { + remoteBegin = begin + remoteWrite = write as (message: { + type: `insert` + value: Todo + }) => void + remoteCommit = commit + markReady() + return {} + }, + }, + persistence: { + adapter, + }, + }), + ) + + const readyPromise = collection.stateWhenReady() + for (let attempt = 0; attempt < 20 && !resolveLoadSubset; attempt++) { + await flushAsyncWork() + } + + remoteBegin?.() + remoteWrite?.({ + type: `insert`, + value: { id: `during-hydrate`, title: `During hydrate` }, + }) + const receipt = remoteCommit?.() + expect(receipt).toBeInstanceOf(Promise) + let receiptSettled = false + void Promise.resolve(receipt).then(() => { + receiptSettled = true + }) + + resolveLoadSubset?.() + await readyPromise + await collection.cleanup() + await flushAsyncWork() + + expect(receiptSettled).toBe(true) + }) + it(`marks ready even when persisted startup fails before markReady`, async () => { const adapter = createRecordingAdapter() adapter.loadSubset = async () => { diff --git a/packages/db/src/client.ts b/packages/db/src/client.ts index 7ba774aa6c..e954eb35e5 100644 --- a/packages/db/src/client.ts +++ b/packages/db/src/client.ts @@ -8,6 +8,7 @@ import { TransactionScope } from './transactions.js' import { getBuilderFromConfig } from './query/live/collection-registry.js' import { createLiveQueryCollection } from './query/live-query-collection.js' import { createLiveQueryObserver } from './live-query-observer.js' +import { createDeferred } from './deferred.js' import { getLiveQueryHash, prepareLiveQueryValue, @@ -877,6 +878,7 @@ export class DbClient { deletedKeys: new Set(), rowMetadataWrites, collectionMetadataWrites: new Map(), + applied: createDeferred(), immediate: true, preserveHydrationSeedKeys: seedKind !== undefined, }) diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index 69b8b6f9cf..d5cb986a9c 100644 --- a/packages/db/src/collection/state.ts +++ b/packages/db/src/collection/state.ts @@ -19,6 +19,7 @@ import type { CollectionLifecycleManager } from './lifecycle' import type { CollectionChangesManager } from './changes' import type { CollectionIndexesManager } from './indexes' import type { CollectionEventsManager } from './events' +import type { Deferred } from '../deferred' interface PendingSyncedTransaction< T extends object = Record, @@ -31,6 +32,8 @@ interface PendingSyncedTransaction< deletedKeys: Set rowMetadataWrites: Map collectionMetadataWrites: Map + /** Resolves after this transaction's writes and events are visible. */ + applied: Deferred optimisticSnapshot?: { upserts: Map deletes: Set @@ -1360,6 +1363,27 @@ export class CollectionStateManager< if (!this.hasReceivedFirstCommit) { this.hasReceivedFirstCommit = true } + + for (const transaction of committedSyncedTransactions) { + transaction.applied.resolve() + } + } + } + + /** Abandons one committed transaction before it becomes visible. */ + public cancelPendingSyncedTransaction( + transaction: PendingSyncedTransaction, + ): void { + const index = this.pendingSyncedTransactions.indexOf(transaction) + if (index === -1) return + + this.pendingSyncedTransactions.splice(index, 1) + transaction.applied.resolve() + + if (this.pendingSyncedTransactions.length === 0) { + this.preSyncVisibleState.clear() + this.recentlySyncedKeys.clear() + this.changes.emitEvents([], true) } } @@ -1439,6 +1463,11 @@ export class CollectionStateManager< * This can be called manually or automatically by garbage collection */ public cleanup(): void { + for (const transaction of this.pendingSyncedTransactions) { + // Applied receipts never reject. Cleanup abandons the collection and + // releases callers that may have retained and ignored a receipt. + transaction.applied.resolve() + } this.syncedData.clear() this.syncedMetadata.clear() this.syncedCollectionMetadata.clear() diff --git a/packages/db/src/collection/sync.ts b/packages/db/src/collection/sync.ts index 5a8f49f8e8..4c7539417a 100644 --- a/packages/db/src/collection/sync.ts +++ b/packages/db/src/collection/sync.ts @@ -134,6 +134,7 @@ export class CollectionSyncManager< rowMetadataWrites: new Map(), collectionMetadataWrites: new Map(), immediate: options?.immediate, + applied: createDeferred(), }) }, write: ( @@ -231,8 +232,8 @@ export class CollectionSyncManager< }) } }, - commit: () => { - if (!isCurrentSync()) return + commit: (signal?: AbortSignal) => { + if (!isCurrentSync()) return true const pendingTransaction = this.state.pendingSyncedTransactions[ this.state.pendingSyncedTransactions.length - 1 @@ -244,9 +245,31 @@ export class CollectionSyncManager< throw new SyncTransactionAlreadyCommittedError() } + if (signal?.aborted) { + this.state.cancelPendingSyncedTransaction(pendingTransaction) + return true + } + pendingTransaction.committed = true + const cancel = () => { + this.state.cancelPendingSyncedTransaction(pendingTransaction) + } + signal?.addEventListener(`abort`, cancel, { once: true }) + this.state.commitPendingTransactions() + if (!pendingTransaction.applied.isPending()) { + signal?.removeEventListener(`abort`, cancel) + return true + } + + const receipt = pendingTransaction.applied.promise + if (signal) { + void receipt.then(() => { + signal.removeEventListener(`abort`, cancel) + }) + } + return receipt }, markReady: () => { if (isCurrentSync()) this.lifecycle.markReady() diff --git a/packages/db/src/query/live/ARCHITECTURE.md b/packages/db/src/query/live/ARCHITECTURE.md index ad44e21ae5..29bdef5868 100644 --- a/packages/db/src/query/live/ARCHITECTURE.md +++ b/packages/db/src/query/live/ARCHITECTURE.md @@ -26,8 +26,9 @@ This architecture covers: - coherent publication to public Collections; - the boundaries with query-db ownership and physical query planning. -It does not define new public APIs. Optimistic transactions are another source -of weighted input changes; they do not have a separate routing model. +The applied-settlement receipt described below is its only new public boundary +contract. Optimistic transactions are another source of weighted input changes; +they do not have a separate routing model. ## One relational graph @@ -448,6 +449,19 @@ adapter from writing after it ignores that signal. Buffering, snapshot tokens, shape offsets, Collection transactions, and local indexes are source-specific ways to satisfy that contract; they are not materializer state. +Every sync `commit()` returns an applied receipt: `true` when that +transaction's writes and events are already visible, or a promise when the +transaction is parked in the causal queue. The promise resolves only after the +writes and events become visible, or after collection cleanup abandons the +transaction. A successful `loadSubset` implementation must await or return +every receipt for the transactions that establish its result. A source must +not add priority merely to make a subset load settle. +Existing immediate bootstrap and persistence-hydration paths, plus truncate, +retain their queue-bypass contract; if one applies a parked subset transaction +as part of that prefix, the subset receipt settles only after the writes are +visible. Rejected, canceled, and obsolete acquisitions establish no coverage. +Sources must honor cancellation before publishing request-scoped rows. + This project uses a single graph-run order rather than multi-dimensional timely-dataflow frontiers. Do not introduce a general timestamp or frontier framework unless a source contract proves that the generation and up-to-date @@ -534,17 +548,20 @@ create recursive Collection machinery. 6. **Stale demand:** an obsolete graph or demand generation cannot settle current readiness, and a conforming source cannot publish its request-scoped rows after cancellation. -7. **Nested propagation:** every materialized relation consumes the fully +7. **Applied settlement:** a successful subset load settles only after its + establishing sync transactions are visible; a source must not add queue + priority merely to force the load to settle. +8. **Nested propagation:** every materialized relation consumes the fully materialized output relation of its children. -8. **Publication:** reads, events, and downstream queries observe the same +9. **Publication:** reads, events, and downstream queries observe the same complete graph result. -9. **Initial demand:** preload completes when every initially reachable demand - is covered; obsolete demand does not block it. -10. **Ownership:** a query-db row exists exactly while an explicit owner +10. **Initial demand:** preload completes when every initially reachable demand + is covered; obsolete demand does not block it. +11. **Ownership:** a query-db row exists exactly while an explicit owner remains. -11. **Work:** irrelevant rows do not cause unrelated scans or activate unrelated +12. **Work:** irrelevant rows do not cause unrelated scans or activate unrelated routes when an applicable index exists. -12. **Space:** state scales with retained D2 relation/index rows, active demands, +13. **Space:** state scales with retained D2 relation/index rows, active demands, materialization cells, visible rows, and required Collection facades. ## Glossary diff --git a/packages/db/src/types.ts b/packages/db/src/types.ts index 45f0ddd046..f49797e784 100644 --- a/packages/db/src/types.ts +++ b/packages/db/src/types.ts @@ -333,10 +333,18 @@ export type LoadSubsetOptions = { /** * Loads one subset and transfers its ongoing resource ownership only after * returning `true` or a promise. An implementation that throws synchronously - * must release any partially acquired resource before throwing. + * must release any partially acquired resource before throwing. A successful + * implementation must await or return every applied receipt from the sync + * `commit()` calls that establish the loaded subset. */ export type LoadSubsetFn = (options: LoadSubsetOptions) => true | Promise +/** + * Confirms whether a committed sync transaction is already visible or is + * waiting for its turn in the collection's causal queue. + */ +export type SyncAppliedReceipt = true | Promise + export type UnloadSubsetFn = (options: LoadSubsetOptions) => void export type CleanupFn = () => void @@ -359,7 +367,15 @@ export interface SyncConfig< */ begin: (options?: { immediate?: boolean }) => void write: (message: ChangeMessageOrDeleteKeyMessage) => void - commit: () => void + /** + * Commit the active sync transaction in FIFO order. + * Returns `true` when the writes and events are already visible. Otherwise + * returns a receipt that resolves after they become visible, or after + * collection cleanup or an optional request abort abandons the transaction. + * Pass a signal only for request-scoped work that must not publish after + * cancellation. The receipt never rejects. + */ + commit: (signal?: AbortSignal) => SyncAppliedReceipt /** Signal that a usable initial or recovered snapshot is available. */ markReady: () => void /** diff --git a/packages/db/tests/query/bucket-facade-adapter.test.ts b/packages/db/tests/query/bucket-facade-adapter.test.ts index 2992656bad..4b969e2114 100644 --- a/packages/db/tests/query/bucket-facade-adapter.test.ts +++ b/packages/db/tests/query/bucket-facade-adapter.test.ts @@ -69,11 +69,12 @@ describe(`BucketFacadeAdapter`, () => { const commit = sync.commit let shouldThrow = true sync.commit = () => { - commit() + const applied = commit() if (shouldThrow) { shouldThrow = false throw new Error(`facade flush failed`) } + return applied } const replacement = { id: 1, value: `replacement` } diff --git a/packages/db/tests/query/load-subset-oracle.property.test.ts b/packages/db/tests/query/load-subset-oracle.property.test.ts index cf2645607c..21d559978d 100644 --- a/packages/db/tests/query/load-subset-oracle.property.test.ts +++ b/packages/db/tests/query/load-subset-oracle.property.test.ts @@ -15,7 +15,7 @@ import { import { evaluateReferenceExpression } from '../reference-expression.js' import { TraceAssertionError } from '../trace-runner.js' import type { BasicExpression } from '../../src/query/ir.js' -import type { LoadSubsetOptions } from '../../src/types.js' +import type { LoadSubsetOptions, SyncAppliedReceipt } from '../../src/types.js' type PredicateSpec = | { kind: `all` } @@ -79,6 +79,15 @@ type CoverageSubjectFactory = ( recordLoad: (options: LoadSubsetOptions) => true | Promise, ) => CoverageSubject +function requirePendingAppliedReceipt( + receipt: SyncAppliedReceipt, +): Promise { + if (receipt === true) { + throw new Error(`Expected an asynchronous subset load`) + } + return receipt +} + class CoveredDemandRefetchedError extends Error { constructor( readonly checkpoint: number, @@ -1033,7 +1042,11 @@ const coverageRandomParameters = oracleRandomParameters( let collectionSequence = 0 -async function expectPersistingLoadIsApplied(persisting: boolean) { +async function expectPersistingLoadIsApplied( + persisting: boolean, + delivery: `synchronous` | `asynchronous` = `synchronous`, + transactionStart: `during-load` | `before-load` = `during-load`, +) { const rows: Array = [ { id: `r1`, projectId: `p1` }, { id: `r2`, projectId: `p1` }, @@ -1045,16 +1058,25 @@ async function expectPersistingLoadIsApplied(persisting: boolean) { syncMode: `on-demand`, sync: { sync: ({ begin, write, commit, markReady }) => { + if (transactionStart === `before-load`) begin() markReady() return { loadSubset: () => { loadCalls += 1 - begin() - for (const row of rows) { - write({ type: `insert`, value: { ...row } }) + const applyRows = () => { + if (transactionStart === `during-load`) begin() + for (const row of rows) { + write({ type: `insert`, value: { ...row } }) + } + return commit() + } + if (delivery === `synchronous`) { + return applyRows() } - commit() - return Promise.resolve() + return Promise.resolve().then(async () => { + const applied = applyRows() + if (applied !== true) await applied + }) }, } }, @@ -1073,7 +1095,24 @@ async function expectPersistingLoadIsApplied(persisting: boolean) { ) try { - const result = await live.toArrayWhenReady() + const ready = live.toArrayWhenReady() + if (persisting) { + let settled = false + void ready.then(() => { + settled = true + }) + await Promise.resolve() + await Promise.resolve() + + expect(settled).toBe(false) + expect(source.get(`r1`)).toBeUndefined() + expect(source.get(`r2`)).toBeUndefined() + + persistence.resolve() + await transaction.isPersisted.promise + } + + const result = await ready expect(loadCalls).toBe(1) try { expect(result.map(({ id }) => id).sort()).toEqual([`r1`, `r2`]) @@ -1090,6 +1129,550 @@ async function expectPersistingLoadIsApplied(persisting: boolean) { } } +async function expectAppliedReceiptTiming( + gate: `free` | `parked`, + delivery: `synchronous` | `asynchronous`, +): Promise { + const source = createCollection({ + id: `load-subset-applied-timing-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: ({ begin, write, commit, markReady }) => { + markReady() + return { + loadSubset: () => { + const applyRow = () => { + begin() + write({ + type: `insert`, + value: { id: `remote`, projectId: `p1` }, + }) + return commit() + } + + return delivery === `synchronous` + ? applyRow() + : Promise.resolve().then(async () => { + const applied = applyRow() + if (applied !== true) await applied + }) + }, + } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + if (gate === `parked`) { + transaction.mutate(() => source.insert({ id: `local`, projectId: `p2` })) + expect(transaction.state).toBe(`persisting`) + } + + const receipt = source._sync.loadSubset({}) + + try { + if (gate === `free` && delivery === `synchronous`) { + expect(receipt).toBe(true) + expect(source.get(`remote`)).toEqual( + expect.objectContaining({ id: `remote`, projectId: `p1` }), + ) + return + } + + const pending = requirePendingAppliedReceipt(receipt) + let settled = false + let visibleWhenSettled = false + void pending.then(() => { + settled = true + visibleWhenSettled = source.get(`remote`)?.id === `remote` + }) + + expect(settled).toBe(false) + expect(source.get(`remote`)).toBeUndefined() + await Promise.resolve() + await Promise.resolve() + + if (gate === `parked`) { + expect(settled).toBe(false) + expect(source.get(`remote`)).toBeUndefined() + persistence.resolve() + await transaction.isPersisted.promise + } + + await pending + expect(settled).toBe(true) + expect(visibleWhenSettled).toBe(true) + expect(source.get(`remote`)).toEqual( + expect.objectContaining({ id: `remote`, projectId: `p1` }), + ) + } finally { + persistence.resolve() + if (gate === `parked`) { + await transaction.isPersisted.promise.catch(() => undefined) + } + await source.cleanup() + } +} + +async function expectAppliedLoadDoesNotFlushEarlierParkedSync() { + const rows: Array = [ + { id: `r1`, projectId: `p1` }, + { id: `r2`, projectId: `p1` }, + ] + let publishUnrelated!: () => void + const source = createCollection({ + id: `load-subset-applied-order-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: ({ begin, write, commit, markReady }) => { + publishUnrelated = () => { + begin() + write({ + type: `insert`, + value: { id: `unrelated`, projectId: `p2` }, + }) + commit() + } + markReady() + return { + loadSubset: () => { + begin() + for (const row of rows) { + write({ type: `insert`, value: { ...row } }) + } + return commit() + }, + } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `other`, projectId: `p2` })) + expect(transaction.state).toBe(`persisting`) + publishUnrelated() + + const live = createLiveQueryCollection((query) => + query.from({ row: source }).where(({ row }) => eq(row.projectId, `p1`)), + ) + + try { + const ready = live.toArrayWhenReady() + let settled = false + void ready.then(() => { + settled = true + }) + await Promise.resolve() + await Promise.resolve() + + expect(settled).toBe(false) + expect(source.get(`unrelated`)).toBeUndefined() + + persistence.resolve() + await transaction.isPersisted.promise + await expect(ready).resolves.toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: `r1` }), + expect.objectContaining({ id: `r2` }), + ]), + ) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await live.cleanup() + await source.cleanup() + } +} + +async function expectCoverageWaitsForAppliedRows() { + let publishUnrelated!: () => void + let transportCalls = 0 + const source = createCollection({ + id: `load-subset-applied-coverage-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: ({ begin, write, commit, markReady }) => { + publishUnrelated = () => { + begin() + write({ + type: `insert`, + value: { id: `unrelated`, projectId: `p2` }, + }) + commit() + } + const deduplicated = new DeduplicatedLoadSubset({ + loadSubset: () => { + transportCalls += 1 + begin() + write({ + type: `insert`, + value: { id: `r1`, projectId: `p1` }, + }) + return commit() + }, + }) + markReady() + return { loadSubset: deduplicated.loadSubset } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `other`, projectId: `p2` })) + publishUnrelated() + + try { + const first = source._sync.loadSubset({}) + expect(first).toBeInstanceOf(Promise) + await Promise.resolve() + await Promise.resolve() + + const concurrent = source._sync.loadSubset({}) + expect(concurrent).toBe(first) + expect(transportCalls).toBe(1) + expect(source.get(`r1`)).toBeUndefined() + + persistence.resolve() + await transaction.isPersisted.promise + await Promise.all([first, concurrent]) + expect(source.get(`r1`)).toEqual( + expect.objectContaining({ id: `r1`, projectId: `p1` }), + ) + expect(source._sync.loadSubset({})).toBe(true) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await source.cleanup() + } +} + +async function expectConcurrentStreamCommitStaysParked() { + let publishUnrelated!: () => void + let publishSubset!: () => void + const source = createCollection({ + id: `load-subset-applied-concurrent-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: ({ begin, write, commit, markReady }) => { + publishUnrelated = () => { + begin() + write({ + type: `insert`, + value: { id: `unrelated`, projectId: `p2` }, + }) + commit() + } + markReady() + return { + loadSubset: () => + new Promise((resolve) => { + publishSubset = () => { + begin() + write({ + type: `insert`, + value: { id: `r1`, projectId: `p1` }, + }) + const applied = commit() + if (applied === true) { + resolve() + } else { + void applied.then(resolve) + } + } + }), + } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `other`, projectId: `p2` })) + + const load = requirePendingAppliedReceipt(source._sync.loadSubset({})) + publishUnrelated() + publishSubset() + + try { + let settled = false + void load.then(() => { + settled = true + }) + await Promise.resolve() + await Promise.resolve() + + expect(settled).toBe(false) + expect(source.get(`unrelated`)).toBeUndefined() + expect(source.get(`r1`)).toBeUndefined() + persistence.resolve() + await transaction.isPersisted.promise + await load + expect(source.get(`unrelated`)).toEqual( + expect.objectContaining({ id: `unrelated`, projectId: `p2` }), + ) + expect(source.get(`r1`)).toEqual( + expect.objectContaining({ id: `r1`, projectId: `p1` }), + ) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await source.cleanup() + } +} + +async function expectLaterImmediateCommitSettlesAppliedSubset() { + let publishLater!: () => SyncAppliedReceipt + const source = createCollection({ + id: `load-subset-applied-priority-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: ({ begin, write, commit, markReady }) => { + begin() + write({ + type: `insert`, + value: { id: `initial`, projectId: `p0` }, + }) + void commit() + publishLater = () => { + begin({ immediate: true }) + write({ + type: `insert`, + value: { id: `later`, projectId: `p2` }, + }) + return commit() + } + markReady() + return { + loadSubset: () => { + begin() + write({ + type: `insert`, + value: { id: `subset`, projectId: `p1` }, + }) + return commit() + }, + } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `local`, projectId: `p3` })) + + const load = requirePendingAppliedReceipt(source._sync.loadSubset({})) + const later = publishLater() + + try { + let loadSettled = false + let subsetVisibleWhenSettled = false + void load.then(() => { + loadSettled = true + subsetVisibleWhenSettled = source.get(`subset`)?.id === `subset` + }) + await later + await load + + expect(loadSettled).toBe(true) + expect(subsetVisibleWhenSettled).toBe(true) + expect(source.get(`subset`)).toEqual( + expect.objectContaining({ id: `subset` }), + ) + expect(source.get(`later`)).toEqual( + expect.objectContaining({ id: `later` }), + ) + expect(source.get(`initial`)).toEqual( + expect.objectContaining({ id: `initial` }), + ) + + persistence.resolve() + await transaction.isPersisted.promise + await Promise.all([load, later]) + + expect(source.get(`subset`)).toEqual( + expect.objectContaining({ id: `subset` }), + ) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await source.cleanup() + } +} + +async function expectAbortedReceiptDoesNotPublishCoverage( + abortPhase: `before-commit` | `after-commit`, +) { + let transportCalls = 0 + const committed = createDeferred() + const deduplicated = new DeduplicatedLoadSubset({ + loadSubset: async ({ signal }) => { + transportCalls += 1 + if (abortPhase === `before-commit`) { + // Give cancellation a chance to revoke this request before its + // request-scoped rows enter the collection transaction. + await Promise.resolve() + if (signal?.aborted) { + return + } + } + begin() + write({ + type: `insert`, + value: { id: `row`, projectId: `p1` }, + }) + const applied = commit(signal) + committed.resolve() + if (applied !== true) await applied + }, + }) + let begin!: () => void + let write!: (message: { type: `insert`; value: PersistedLoadRow }) => void + let commit!: (signal?: AbortSignal) => SyncAppliedReceipt + const source = createCollection({ + id: `load-subset-applied-abort-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: (params) => { + begin = params.begin + write = params.write + commit = params.commit + params.markReady() + return { loadSubset: deduplicated.loadSubset } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `local`, projectId: `p2` })) + const controller = new AbortController() + const first = requirePendingAppliedReceipt( + source._sync.loadSubset({ signal: controller.signal }), + ) + if (abortPhase === `after-commit`) { + await committed.promise + } + controller.abort() + + try { + persistence.resolve() + await transaction.isPersisted.promise + await first + expect(transportCalls).toBe(1) + expect(source.get(`row`)).toBeUndefined() + + const retry = source._sync.loadSubset({}) + if (retry !== true) await retry + expect(transportCalls).toBe(2) + expect(source.get(`row`)).toEqual(expect.objectContaining({ id: `row` })) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await source.cleanup() + } +} + +async function expectCleanupSettlesReceiptOnce() { + let receipt!: Promise + let transportCalls = 0 + const deduplicated = new DeduplicatedLoadSubset({ + loadSubset: () => { + transportCalls += 1 + begin() + write({ + type: `insert`, + value: { id: `row`, projectId: `p1` }, + }) + const applied = commit() + if (transportCalls === 1) { + if (applied === true) { + throw new Error(`Expected the subset transaction to remain parked`) + } + receipt = applied + } + return applied + }, + }) + let begin!: () => void + let write!: (message: { type: `insert`; value: PersistedLoadRow }) => void + let commit!: () => SyncAppliedReceipt + const source = createCollection({ + id: `load-subset-applied-cleanup-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: (params) => { + begin = params.begin + write = params.write + commit = params.commit + params.markReady() + return { + loadSubset: deduplicated.loadSubset, + cleanup: () => deduplicated.reset(), + } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `local`, projectId: `p2` })) + const load = requirePendingAppliedReceipt(source._sync.loadSubset({})) + let settlements = 0 + void receipt.then(() => { + settlements += 1 + }) + + await source.cleanup() + await Promise.all([load, receipt]) + expect(settlements).toBe(1) + + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await Promise.resolve() + expect(settlements).toBe(1) + + // Restarting installs fresh sync controls. Reacquisition must both perform + // transport work and publish its rows; stale callbacks cannot prove either. + source.startSyncImmediate() + const retry = source._sync.loadSubset({}) + if (retry !== true) await retry + expect(transportCalls).toBe(2) + expect(source.get(`row`)).toEqual(expect.objectContaining({ id: `row` })) + + await source.cleanup() +} + async function expectDerivedSyncDuringOptimisticMutation(): Promise { let begin!: () => void let write!: (message: { type: `insert`; value: OptimisticDerivedRow }) => void @@ -1965,14 +2548,50 @@ describe(`loadSubset coverage oracle`, () => { }) it(`applies loaded rows before resolving readiness behind a persisting mutation`, async () => { - await expectAssertionFailure(expectPersistingLoadIsApplied, { - checkpoint: 0, - classify: ({ actual, expected }) => - Array.isArray(actual) && - actual.length === 0 && - Array.isArray(expected) && - expected.join(`,`) === `r1,r2`, - })(true) + await expectPersistingLoadIsApplied(true) + }) + + it(`applies asynchronously delivered rows before resolving readiness`, async () => { + await expectPersistingLoadIsApplied(true, `asynchronous`) + }) + + it(`applies a transaction opened before its subset demand`, async () => { + await expectPersistingLoadIsApplied(true, `synchronous`, `before-load`) + }) + + it.each([ + [`free`, `synchronous`], + [`free`, `asynchronous`], + [`parked`, `synchronous`], + [`parked`, `asynchronous`], + ] as const)( + `preserves applied-receipt timing with a %s gate and %s delivery`, + expectAppliedReceiptTiming, + ) + + it(`does not flush earlier parked sync work to apply a subset load`, async () => { + await expectAppliedLoadDoesNotFlushEarlierParkedSync() + }) + + it(`publishes coverage only after its establishing rows apply`, async () => { + await expectCoverageWaitsForAppliedRows() + }) + + it(`keeps an unrelated stream commit parked during a subset acquisition`, async () => { + await expectConcurrentStreamCommitStaysParked() + }) + + it(`settles a subset receipt after a later immediate commit applies it`, async () => { + await expectLaterImmediateCommitSettlesAppliedSubset() + }) + + it.each([`before-commit`, `after-commit`] as const)( + `does not publish coverage when a parked receipt is aborted %s`, + expectAbortedReceiptDoesNotPublishCoverage, + ) + + it(`settles an abandoned receipt once without publishing coverage`, async () => { + await expectCleanupSettlesReceiptOnce() }) it(`publishes synced source rows while a derived mutation persists`, async () => { diff --git a/packages/electric-db-collection/src/electric.ts b/packages/electric-db-collection/src/electric.ts index 7b38d745d8..a148b26da6 100644 --- a/packages/electric-db-collection/src/electric.ts +++ b/packages/electric-db-collection/src/electric.ts @@ -47,6 +47,7 @@ import type { DeleteMutationFnParams, InsertMutationFnParams, LoadSubsetOptions, + SyncAppliedReceipt, SyncConfig, SyncMode, UpdateMutationFnParams, @@ -526,6 +527,8 @@ function createLoadSubsetDedupe>({ begin, write, commit, + getCommitCursor, + waitForCommitsAfter, collectionId, encodeColumnName, signal, @@ -539,7 +542,9 @@ function createLoadSubsetDedupe>({ value: T metadata: Record }) => void - commit: () => void + commit: () => SyncAppliedReceipt + getCommitCursor: () => number + waitForCommitsAfter: (cursor: number) => Promise collectionId?: string /** * Optional function to encode column names (e.g., camelCase to snake_case). @@ -573,6 +578,7 @@ function createLoadSubsetDedupe>({ } const loadSubset = async (opts: LoadSubsetOptions) => { + const commitCursor = getCommitCursor() if (opts.signal?.aborted) return if (isBufferingInitialSync()) { @@ -593,7 +599,7 @@ function createLoadSubsetDedupe>({ metadata: { ...row.headers }, }) } - commit() + await commit() debug(`${logPrefix}Applied snapshot with ${rows.length} rows`) } } catch (error) { @@ -691,6 +697,7 @@ function createLoadSubsetDedupe>({ } throw error } + await waitForCommitsAfter(commitCursor) } return new DeduplicatedLoadSubset({ loadSubset }) @@ -1487,13 +1494,32 @@ function createElectricSync>( const { begin, write, - commit, + commit: commitSyncTransaction, markReady, markError, truncate, collection, metadata, } = params + let commitSequence = 0 + const pendingAppliedReceipts = new Map>() + const commit = (): SyncAppliedReceipt => { + const sequence = ++commitSequence + const applied = commitSyncTransaction() + if (applied === true) { + return true + } + pendingAppliedReceipts.set(sequence, applied) + void applied.then(() => pendingAppliedReceipts.delete(sequence)) + return applied + } + const waitForCommitsAfter = async (cursor: number): Promise => { + await Promise.all( + Array.from(pendingAppliedReceipts, ([sequence, applied]) => + sequence > cursor ? applied : undefined, + ), + ) + } const readPersistedResumeState = (): ElectricResumeState | undefined => { const persistedResumeState = metadata?.collection.get(`electric:resume`) return parseElectricResumeState(persistedResumeState) @@ -1730,6 +1756,8 @@ function createElectricSync>( begin, write, commit, + getCommitCursor: () => commitSequence, + waitForCommitsAfter, collectionId, // Pass the columnMapper's encode function to transform column names // (e.g., camelCase to snake_case) when compiling SQL for subset queries @@ -1892,6 +1920,8 @@ function createElectricSync>( } if (commitPoint !== null) { + let applied: SyncAppliedReceipt = true + const wasBufferingInitialSync = isBufferingInitialSync() // PROGRESSIVE MODE: Atomic swap on first up-to-date (not subset-end) // EXCEPTION: Skip atomic swap if a transaction is already started (e.g., from must-refetch). // In that case, do a normal commit to properly close the existing transaction. @@ -1946,7 +1976,7 @@ function createElectricSync>( // Commit the atomic swap stageResumeMetadata() - commit() + applied = commit() // Exit buffering phase by marking that we've received up-to-date // isBufferingInitialSync() will now return false @@ -1960,15 +1990,19 @@ function createElectricSync>( // Both up-to-date and subset-end trigger a commit if (transactionStarted) { stageResumeMetadata() - commit() + applied = commit() transactionStarted = false } else if (commitPoint === `up-to-date` && metadata) { begin() stageResumeMetadata() - commit() + applied = commit() } } - wrappedMarkReady(isBufferingInitialSync()) + if (applied === true) { + wrappedMarkReady(wasBufferingInitialSync) + } else { + void applied.then(() => wrappedMarkReady(wasBufferingInitialSync)) + } // Track that we've received the first up-to-date for progressive mode if (commitPoint === `up-to-date`) { diff --git a/packages/electric-db-collection/tests/electric.test.ts b/packages/electric-db-collection/tests/electric.test.ts index 06faf86a79..bc156fd159 100644 --- a/packages/electric-db-collection/tests/electric.test.ts +++ b/packages/electric-db-collection/tests/electric.test.ts @@ -23,6 +23,14 @@ import type { StandardSchemaV1 } from '@standard-schema/spec' const NativeAbortController = globalThis.AbortController +function createDeferred() { + let resolve!: (value: T | PromiseLike) => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + // Mock the ShapeStream module const mockSubscribe = vi.fn() const mockRequestSnapshot = vi.fn() @@ -233,6 +241,38 @@ describe(`Electric Integration`, () => { ) }) + it(`marks the source ready only after its initial rows are applied`, async () => { + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => + collection.insert({ id: 99, name: `Optimistic user` }), + ) + + subscriber([ + { + key: `1`, + value: { id: 1, name: `Synced user` }, + headers: { operation: `insert` }, + }, + { headers: { control: `up-to-date` } }, + ]) + await Promise.resolve() + + expect(collection.status).toBe(`loading`) + expect(collection.get(1)).toBeUndefined() + + persistence.resolve() + await transaction.isPersisted.promise + await collection.stateWhenReady() + + expect(collection.status).toBe(`ready`) + expect(collection.get(1)).toEqual( + expect.objectContaining({ id: 1, name: `Synced user` }), + ) + }) + it(`should handle multiple changes before committing`, () => { // First batch of changes subscriber([ diff --git a/packages/powersync-db-collection/src/powersync.ts b/packages/powersync-db-collection/src/powersync.ts index b52374012c..76b8dedd3e 100644 --- a/packages/powersync-db-collection/src/powersync.ts +++ b/packages/powersync-db-collection/src/powersync.ts @@ -11,6 +11,7 @@ import type { CleanupFn, LoadSubsetOptions, OperationType, + SyncAppliedReceipt, SyncConfig, } from '@tanstack/db' import type { @@ -345,18 +346,21 @@ function createPowerSyncCollectionConfig< await dispose(context ? { context } : undefined) } - async function createDiffTrigger(options: { - setupContext?: LockContext - when: Record - writeType: (rowId: string) => OperationType - batchQuery: ( - lockContext: LockContext, - batchSize: number, - cursor: number, - ) => Promise> - onReady: () => void - }) { - const { setupContext, when, writeType, batchQuery, onReady } = options + async function createDiffTrigger( + options: { + setupContext?: LockContext + immediate?: boolean + when: Record + writeType: (rowId: string) => OperationType + batchQuery: ( + lockContext: LockContext, + batchSize: number, + cursor: number, + ) => Promise> + }, + appliedReceipts: Array, + ) { + const { setupContext, immediate, when, writeType, batchQuery } = options return await database.triggers.createDiffTrigger({ source: viewName, @@ -368,7 +372,7 @@ function createPowerSyncCollectionConfig< let currentBatchCount = syncBatchSize let cursor = 0 while (currentBatchCount == syncBatchSize) { - begin() + begin(immediate ? { immediate: true } : undefined) const batchItems = await batchQuery( context, @@ -383,9 +387,8 @@ function createPowerSyncCollectionConfig< value: deserializeSyncRow(row), }) } - commit() + appliedReceipts.push(commit()) } - onReady() database.logger.info( `Sync is ready for ${viewName} into ${trackedTableName}`, ) @@ -395,9 +398,10 @@ function createPowerSyncCollectionConfig< } async function flushDiffRecords(): Promise { + const ignoredReceipts: Array = [] await database .writeTransaction(async (context) => { - await flushDiffRecordsWithContext(context) + await flushDiffRecordsWithContext(context, ignoredReceipts) }) .catch((error) => { database.logger.error( @@ -410,6 +414,7 @@ function createPowerSyncCollectionConfig< // We can use this directly if we want to pair a flush with dispose+recreate diff trigger. async function flushDiffRecordsWithContext( context: LockContext, + appliedReceipts: Array, ): Promise { // There is nothing to flush if no tracking table is currently active. if (!disposeTracking) { @@ -452,7 +457,12 @@ function createPowerSyncCollectionConfig< // clear the current operations await context.execute(`DELETE FROM ${trackedTableName}`) - commit() + const applied = commit() + appliedReceipts.push(applied) + // Mutation persistence is what releases the Collection's FIFO gate. + // Confirm these local operations after the sync transaction is + // staged; waiting for its applied receipt would deadlock the user + // transaction that currently parks it. pendingOperationStore.resolvePendingFor(pendingOperations) } catch (error) { database.logger.error( @@ -504,24 +514,32 @@ function createPowerSyncCollectionConfig< start(async () => { onUnload = await restConfig.onLoad?.() - disposeTracking = await createDiffTrigger({ - when: { - [DiffTriggerOperation.INSERT]: `TRUE`, - [DiffTriggerOperation.UPDATE]: `TRUE`, - [DiffTriggerOperation.DELETE]: `TRUE`, + const appliedReceipts: Array = [] + disposeTracking = await createDiffTrigger( + { + // Initial eager hydration must make the source usable before + // PowerSync can persist a mutation queued during startup. + immediate: true, + when: { + [DiffTriggerOperation.INSERT]: `TRUE`, + [DiffTriggerOperation.UPDATE]: `TRUE`, + [DiffTriggerOperation.DELETE]: `TRUE`, + }, + writeType: (_rowId: string) => `insert`, + batchQuery: ( + lockContext: LockContext, + batchSize: number, + cursor: number, + ) => + lockContext.getAll( + sanitizeSQL`SELECT * FROM ${viewName} LIMIT ? OFFSET ?`, + [batchSize, cursor], + ), }, - writeType: (_rowId: string) => `insert`, - batchQuery: ( - lockContext: LockContext, - batchSize: number, - cursor: number, - ) => - lockContext.getAll( - sanitizeSQL`SELECT * FROM ${viewName} LIMIT ? OFFSET ?`, - [batchSize, cursor], - ), - onReady: () => markReady(), - }) + appliedReceipts, + ) + await Promise.all(appliedReceipts) + markReady() }).catch((error) => { database.logger.error( `Could not start syncing process for ${viewName} into ${trackedTableName}`, @@ -564,6 +582,7 @@ function createPowerSyncCollectionConfig< options?: LoadSubsetOptions, ): Promise => { if (hasStopped()) return + const appliedReceipts: Array = [] if (options) { activeWhereExpressions.push(options.where) @@ -585,9 +604,10 @@ function createPowerSyncCollectionConfig< // when no tracking table is currently active. if (activeWhereExpressions.length === 0) { await database.writeLock(async (ctx) => { - await flushDiffRecordsWithContext(ctx) + await flushDiffRecordsWithContext(ctx, appliedReceipts) await safelyDisposeTracking(ctx) }) + await Promise.all(appliedReceipts) return } @@ -619,30 +639,33 @@ function createPowerSyncCollectionConfig< await database.writeLock(async (ctx) => { // Replace any active tracking with one covering the new set of // predicates. - await flushDiffRecordsWithContext(ctx) + await flushDiffRecordsWithContext(ctx, appliedReceipts) await safelyDisposeTracking(ctx) - disposeTracking = await createDiffTrigger({ - setupContext: ctx, - when: { - [DiffTriggerOperation.INSERT]: newDataWhenClause, - [DiffTriggerOperation.UPDATE]: `(${newDataWhenClause}) OR (${oldDataWhenClause})`, - [DiffTriggerOperation.DELETE]: oldDataWhenClause, + disposeTracking = await createDiffTrigger( + { + setupContext: ctx, + when: { + [DiffTriggerOperation.INSERT]: newDataWhenClause, + [DiffTriggerOperation.UPDATE]: `(${newDataWhenClause}) OR (${oldDataWhenClause})`, + [DiffTriggerOperation.DELETE]: oldDataWhenClause, + }, + writeType: (rowId: string) => + collection.has(rowId) ? `update` : `insert`, + batchQuery: ( + lockContext: LockContext, + batchSize: number, + cursor: number, + ) => + lockContext.getAll( + `SELECT * FROM ${viewName} WHERE ${viewWhereClause} LIMIT ? OFFSET ?`, + [batchSize, cursor], + ), }, - writeType: (rowId: string) => - collection.has(rowId) ? `update` : `insert`, - batchQuery: ( - lockContext: LockContext, - batchSize: number, - cursor: number, - ) => - lockContext.getAll( - `SELECT * FROM ${viewName} WHERE ${viewWhereClause} LIMIT ? OFFSET ?`, - [batchSize, cursor], - ), - onReady: () => {}, - }) + appliedReceipts, + ) }) + await Promise.all(appliedReceipts) } const toInlinedWhereClause = (compiled: { @@ -698,7 +721,11 @@ function createPowerSyncCollectionConfig< for (const { id } of rowsToEvict) { write({ type: `delete`, key: id }) } - commit() + // Eviction does not establish new subset coverage. Keep trigger + // replacement in the same unload turn even when this delete waits + // behind a persisting mutation; the later load tracks its own + // establishing receipts. + void commit() } // Recreate the diff trigger for the remaining active WHERE expressions. diff --git a/packages/powersync-db-collection/tests/on-demand-sync.test.ts b/packages/powersync-db-collection/tests/on-demand-sync.test.ts index dffcc8505f..8d1dc34122 100644 --- a/packages/powersync-db-collection/tests/on-demand-sync.test.ts +++ b/packages/powersync-db-collection/tests/on-demand-sync.test.ts @@ -5,6 +5,7 @@ import { and, createCollection, createLiveQueryCollection, + createTransaction, eq, gt, gte, @@ -140,6 +141,82 @@ describe(`On-Demand Sync Mode`, () => { expect(prices).toEqual([150, 200]) }) + it(`resolves subset readiness only after its rows are applied`, async () => { + const db = await createDatabase() + await createTestProducts(db) + + let resolvePersistence!: () => void + const persistence = new Promise((resolve) => { + resolvePersistence = resolve + }) + const transaction = createTransaction({ + mutationFn: () => persistence, + }) + const options = powerSyncCollectionOptions({ + database: db, + table: APP_SCHEMA.props.products, + syncMode: `on-demand`, + onLoadSubset: () => { + transaction.mutate(() => + collection.insert({ + id: `local`, + name: `Local product`, + price: 1, + category: `local`, + }), + ) + }, + }) + const collection = createCollection(options) + onTestFinished(() => collection.cleanup()) + await collection.stateWhenReady() + + const electronics = createLiveQueryCollection({ + query: (q) => + q + .from({ product: collection }) + .where(({ product }) => eq(product.category, `electronics`)), + }) + onTestFinished(() => electronics.cleanup()) + const preload = electronics.preload() + let settled = false + void preload.then(() => { + settled = true + }) + + try { + const { trackedTableName } = options.utils.getMeta() + await vi.waitFor( + async () => { + const table = await db.writeLock((context) => + context.get<{ count: number }>( + `SELECT COUNT(*) as count FROM sqlite_temp_master WHERE type = 'table' AND name = ?`, + [trackedTableName], + ), + ) + expect(table.count).toBe(1) + }, + { timeout: 2_000 }, + ) + + expect(transaction.state).toBe(`persisting`) + expect(settled).toBe(false) + expect(electronics.size).toBe(0) + + resolvePersistence() + await transaction.isPersisted.promise + await preload + + expect(electronics.toArray.map((product) => product.name).sort()).toEqual( + [`Product A`, `Product B`, `Product D`], + ) + } finally { + resolvePersistence() + await transaction.isPersisted.promise.catch(() => undefined) + await Promise.allSettled([preload]) + } + }) + it(`should reactively update live query when new matching data is inserted into SQLite`, async () => { const db = await createDatabase() await createTestProducts(db) diff --git a/packages/query-db-collection/src/manual-sync.ts b/packages/query-db-collection/src/manual-sync.ts index addf6808b2..ab61dd5eb6 100644 --- a/packages/query-db-collection/src/manual-sync.ts +++ b/packages/query-db-collection/src/manual-sync.ts @@ -5,7 +5,11 @@ import { UpdateOperationItemNotFoundError, } from './errors' import type { QueryClient } from '@tanstack/query-core' -import type { ChangeMessage, Collection } from '@tanstack/db' +import type { + ChangeMessage, + Collection, + SyncAppliedReceipt, +} from '@tanstack/db' // Track active batch operations per context to prevent cross-collection contamination const activeBatchContexts = new WeakMap< @@ -42,7 +46,7 @@ export interface SyncContext< */ begin: (options?: { immediate?: boolean }) => void write: (message: Omit, `key`>) => void - commit: () => void + commit: () => SyncAppliedReceipt /** * Optional function to update the query cache with the latest synced data. * Handles both direct array caches and wrapped response formats (when `select` is used). diff --git a/packages/query-db-collection/src/query.ts b/packages/query-db-collection/src/query.ts index 5396bd49d6..f27a247306 100644 --- a/packages/query-db-collection/src/query.ts +++ b/packages/query-db-collection/src/query.ts @@ -16,6 +16,7 @@ import type { DeleteMutationFnParams, InsertMutationFnParams, LoadSubsetOptions, + SyncAppliedReceipt, SyncConfig, SyncMetadataApi, UpdateMutationFnParams, @@ -1321,19 +1322,9 @@ export function queryCollectionOptions( // Error already occurred, reject immediately return Promise.reject(currentResult.error) } else { - // Check QueryClient cache directly - observer's getCurrentResult() may show - // a loading state even when data exists in cache. This happens because observer - // state can lag behind the QueryClient cache during unsubscribe/resubscribe - // cycles (e.g., when a live query is cleaned up and recreated). - const cachedData = queryClient.getQueryData(key) - if (cachedData !== undefined) { - return waitForQueryReady(observer, hashedQueryKey).then(() => - pendingResultApplications.get(hashedQueryKey), - ) - } - - // Query is still loading, wait for the first result - return waitForQueryReady(observer, hashedQueryKey) + return waitForQueryReady(observer, hashedQueryKey).then(() => + pendingResultApplications.get(hashedQueryKey), + ) } } @@ -1412,12 +1403,14 @@ export function queryCollectionOptions( subscribeToQuery(localObserver, hashedQueryKey) } - return readyPromise + return readyPromise.then(() => + pendingResultApplications.get(hashedQueryKey), + ) } type UpdateHandler = Parameters[0] - const applySuccessfulResult = ( + const applySuccessfulResult = async ( queryKey: QueryKey, result: QueryObserverResult, persistedBaseline?: Map< @@ -1427,17 +1420,13 @@ export function queryCollectionOptions( owners: Set } >, - ) => { + ): Promise => { const hashedQueryKey = hashKey(queryKey) if (collection.status === `cleaned-up`) { return } - // Clear error state - state.lastError = undefined - state.errorCount = 0 - const rawData = result.data const newItemsArray = select ? select(rawData) : rawData @@ -1460,12 +1449,6 @@ export function queryCollectionOptions( const previouslyOwnedRows = shouldUsePersistedBaseline ? new Set(persistedBaseline.keys()) : getHydratedOwnedRowsForQueryBaseline(hashedQueryKey) - // From this point onward the result, including an empty result, is the - // authoritative ownership baseline until this query is cleaned up. - queryToRows.set( - hashedQueryKey, - queryToRows.get(hashedQueryKey) ?? new Set(), - ) const newItemsMap = new Map() newItemsArray.forEach((item) => { @@ -1473,52 +1456,108 @@ export function queryCollectionOptions( newItemsMap.set(key, item) }) - begin() - if (metadata) { - metadata.collection.delete( - `${QUERY_COLLECTION_GC_PREFIX}${hashedQueryKey}`, + const previousOwnedRows = queryToRows.has(hashedQueryKey) + ? new Set(queryToRows.get(hashedQueryKey)) + : undefined + const affectedRowKeys = new Set([ + ...previouslyOwnedRows, + ...newItemsMap.keys(), + ]) + const previousOwnersByRow = new Map< + string | number, + Set | undefined + >() + affectedRowKeys.forEach((key) => { + const owners = rowToQueries.get(key) + previousOwnersByRow.set(key, owners ? new Set(owners) : undefined) + }) + let transactionActive = false + + try { + // From this point onward the result, including an empty result, is the + // authoritative ownership baseline until this query is cleaned up. + queryToRows.set( + hashedQueryKey, + queryToRows.get(hashedQueryKey) ?? new Set(), ) - } - previouslyOwnedRows.forEach((key) => { - const oldItem = shouldUsePersistedBaseline - ? persistedBaseline.get(key)?.value - : currentSyncedItems.get(key) - if (!oldItem) { - return + begin() + transactionActive = true + if (metadata) { + metadata.collection.delete( + `${QUERY_COLLECTION_GC_PREFIX}${hashedQueryKey}`, + ) } - const newItem = newItemsMap.get(key) - if (!newItem) { + + previouslyOwnedRows.forEach((key) => { + const oldItem = shouldUsePersistedBaseline + ? persistedBaseline.get(key)?.value + : currentSyncedItems.get(key) + if (!oldItem) { + return + } + const newItem = newItemsMap.get(key) + if (!newItem) { + const owners = getPersistedOwners(key) + owners.delete(hashedQueryKey) + setPersistedOwners(key, owners) + const needToRemove = removeRowOwner(key, hashedQueryKey) + if (needToRemove) { + write({ type: `delete`, value: oldItem }) + } + } else if (!deepEquals(oldItem, newItem)) { + write({ type: `update`, value: newItem }) + } + }) + + newItemsMap.forEach((newItem, key) => { const owners = getPersistedOwners(key) - owners.delete(hashedQueryKey) - setPersistedOwners(key, owners) - const needToRemove = removeRowOwner(key, hashedQueryKey) - if (needToRemove) { - write({ type: `delete`, value: oldItem }) + if (!owners.has(hashedQueryKey)) { + owners.add(hashedQueryKey) + setPersistedOwners(key, owners) } - } else if (!deepEquals(oldItem, newItem)) { - write({ type: `update`, value: newItem }) - } - }) + addRowOwner(key, hashedQueryKey) + if (!currentSyncedItems.has(key)) { + write({ type: `insert`, value: newItem }) + } + }) - newItemsMap.forEach((newItem, key) => { - const owners = getPersistedOwners(key) - if (!owners.has(hashedQueryKey)) { - owners.add(hashedQueryKey) - setPersistedOwners(key, owners) + const applied = commit() + transactionActive = false + retainedQueriesPendingRevalidation.delete(hashedQueryKey) + cancelPersistedRetentionExpiry(hashedQueryKey) + + // Readiness is publication: do not expose it until the establishing + // transaction's rows and events are visible. + if (applied !== true) { + await applied } - addRowOwner(key, hashedQueryKey) - if (!currentSyncedItems.has(key)) { - write({ type: `insert`, value: newItem }) + markReady() + } catch (error) { + if (previousOwnedRows === undefined) { + queryToRows.delete(hashedQueryKey) + } else { + queryToRows.set(hashedQueryKey, previousOwnedRows) } - }) - - commit() - retainedQueriesPendingRevalidation.delete(hashedQueryKey) - cancelPersistedRetentionExpiry(hashedQueryKey) + previousOwnersByRow.forEach((owners, key) => { + if (owners === undefined) { + rowToQueries.delete(key) + } else { + rowToQueries.set(key, owners) + } + }) - // Mark collection as ready after first successful query result - markReady() + if (transactionActive) { + const cancellation = new AbortController() + cancellation.abort() + try { + commit(cancellation.signal) + } catch { + // Preserve the application error that caused the rollback. + } + } + throw error + } } const reconcileSuccessfulResult = async ( @@ -1535,7 +1574,43 @@ export function queryCollectionOptions( ) { return } - applySuccessfulResult(queryKey, result, persistedBaseline) + await applySuccessfulResult(queryKey, result, persistedBaseline) + } + + const trackResultApplication = ( + hashedQueryKey: string, + application: Promise, + ): void => { + pendingResultApplications.set(hashedQueryKey, application) + const finish = () => { + if (pendingResultApplications.get(hashedQueryKey) === application) { + pendingResultApplications.delete(hashedQueryKey) + } + } + void application.then(finish, (error) => { + finish() + state.lastError = error + state.errorCount++ + state.lastErrorUpdatedAt = Date.now() + console.error( + `[QueryCollection] Error applying query ${String(hashToQueryKey.get(hashedQueryKey))}:`, + error, + ) + if (collection.status === `loading`) { + markError(error) + } + }) + } + + const enqueueResultApplication = ( + hashedQueryKey: string, + apply: () => Promise, + ): void => { + const previousApplication = pendingResultApplications.get(hashedQueryKey) + const application = previousApplication + ? previousApplication.then(apply, apply) + : apply() + trackResultApplication(hashedQueryKey, application) } // eslint-disable-next-line no-shadow @@ -1543,6 +1618,11 @@ export function queryCollectionOptions( const hashedQueryKey = hashKey(queryKey) const handleQueryResult: UpdateHandler = (result) => { if (result.isSuccess) { + // Error state follows observer notification order, not the later + // publication time of a queued successful result. + state.lastError = undefined + state.errorCount = 0 + // Skip processing this result while data refreshes are deferred. // Optimistic state covers the gap. Once the barrier resolves, // trigger a fresh refetch to get authoritative data. @@ -1579,26 +1659,13 @@ export function queryCollectionOptions( const applicationToken = {} resultApplicationTokens.set(hashedQueryKey, applicationToken) - const application = reconcileSuccessfulResult( - queryKey, - result, - applicationToken, - ).catch((error) => { - console.error( - `[QueryCollection] Error reconciling query ${String(queryKey)}:`, - error, - ) - }) - pendingResultApplications.set(hashedQueryKey, application) - void application.finally(() => { - if ( - pendingResultApplications.get(hashedQueryKey) === application - ) { - pendingResultApplications.delete(hashedQueryKey) - } - }) + enqueueResultApplication(hashedQueryKey, () => + reconcileSuccessfulResult(queryKey, result, applicationToken), + ) } else { - applySuccessfulResult(queryKey, result) + enqueueResultApplication(hashedQueryKey, () => + applySuccessfulResult(queryKey, result), + ) } } else if (result.isError) { const isNewError = @@ -2085,7 +2152,7 @@ export function queryCollectionOptions( getKey: (item: any) => string | number begin: () => void write: (message: Omit, `key`>) => void - commit: () => void + commit: () => SyncAppliedReceipt updateCacheData?: (items: Array) => void } | null = null diff --git a/packages/query-db-collection/tests/query.test.ts b/packages/query-db-collection/tests/query.test.ts index f718170143..1d4ea34853 100644 --- a/packages/query-db-collection/tests/query.test.ts +++ b/packages/query-db-collection/tests/query.test.ts @@ -12,6 +12,7 @@ import { collectionOptions, createCollection, createLiveQueryCollection, + createTransaction, eq, ilike, inArray, @@ -672,6 +673,95 @@ describe(`QueryCollection`, () => { } }) + it(`keeps an eager result loading until its rows are applied`, async () => { + const queryResult = createDeferred>() + const queryFn = vi.fn(() => queryResult.promise) + const collection = createCollection( + queryCollectionOptions({ + id: `eager-applied-settlement`, + queryClient, + queryKey: [`eager-applied-settlement`], + queryFn, + getKey, + syncMode: `eager`, + startSync: true, + }), + ) + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => + collection.insert({ id: `local`, name: `Local` }), + ) + + try { + const ready = collection.stateWhenReady() + await vi.waitFor(() => expect(queryFn).toHaveBeenCalledOnce()) + queryResult.resolve([{ id: `server`, name: `Server` }]) + await flushPromises() + + expect(collection.status).toBe(`loading`) + expect(collection.get(`server`)).toBeUndefined() + + persistence.resolve() + await transaction.isPersisted.promise + await ready + + expect(collection.status).toBe(`ready`) + expect(collection.get(`server`)).toEqual( + expect.objectContaining({ id: `server`, name: `Server` }), + ) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await collection.cleanup() + } + }) + + it(`applies successive eager results in publication order`, async () => { + const queryKey = [`eager-result-publication-order`] + const collection = createCollection( + queryCollectionOptions({ + id: `eager-result-publication-order`, + queryClient, + queryKey, + queryFn: vi.fn().mockResolvedValue([]), + getKey, + syncMode: `eager`, + startSync: true, + }), + ) + + await collection.stateWhenReady() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => + collection.insert({ id: `local`, name: `Local` }), + ) + + try { + queryClient.setQueryData(queryKey, [{ id: `server`, name: `Server` }]) + await flushPromises() + queryClient.setQueryData(queryKey, []) + await flushPromises() + + persistence.resolve() + await transaction.isPersisted.promise + + await vi.waitFor(() => { + expect(collection.get(`server`)).toBeUndefined() + }) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await collection.cleanup() + } + }) + it(`does not materialize QueryClient placeholder defaults`, async () => { const placeholderQueryClient = new QueryClient({ defaultOptions: { @@ -4522,6 +4612,55 @@ describe(`QueryCollection`, () => { return createCollection(options) } + it.each([`select`, `getKey`, `write`] as const)( + `reports an error when %s throws while applying a successful result`, + async (failureStage) => { + const applicationError = new Error(`${failureStage} failed`) + const consoleErrorSpy = vi + .spyOn(console, `error`) + .mockImplementation(() => {}) + let keyCalls = 0 + const throwingGetKey = (item: TestItem) => { + keyCalls++ + if ( + failureStage === `getKey` || + (failureStage === `write` && keyCalls === 2) + ) { + throw applicationError + } + return item.id + } + + const options = queryCollectionOptions({ + id: `successful-result-${failureStage}-error-test`, + queryClient, + queryKey: [`successful-result-${failureStage}-error-test`], + queryFn: vi.fn().mockResolvedValue([{ id: `1`, name: `Item 1` }]), + getKey: throwingGetKey, + select: + failureStage === `select` + ? () => { + throw applicationError + } + : undefined, + startSync: true, + retry: false, + }) + const collection = createCollection(options) + + await expect(collection.preload()).rejects.toBe(applicationError) + expect(collection.status).toBe(`error`) + expect(collection.utils.lastError).toBe(applicationError) + expect(collection.utils.errorCount).toBe(1) + expect(collection.size).toBe(0) + expect(inspectOwnershipMaps(options).rowToQueries.size).toBe(0) + expect(inspectOwnershipMaps(options).queryToRows.size).toBe(0) + + await collection.cleanup() + consoleErrorSpy.mockRestore() + }, + ) + it(`should track error state, count, and support recovery`, async () => { const initialData = [{ id: `1`, name: `Item 1` }] const updatedData = [{ id: `1`, name: `Updated Item 1` }] diff --git a/packages/rxdb-db-collection/src/rxdb.ts b/packages/rxdb-db-collection/src/rxdb.ts index ef61631ace..4a0e44d551 100644 --- a/packages/rxdb-db-collection/src/rxdb.ts +++ b/packages/rxdb-db-collection/src/rxdb.ts @@ -129,7 +129,7 @@ export function rxdbCollectionOptions( sync: (params: SyncParams) => { const { begin, write, commit, markReady, markError, collection } = params - let ready = false + let initialFetchComplete = false async function initialFetch() { /** * RxDB stores a last-write-time @@ -140,7 +140,7 @@ export function rxdbCollectionOptions( const syncBatchSize = config.syncBatchSize ? config.syncBatchSize : 1000 begin() - while (!ready) { + while (!initialFetchComplete) { let query: FilledMangoQuery if (cursor) { query = { @@ -184,7 +184,7 @@ export function rxdbCollectionOptions( cursor = lastOfArray(docs) if (docs.length === 0) { - ready = true + initialFetchComplete = true break } @@ -195,13 +195,14 @@ export function rxdbCollectionOptions( }) }) } - commit() + await commit() } type WriteMessage = Parameters[0] const buffer: Array = [] + let buffering = true const queue = (msg: WriteMessage) => { - if (!ready) { + if (buffering) { buffer.push(msg) return } @@ -249,17 +250,30 @@ export function rxdbCollectionOptions( } async function start() { + const isCleanedUp = () => collection.status === `cleaned-up` + startOngoingFetch() await initialFetch() + if (isCleanedUp()) { + return + } - if (buffer.length) { + // Keep buffering until every older startup batch is applied. Events + // that arrive while one receipt is parked join the next FIFO batch. + while (buffer.length) { + const pending = buffer.splice(0) begin() - for (const msg of buffer) write(msg) - commit() - buffer.length = 0 + for (const msg of pending) write(msg) + await commit() + if (isCleanedUp()) { + return + } } + buffering = false - markReady() + if (!isCleanedUp()) { + markReady() + } } void start().catch((error: unknown) => { diff --git a/packages/rxdb-db-collection/tests/rxdb.test.ts b/packages/rxdb-db-collection/tests/rxdb.test.ts index dc6a4d2659..02c63aa1d3 100644 --- a/packages/rxdb-db-collection/tests/rxdb.test.ts +++ b/packages/rxdb-db-collection/tests/rxdb.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { createCollection } from '@tanstack/db' +import { createCollection, createTransaction } from '@tanstack/db' import { addRxPlugin, createRxDatabase, @@ -22,6 +22,14 @@ type RxCollections = { test: RxCollection } // Helper to advance timers and allow microtasks to flush const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0)) +function createDeferred() { + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + describe(`RxDB Integration`, () => { addRxPlugin(RxDBDevModePlugin) @@ -134,6 +142,78 @@ describe(`RxDB Integration`, () => { } }) + it(`marks initial sync ready only after its rows are applied`, async () => { + const db = await getDatababase([{ id: `server`, name: `Server` }]) + const rxCollection: RxCollection = db.test + const releaseInitialQuery = createDeferred() + const initialQueryStarted = createDeferred() + const storageQuery = rxCollection.storageInstance.query.bind( + rxCollection.storageInstance, + ) + const query = vi + .spyOn(rxCollection.storageInstance, `query`) + .mockImplementationOnce(async (preparedQuery) => { + const result = await storageQuery(preparedQuery) + initialQueryStarted.resolve() + await releaseInitialQuery.promise + return result + }) + const collection = createCollection( + rxdbCollectionOptions({ + rxCollection, + startSync: true, + syncBatchSize: 10, + }), + ) + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + + try { + await initialQueryStarted.promise + transaction.mutate(() => + collection.insert({ id: `local`, name: `Local` }), + ) + const buffered = await rxCollection.insert({ + id: `buffered`, + name: `Buffered`, + }) + releaseInitialQuery.resolve() + + const ready = collection.preload() + await flushPromises() + + // The initial receipt is still parked. A later live change for the + // same row must not overtake the older buffered insert. + await buffered.getLatest().patch({ name: `Newest` }) + await flushPromises() + + expect(collection.status).toBe(`loading`) + expect(collection.get(`server`)).toBeUndefined() + expect(collection.get(`buffered`)).toBeUndefined() + + persistence.resolve() + await transaction.isPersisted.promise + await ready + + expect(collection.get(`server`)).toEqual( + expect.objectContaining({ id: `server`, name: `Server` }), + ) + expect(collection.get(`buffered`)).toEqual( + expect.objectContaining({ id: `buffered`, name: `Newest` }), + ) + expect(collection.status).toBe(`ready`) + } finally { + releaseInitialQuery.resolve() + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + query.mockRestore() + await collection.cleanup() + await db.remove() + } + }) + it(`should initialize and fetch initial data`, async () => { const initialItems = getTestData(2) diff --git a/packages/trailbase-db-collection/src/trailbase.ts b/packages/trailbase-db-collection/src/trailbase.ts index 1fa1c4bf8a..d256815721 100644 --- a/packages/trailbase-db-collection/src/trailbase.ts +++ b/packages/trailbase-db-collection/src/trailbase.ts @@ -253,7 +253,7 @@ export function trailBaseCollectionOptions< }) } - commit() + await commit(opts.signal) remaining -= length @@ -302,7 +302,7 @@ export function trailBaseCollectionOptions< } else { console.error(`Error: ${event.Error}`) } - commit() + void commit() if (value) { seenIds.setState((curr: Map) => { diff --git a/packages/trailbase-db-collection/tests/trailbase.test.ts b/packages/trailbase-db-collection/tests/trailbase.test.ts index 93f4e81240..f784749c01 100644 --- a/packages/trailbase-db-collection/tests/trailbase.test.ts +++ b/packages/trailbase-db-collection/tests/trailbase.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { createCollection } from '@tanstack/db' +import { createCollection, createTransaction } from '@tanstack/db' import { trailBaseCollectionOptions } from '../src/trailbase' import { stripVirtualProps } from '../../db/tests/utils' import type { @@ -138,6 +138,63 @@ async function expectWildcardFailureSettlesPreload(): Promise { } describe(`TrailBase Integration`, () => { + it(`marks initial sync ready only after its rows are applied`, async () => { + const recordApi = new MockRecordApi() + let resolveList!: (response: ListResponse) => void + recordApi.list.mockReturnValue( + new Promise>((resolve) => { + resolveList = resolve + }), + ) + recordApi.subscribe.mockResolvedValue(new TransformStream().readable) + + let resolvePersistence!: () => void + const persistence = new Promise((resolve) => { + resolvePersistence = resolve + }) + const collection = createCollection(setUp(recordApi)) + const transaction = createTransaction({ + mutationFn: () => persistence, + }) + const preload = collection.preload() + + try { + await vi.waitFor(() => expect(recordApi.list).toHaveBeenCalledOnce()) + transaction.mutate(() => + collection.insert({ id: 2, updated: 0, data: `local` }), + ) + expect(transaction.state).toBe(`persisting`) + + resolveList({ + records: [{ id: 1, updated: 0, data: `server` }], + }) + await Promise.resolve() + await Promise.resolve() + + expect(collection.status).toBe(`loading`) + expect(collection.get(1)).toBeUndefined() + + resolvePersistence() + await transaction.isPersisted.promise + await preload + + expect(collection.status).toBe(`ready`) + expect(collection.get(1)).toEqual( + expect.objectContaining({ + id: 1, + updated: 0, + data: `server`, + }), + ) + } finally { + resolveList({ records: [] }) + resolvePersistence() + await transaction.isPersisted.promise.catch(() => undefined) + await collection.cleanup() + await Promise.allSettled([preload]) + } + }) + it(`settles preload when wildcard subscription startup fails`, async () => { await expectWildcardFailureSettlesPreload() }) @@ -219,6 +276,58 @@ describe(`TrailBase Integration`, () => { } }) + it(`does not publish a parked subset page after its request is aborted`, async () => { + const recordApi = new MockRecordApi() + recordApi.list.mockResolvedValue({ + records: [{ id: 1, updated: 0, data: `obsolete` }], + }) + recordApi.subscribe.mockResolvedValue(new TransformStream().readable) + const collection = createCollection( + trailBaseCollectionOptions({ + recordApi, + getKey: (item: Data) => item.id ?? -1, + startSync: true, + syncMode: `on-demand`, + parse: {}, + serialize: {}, + }), + ) + let resolvePersistence!: () => void + const persistence = new Promise((resolve) => { + resolvePersistence = resolve + }) + const transaction = createTransaction({ + mutationFn: () => persistence, + }) + const abortController = new AbortController() + + try { + await vi.waitFor(() => expect(collection.status).toBe(`ready`)) + transaction.mutate(() => + collection.insert({ id: 2, updated: 0, data: `local` }), + ) + const load = collection._sync.loadSubset({ + signal: abortController.signal, + }) + await vi.waitFor(() => expect(recordApi.list).toHaveBeenCalledOnce()) + await Promise.resolve() + await Promise.resolve() + + expect(collection.get(1)).toBeUndefined() + abortController.abort() + resolvePersistence() + await transaction.isPersisted.promise + if (load instanceof Promise) await load + + expect(collection.get(1)).toBeUndefined() + } finally { + abortController.abort() + resolvePersistence() + await transaction.isPersisted.promise.catch(() => undefined) + await collection.cleanup() + } + }) + it(`initial fetch, receive update and cancel`, async () => { const records: Array = [ { From 561a137e5bfeb930d943e3347e3601088c11c423 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 18:41:05 -0600 Subject: [PATCH 05/14] docs: add applied settlement changeset --- .changeset/settle-subset-after-publication.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/settle-subset-after-publication.md diff --git a/.changeset/settle-subset-after-publication.md b/.changeset/settle-subset-after-publication.md new file mode 100644 index 0000000000..834be1fb4c --- /dev/null +++ b/.changeset/settle-subset-after-publication.md @@ -0,0 +1,11 @@ +--- +'@tanstack/db': patch +'@tanstack/db-sqlite-persistence-core': patch +'@tanstack/electric-db-collection': patch +'@tanstack/powersync-db-collection': patch +'@tanstack/query-db-collection': patch +'@tanstack/rxdb-db-collection': patch +'@tanstack/trailbase-db-collection': patch +--- + +Settle subset loads only after their committed rows and events are visible. Preserve causal publication, cancellation, and error handling across the affected sync adapters. From bdf9d7a332d75aa3bf9276663dc0257baa006ecb Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 20:12:54 -0600 Subject: [PATCH 06/14] fix(db): preserve observable demand identity --- .changeset/canonical-demand-identity.md | 2 +- packages/db/src/query/index.ts | 2 +- packages/db/src/query/ir-stable-identity.ts | 161 +++++++++++++----- packages/db/src/query/live/ARCHITECTURE.md | 16 +- packages/db/src/query/predicate-utils.ts | 44 +++-- packages/db/src/query/subset-dedupe.ts | 6 +- .../db/tests/query/compiler/basic.test.ts | 57 +++++++ .../db/tests/query/ir-stable-identity.test.ts | 125 +++++++++++++- .../query/load-subset-oracle.property.test.ts | 27 +-- .../db/tests/query/predicate-utils.test.ts | 66 +++++++ 10 files changed, 420 insertions(+), 86 deletions(-) diff --git a/.changeset/canonical-demand-identity.md b/.changeset/canonical-demand-identity.md index dc4d0602d4..5a61f5db8d 100644 --- a/.changeset/canonical-demand-identity.md +++ b/.changeset/canonical-demand-identity.md @@ -3,4 +3,4 @@ '@tanstack/query-db-collection': patch --- -Canonicalize equivalent loadSubset queries to one demand identity while preserving distinct runtime values and window requests. Query DB now reuses the same canonical identity for its on-demand cache keys. +Canonicalize equivalent loadSubset queries to one demand identity while preserving observable output aliases, exact projected values, and distinct ordered windows. Query DB now reuses the same canonical identity for its on-demand cache keys. diff --git a/packages/db/src/query/index.ts b/packages/db/src/query/index.ts index 2c587b589b..75c020758e 100644 --- a/packages/db/src/query/index.ts +++ b/packages/db/src/query/index.ts @@ -116,7 +116,7 @@ export { isLimitSubset, isOffsetLimitSubset, isPredicateSubset, - isLoadSubsetCoveredBy, + isLoadSubsetRequestSubsumedBy, } from './predicate-utils.js' export { DeduplicatedLoadSubset } from './subset-dedupe.js' diff --git a/packages/db/src/query/ir-stable-identity.ts b/packages/db/src/query/ir-stable-identity.ts index 871321601f..3c0ead5ccf 100644 --- a/packages/db/src/query/ir-stable-identity.ts +++ b/packages/db/src/query/ir-stable-identity.ts @@ -26,7 +26,10 @@ type StableIdentityValue = | Array | { [key: string]: StableIdentityValue } -type RuntimeValueIdentity = `structural` | `semantic` +type ValueIdentityContext = + | `exact-output` + | `equality-operand` + | `ordering-operand` type AliasScope = { bindings: ReadonlyMap @@ -86,7 +89,12 @@ export function getQueryIdentity(query: QueryIR): QueryIdentity { /** Returns the semantic identity of one structured expression. */ export function getStableExpressionHash(expression: BasicExpression): string { return JSON.stringify( - canonicalizeExpression(expression, `expression`, new WeakSet(), `semantic`), + canonicalizeExpression( + expression, + `expression`, + new WeakSet(), + `exact-output`, + ), ) } @@ -143,13 +151,13 @@ export function getLoadSubsetDemandKey( options.cursor.whereFrom, `loadSubset.cursor.whereFrom`, seen, - `semantic`, + `exact-output`, ), whereCurrent: canonicalizeExpression( options.cursor.whereCurrent, `loadSubset.cursor.whereCurrent`, seen, - `semantic`, + `exact-output`, ), } if (options.cursor.lastKey !== undefined) { @@ -260,6 +268,18 @@ function canonicalizeQueryInScope( ) } + if ( + !query.select && + (Boolean(query.join?.length) || Boolean(query.groupBy?.length)) + ) { + // Without an explicit projection, these query shapes return a namespaced + // row. Its alias keys are public output and therefore part of identity. + result.implicitOutput = { + type: `namespaced`, + aliases: Array.from(scope.bindings.keys()), + } + } + if (query.join) { result.join = query.join.map((join, index) => canonicalizeJoin(join, `${path}.join[${index}]`, seen, scope), @@ -281,7 +301,7 @@ function canonicalizeQueryInScope( expression, `${path}.groupBy[${index}]`, seen, - `semantic`, + `exact-output`, scope, ), ) @@ -302,7 +322,7 @@ function canonicalizeQueryInScope( orderBy, `${path}.orderBy[${index}]`, seen, - `semantic`, + `ordering-operand`, scope, ), ) @@ -363,7 +383,7 @@ function canonicalizeLoadSubsetQuery( options.where, `${path}.where`, seen, - `semantic`, + `exact-output`, ) } @@ -373,7 +393,7 @@ function canonicalizeLoadSubsetQuery( orderBy, `${path}.orderBy[${index}]`, seen, - `semantic`, + `ordering-operand`, ), ) } @@ -394,14 +414,14 @@ function canonicalizeJoin( join.left, `${path}.left`, seen, - `semantic`, + `equality-operand`, scope, ), right: canonicalizeExpression( join.right, `${path}.right`, seen, - `semantic`, + `equality-operand`, scope, ), } @@ -488,20 +508,20 @@ function canonicalizeSelectValue( toExpression(value), path, seen, - `semantic`, + `exact-output`, scope, ) } if (isExpression(value)) { - return canonicalizeExpression(value, path, seen, `semantic`, scope) + return canonicalizeExpression(value, path, seen, `exact-output`, scope) } if (isPlainObject(value)) { return canonicalizeSelect(value as Select, path, seen, scope) } - return canonicalizeSemanticRuntimeValue(value, path, seen, scope) + return canonicalizeExactOutputRuntimeValue(value, path, seen) } function canonicalizeWhere( @@ -517,7 +537,7 @@ function canonicalizeWhere( where.expression, `${path}.expression`, seen, - `semantic`, + `exact-output`, scope, ), } @@ -529,14 +549,14 @@ function canonicalizeWhere( return result } - return canonicalizeExpression(where, path, seen, `semantic`, scope) + return canonicalizeExpression(where, path, seen, `exact-output`, scope) } function canonicalizeOrderBy( orderBy: OrderByClause, path: string, seen: WeakSet, - runtimeValueIdentity: RuntimeValueIdentity = `structural`, + valueContext: ValueIdentityContext = `exact-output`, scope?: AliasScope, ): StableIdentityValue { return { @@ -544,7 +564,7 @@ function canonicalizeOrderBy( orderBy.expression, `${path}.expression`, seen, - runtimeValueIdentity, + valueContext, scope, ), compareOptions: canonicalizeRuntimeValue( @@ -563,7 +583,7 @@ function canonicalizeExpression( | ConditionalSelect, path: string, seen: WeakSet, - runtimeValueIdentity: RuntimeValueIdentity = `structural`, + valueContext: ValueIdentityContext = `exact-output`, scope?: AliasScope, ): StableIdentityValue { if (expression.type === `ref`) { @@ -594,14 +614,24 @@ function canonicalizeExpression( return { type: `val`, value: - runtimeValueIdentity === `semantic` - ? canonicalizeSemanticRuntimeValue( + valueContext === `equality-operand` + ? canonicalizeEqualityRuntimeValue( expression.value, `${path}.value`, seen, scope, ) - : canonicalizeRuntimeValue(expression.value, `${path}.value`, seen), + : valueContext === `ordering-operand` + ? canonicalizeOrderingRuntimeValue( + expression.value, + `${path}.value`, + seen, + ) + : canonicalizeExactOutputRuntimeValue( + expression.value, + `${path}.value`, + seen, + ), } } @@ -613,25 +643,19 @@ function canonicalizeExpression( Array.isArray(expression.args[1].value) ) { const candidates = expression.args[1].value.map((value, index) => - runtimeValueIdentity === `semantic` - ? canonicalizeSemanticRuntimeValue( - value, - `${path}.args[1].value[${index}]`, - seen, - scope, - ) - : canonicalizeRuntimeValue( - value, - `${path}.args[1].value[${index}]`, - seen, - ), + canonicalizeEqualityRuntimeValue( + value, + `${path}.args[1].value[${index}]`, + seen, + scope, + ), ) return canonicalizeFunction(expression.name, [ canonicalizeExpression( expression.args[0]!, `${path}.args[0]`, seen, - runtimeValueIdentity, + `equality-operand`, scope, ), { @@ -643,12 +667,21 @@ function canonicalizeExpression( ]) } + const operandContext: ValueIdentityContext = + expression.name === `eq` + ? `equality-operand` + : expression.name === `gt` || + expression.name === `gte` || + expression.name === `lt` || + expression.name === `lte` + ? `ordering-operand` + : `exact-output` const args = expression.args.map((arg, index) => canonicalizeExpression( arg, `${path}.args[${index}]`, seen, - runtimeValueIdentity, + operandContext, scope, ), ) @@ -664,7 +697,7 @@ function canonicalizeExpression( arg, `${path}.args[${index}]`, seen, - runtimeValueIdentity, + `exact-output`, scope, ), ), @@ -679,7 +712,7 @@ function canonicalizeExpression( branch.condition, `${path}.branches[${index}].condition`, seen, - `semantic`, + `exact-output`, scope, ), value: canonicalizeSelectValue( @@ -716,14 +749,14 @@ function canonicalizeExpression( expression.correlationField, `${path}.correlationField`, seen, - `semantic`, + `equality-operand`, scope, ), childCorrelationField: canonicalizeExpression( expression.childCorrelationField, `${path}.childCorrelationField`, seen, - `semantic`, + `equality-operand`, childScope, ), fieldName: expression.fieldName, @@ -743,7 +776,7 @@ function canonicalizeExpression( projection, `${path}.parentProjection[${index}]`, seen, - `semantic`, + `exact-output`, scope, ), ) @@ -953,7 +986,19 @@ function canonicalizeRuntimeValue( throw new UnhashableQueryIRError(path, `non-plain object value`) } -function canonicalizeSemanticRuntimeValue( +function canonicalizeExactOutputRuntimeValue( + value: unknown, + path: string, + seen: WeakSet, +): StableIdentityValue { + if (typeof value === `object` && value !== null) { + return getRuntimeReferenceIdentity(value) + } + + return canonicalizeRuntimeValue(value, path, seen) +} + +function canonicalizeEqualityRuntimeValue( value: unknown, path: string, seen: WeakSet, @@ -964,7 +1009,7 @@ function canonicalizeSemanticRuntimeValue( toExpression(value), path, seen, - `semantic`, + `equality-operand`, scope, ) } @@ -994,6 +1039,38 @@ function canonicalizeSemanticRuntimeValue( return canonicalizeRuntimeValue(value, path, seen) } +function canonicalizeOrderingRuntimeValue( + value: unknown, + path: string, + seen: WeakSet, +): StableIdentityValue { + if (typeof value === `number` && Object.is(value, -0)) { + return canonicalizeRuntimeValue(0, path, seen) + } + + if (value instanceof Date && Number.isNaN(value.getTime())) { + return canonicalizeRuntimeValue(Number.NaN, path, seen) + } + + const normalized = normalizeValue(value) + if (normalized !== value && !(value instanceof Uint8Array)) { + return canonicalizeRuntimeValue(normalized, path, seen) + } + + try { + return canonicalizeRuntimeValue(value, path, seen) + } catch (error) { + if ( + error instanceof UnhashableQueryIRError && + typeof value === `object` && + value !== null + ) { + return getRuntimeReferenceIdentity(value) + } + throw error + } +} + function compareStableIdentityValues( left: StableIdentityValue, right: StableIdentityValue, diff --git a/packages/db/src/query/live/ARCHITECTURE.md b/packages/db/src/query/live/ARCHITECTURE.md index ad44e21ae5..4953a96f5f 100644 --- a/packages/db/src/query/live/ARCHITECTURE.md +++ b/packages/db/src/query/live/ARCHITECTURE.md @@ -98,8 +98,8 @@ reduction that enforces public-key congruence and multiplicity. ## Identity -Aliases are lexical query-language names. They are not runtime identities. The -query builder requires collection aliases to be unique within each lexical +Aliases are lexical query-language names rather than source runtime identities. +The query builder requires collection aliases to be unique within each lexical scope and rejects nested queries that shadow an ancestor alias. Sibling include scopes may reuse an alias because neither alias is visible to the other. Compilation then assigns opaque IDs to the accepted plan: @@ -110,8 +110,11 @@ type RelationNodeId = Brand type MaterializationEdgeId = Brand ``` -Alias text may remain as debug metadata. Renaming an accepted alias to another -unused name cannot change the compiled graph or its result. +An explicit projection can alpha-normalize aliases because its field names +define the public shape. Without a projection, joined and grouped queries return +a namespaced row whose keys are the lexical aliases. Those observable keys are +part of query identity. Alias text may otherwise remain as debug metadata +without becoming source identity. A `CanonicalCorrelationKey` is the canonical tuple of every evaluated parent-dependent value that can affect the child plan. This includes values @@ -522,8 +525,9 @@ create recursive Collection machinery. ## Normative laws 1. **Alpha-renaming:** changing any accepted alias to another unused name cannot - change results; aliases must be unique within one lexical scope and cannot - shadow an ancestor alias. Sibling scopes may reuse aliases. + change an explicitly projected result. An implicit namespaced result keeps + its aliases as public field names. Aliases must be unique within one lexical + scope and cannot shadow an ancestor alias. Sibling scopes may reuse aliases. 2. **Contribution conservation:** a public row exists exactly when its reduced supporting weight and collision policy produce one. 3. **Batch partition:** equivalent valid split and atomic deliveries converge. diff --git a/packages/db/src/query/predicate-utils.ts b/packages/db/src/query/predicate-utils.ts index 2808a19937..deb63d9c88 100644 --- a/packages/db/src/query/predicate-utils.ts +++ b/packages/db/src/query/predicate-utils.ts @@ -2,9 +2,11 @@ import { Func, Value } from './ir.js' import { UnhashableQueryIRError, getStableExpressionHash, + getStableValueHash, } from './ir-stable-identity.js' import type { BasicExpression, OrderBy, PropRef } from './ir.js' import type { LoadSubsetOptions } from '../types.js' +import type { CompareOptions } from './builder/types.js' /** * Check if one where clause is a logical subset of another. @@ -908,16 +910,17 @@ export function isPredicateSubset( } /** - * Returns whether established coverage satisfies a requested demand. + * Returns whether one acquisition request subsumes another demand. * - * Coverage is a directional relation. It must not be replaced with DemandKey + * This is a directional relationship between request shapes, not proof of + * applied or authoritative coverage. It must not be replaced with DemandKey * equality, which answers whether two exact requests are the same. */ -export function isLoadSubsetCoveredBy( +export function isLoadSubsetRequestSubsumedBy( demand: LoadSubsetOptions, - coverage: LoadSubsetOptions, + acquisitionRequest: LoadSubsetOptions, ): boolean { - return isPredicateSubset(demand, coverage) + return isPredicateSubset(demand, acquisitionRequest) } function areCursorExpressionsEqual( @@ -1220,12 +1223,31 @@ function minValue(a: any, b: any): any { return Math.min(a, b) } -function areCompareOptionsEqual( - a: { direction?: `asc` | `desc`; [key: string]: any }, - b: { direction?: `asc` | `desc`; [key: string]: any }, -): boolean { - // For now, just compare direction - could be enhanced for other options - return a.direction === b.direction +function areCompareOptionsEqual(a: CompareOptions, b: CompareOptions): boolean { + if ( + a.direction !== b.direction || + a.nulls !== b.nulls || + a.stringSort !== b.stringSort + ) { + return false + } + + if (a.stringSort !== `locale` || b.stringSort !== `locale`) { + return true + } + + if (a.locale !== b.locale) return false + if (Object.is(a.localeOptions, b.localeOptions)) return true + + try { + return ( + getStableValueHash(a.localeOptions) === + getStableValueHash(b.localeOptions) + ) + } catch (error) { + if (!(error instanceof UnhashableQueryIRError)) throw error + return false + } } interface ComparisonField { diff --git a/packages/db/src/query/subset-dedupe.ts b/packages/db/src/query/subset-dedupe.ts index f8e49d0f08..37ce49e0bf 100644 --- a/packages/db/src/query/subset-dedupe.ts +++ b/packages/db/src/query/subset-dedupe.ts @@ -1,5 +1,5 @@ import { - isLoadSubsetCoveredBy, + isLoadSubsetRequestSubsumedBy, isWhereSubset, minusWherePredicates, unionWherePredicates, @@ -111,7 +111,7 @@ export class DeduplicatedLoadSubset { // Check against limited calls if (options.limit !== undefined || options.cursor !== undefined) { const alreadyLoaded = this.limitedCalls.some((loaded) => - isLoadSubsetCoveredBy(options, loaded), + isLoadSubsetRequestSubsumedBy(options, loaded), ) if (alreadyLoaded) { @@ -125,7 +125,7 @@ export class DeduplicatedLoadSubset { const matchingInflight = this.inflightCalls.find( (inflight) => !inflight.lease.aborted && - isLoadSubsetCoveredBy(options, inflight.options), + isLoadSubsetRequestSubsumedBy(options, inflight.options), ) if (matchingInflight !== undefined) { diff --git a/packages/db/tests/query/compiler/basic.test.ts b/packages/db/tests/query/compiler/basic.test.ts index 2ff6b56adb..52f84b4df9 100644 --- a/packages/db/tests/query/compiler/basic.test.ts +++ b/packages/db/tests/query/compiler/basic.test.ts @@ -188,6 +188,63 @@ describe(`Query2 Compiler`, () => { }) }) + test(`implicit joined results expose their lexical aliases`, () => { + type Post = { id: number; userId: number; title: string } + const usersCollection = { + id: `users`, + config: { autoIndex: `off` }, + } as CollectionImpl + const postsCollection = { + id: `posts`, + config: { autoIndex: `off` }, + } as CollectionImpl + + const resultKeys = (userAlias: string, postAlias: string) => { + const graph = new D2() + const usersInput = graph.newInput<[number, User]>() + const postsInput = graph.newInput<[number, Post]>() + const query: QueryIR = { + from: new CollectionRef(usersCollection, userAlias), + join: [ + { + type: `inner`, + from: new CollectionRef(postsCollection, postAlias), + left: new PropRef([userAlias, `id`]), + right: new PropRef([postAlias, `userId`]), + }, + ], + } + const { pipeline } = compileQuery( + query, + { [userAlias]: usersInput, [postAlias]: postsInput }, + { users: usersCollection, posts: postsCollection }, + {}, + {}, + new Set(), + {}, + () => {}, + ) + const messages: Array> = [] + pipeline.pipe(output((message) => messages.push(message))) + graph.finalize() + + usersInput.sendData(new MultiSet([[[1, sampleUsers[0]!], 1]])) + postsInput.sendData( + new MultiSet([[[10, { id: 10, userId: 1, title: `Hello` }], 1]]), + ) + graph.run() + + const result = messages + .flatMap((message) => message.getInner()) + .map(([data]) => data[1][0]) + .find((row) => row !== undefined) + return Object.keys(result).sort() + } + + expect(resultKeys(`user`, `post`)).toEqual([`post`, `user`]) + expect(resultKeys(`account`, `article`)).toEqual([`account`, `article`]) + }) + test(`compiles a query with WHERE clause`, () => { const usersCollection = { id: `users`, diff --git a/packages/db/tests/query/ir-stable-identity.test.ts b/packages/db/tests/query/ir-stable-identity.test.ts index 8a5447557f..5a85d9ec48 100644 --- a/packages/db/tests/query/ir-stable-identity.test.ts +++ b/packages/db/tests/query/ir-stable-identity.test.ts @@ -46,7 +46,7 @@ import { Value, } from '../../src/query/ir.js' import { compileExpression } from '../../src/query/compiler/evaluators.js' -import { isLoadSubsetCoveredBy } from '../../src/query/predicate-utils.js' +import { isLoadSubsetRequestSubsumedBy } from '../../src/query/predicate-utils.js' import { createRuntimeReferenceIdentityFactory } from '../../src/query/runtime-reference-identity.js' import type { BasicExpression, QueryIR } from '../../src/query/ir.js' import type { LoadSubsetOptions } from '../../src/types.js' @@ -93,6 +93,28 @@ const referenceSemanticPairArbitrary = fc.oneof( ]), ) +const outputExpressionPairArbitrary: fc.Arbitrary<{ + first: BasicExpression + second: BasicExpression +}> = fc.oneof( + fc.integer().map((value) => ({ + first: new Value(value), + second: new Value(value), + })), + fc.string().map((value) => ({ + first: new Func(`concat`, [new Value(value)]), + second: new Func(`concat`, [new Value(value)]), + })), + fc.uint8Array({ minLength: 1, maxLength: 8 }).map((value) => ({ + first: new Func(`concat`, [new Value(Buffer.from(value))]), + second: new Func(`concat`, [new Value(new Uint8Array(value))]), + })), + fc.constant({ + first: new Value(-0), + second: new Value(0), + }), +) + interface Post { id: number userId: number @@ -264,7 +286,7 @@ describe(`semantic expression identity`, () => { getLoadSubsetDemandKey({ where: firstPredicate, limit: 1 }), ).not.toBe(getLoadSubsetDemandKey({ where: secondPredicate, limit: 1 })) expect( - isLoadSubsetCoveredBy( + isLoadSubsetRequestSubsumedBy( { where: firstPredicate, limit: 1 }, { where: secondPredicate, limit: 1 }, ), @@ -507,6 +529,45 @@ function createAlphaRenamedJoinQuery( } } +function createAlphaRenamedImplicitJoinQuery( + userAlias: string, + postAlias: string, +): QueryIR { + return { + from: new CollectionRef( + usersCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + userAlias, + ), + join: [ + { + type: `inner`, + from: new CollectionRef( + postsCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + postAlias, + ), + left: new PropRef([userAlias, `id`]), + right: new PropRef([postAlias, `userId`]), + }, + ], + } +} + +function createProjectedExpressionQuery(expression: BasicExpression): QueryIR { + return { + from: new CollectionRef( + usersCollection as unknown as ConstructorParameters< + typeof CollectionRef + >[0], + `user`, + ), + select: { value: expression }, + } +} + function createAlphaRenamedNestedQuery( innerAlias: string, outerAlias: string, @@ -987,6 +1048,66 @@ describe(`stable QueryIR identity smoke test`, () => { ) }) + it(`keeps aliases that define an implicit joined result shape`, () => { + expect( + getQueryIdentity(createAlphaRenamedImplicitJoinQuery(`user`, `post`)), + ).not.toBe( + getQueryIdentity( + createAlphaRenamedImplicitJoinQuery(`account`, `article`), + ), + ) + }) + + it(`keeps output-producing runtime values exact`, () => { + const bufferExpression = new Func(`concat`, [new Value(Buffer.from([65]))]) + const uint8Expression = new Func(`concat`, [ + new Value(new Uint8Array([65])), + ]) + + expect(compileExpression(bufferExpression)({})).toBe(`A`) + expect(compileExpression(uint8Expression)({})).toBe(`65`) + expect( + getQueryIdentity(createProjectedExpressionQuery(bufferExpression)), + ).not.toBe( + getQueryIdentity(createProjectedExpressionQuery(uint8Expression)), + ) + + expect( + getQueryIdentity(createProjectedExpressionQuery(new Value(-0))), + ).not.toBe(getQueryIdentity(createProjectedExpressionQuery(new Value(0)))) + + const firstObject = { value: 1 } + const secondObject = { value: 1 } + expect( + getQueryIdentity(createProjectedExpressionQuery(new Value(firstObject))), + ).not.toBe( + getQueryIdentity(createProjectedExpressionQuery(new Value(secondObject))), + ) + expect(compileExpression(new Value(firstObject))({})).toBe(firstObject) + expect(compileExpression(new Value(secondObject))({})).toBe(secondObject) + }) + + fcTest.prop([outputExpressionPairArbitrary])( + `equal query identities imply equal projected expression results`, + ({ first, second }) => { + const firstIdentity = getQueryIdentity( + createProjectedExpressionQuery(first), + ) + const secondIdentity = getQueryIdentity( + createProjectedExpressionQuery(second), + ) + + if (firstIdentity === secondIdentity) { + expect( + Object.is( + compileExpression(first)({}), + compileExpression(second)({}), + ), + ).toBe(true) + } + }, + ) + fcTest.prop([ fc .stringMatching(/^[a-z][a-z0-9]{0,8}$/) diff --git a/packages/db/tests/query/load-subset-oracle.property.test.ts b/packages/db/tests/query/load-subset-oracle.property.test.ts index fd6296e0b0..6c3bdead0e 100644 --- a/packages/db/tests/query/load-subset-oracle.property.test.ts +++ b/packages/db/tests/query/load-subset-oracle.property.test.ts @@ -1865,7 +1865,7 @@ describe(`loadSubset coverage oracle`, () => { ], ] as const)( `discovered trace: a different %s starts a distinct window load`, - async (_name, firstOptions, secondOptions) => { + (_name, firstOptions, secondOptions) => { const createRequest = ( compareOptions: typeof firstOptions | typeof secondOptions, ): WindowRequest => ({ @@ -1875,25 +1875,12 @@ describe(`loadSubset coverage oracle`, () => { limit: 1, ...compareOptions, }) - await expectAssertionFailure( - () => - Promise.resolve().then(() => { - try { - expect( - countWindowLoads([ - createRequest(firstOptions), - createRequest(secondOptions), - ]), - ).toBe(2) - } catch (error) { - throw new TraceAssertionError(0, error) - } - }), - { - checkpoint: 0, - classify: ({ actual, expected }) => actual === 1 && expected === 2, - }, - )() + expect( + countWindowLoads([ + createRequest(firstOptions), + createRequest(secondOptions), + ]), + ).toBe(2) }, ) diff --git a/packages/db/tests/query/predicate-utils.test.ts b/packages/db/tests/query/predicate-utils.test.ts index 721c07f5ae..9d832f39f7 100644 --- a/packages/db/tests/query/predicate-utils.test.ts +++ b/packages/db/tests/query/predicate-utils.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { isLimitSubset, + isLoadSubsetRequestSubsumedBy, isOffsetLimitSubset, isOrderBySubset, isPredicateSubset, @@ -676,6 +677,71 @@ describe(`isOrderBySubset`, () => { expect(isOrderBySubset(subset, superset)).toBe(false) }) + it.each([ + [ + `null placement`, + { direction: `asc`, nulls: `first`, stringSort: `lexical` } as const, + { direction: `asc`, nulls: `last`, stringSort: `lexical` } as const, + ], + [ + `string sort mode`, + { direction: `asc`, nulls: `last`, stringSort: `lexical` } as const, + { direction: `asc`, nulls: `last`, stringSort: `locale` } as const, + ], + [ + `locale`, + { + direction: `asc`, + nulls: `last`, + stringSort: `locale`, + locale: `en-US`, + } as const, + { + direction: `asc`, + nulls: `last`, + stringSort: `locale`, + locale: `de-DE`, + } as const, + ], + [ + `locale options`, + { + direction: `asc`, + nulls: `last`, + stringSort: `locale`, + locale: `en-US`, + localeOptions: { numeric: true, sensitivity: `base` }, + } as const, + { + direction: `asc`, + nulls: `last`, + stringSort: `locale`, + locale: `en-US`, + localeOptions: { numeric: false, sensitivity: `base` }, + } as const, + ], + ])(`should return false when %s differs`, (_label, first, second) => { + const expression = ref(`name`) + expect( + isOrderBySubset( + [{ expression, compareOptions: first }], + [{ expression, compareOptions: second }], + ), + ).toBe(false) + expect( + isLoadSubsetRequestSubsumedBy( + { + orderBy: [{ expression, compareOptions: first }], + limit: 10, + }, + { + orderBy: [{ expression, compareOptions: second }], + limit: 20, + }, + ), + ).toBe(false) + }) + it(`should return false when subset is longer than superset`, () => { const subset: OrderBy = [ orderByClause(ref(`age`), `asc`), From 1384a47c959138efd160917f1f04f38f99075d6c Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 20:39:14 -0600 Subject: [PATCH 07/14] fix(db): address demand identity review findings --- packages/db/src/query/ir-stable-identity.ts | 17 +++-- packages/db/src/query/predicate-utils.ts | 73 ++++++++++++++++--- packages/db/src/query/subset-dedupe.ts | 27 ++++++- .../db/tests/query/predicate-utils.test.ts | 44 +++++++++++ packages/db/tests/query/subset-dedupe.test.ts | 25 +++++++ .../load-subset-lifecycle-oracle.test.ts | 63 ++++++++-------- 6 files changed, 200 insertions(+), 49 deletions(-) diff --git a/packages/db/src/query/ir-stable-identity.ts b/packages/db/src/query/ir-stable-identity.ts index 3c0ead5ccf..d7fe07e306 100644 --- a/packages/db/src/query/ir-stable-identity.ts +++ b/packages/db/src/query/ir-stable-identity.ts @@ -827,12 +827,17 @@ function canonicalizeFunction( function sortUniqueStableIdentityValues( values: Array, ): Array { - values.sort(compareStableIdentityValues) - return values.filter( - (value, index) => - index === 0 || - compareStableIdentityValues(value, values[index - 1]!) !== 0, - ) + const keyedValues = values.map((value) => ({ + key: JSON.stringify(value), + value, + })) + keyedValues.sort((a, b) => (a.key < b.key ? -1 : a.key > b.key ? 1 : 0)) + return keyedValues + .filter( + (entry, index) => + index === 0 || entry.key !== keyedValues[index - 1]!.key, + ) + .map((entry) => entry.value) } function isCanonicalFunction( diff --git a/packages/db/src/query/predicate-utils.ts b/packages/db/src/query/predicate-utils.ts index deb63d9c88..27a43f0f16 100644 --- a/packages/db/src/query/predicate-utils.ts +++ b/packages/db/src/query/predicate-utils.ts @@ -46,7 +46,7 @@ export function isWhereSubset( return true } - return isWhereSubsetInternal(subset!, superset!) + return isWhereSubsetInternal(subset!, superset!, new WeakMap()) } function makeDisjunction( @@ -71,6 +71,7 @@ function convertInToOr(inField: InField) { function isWhereSubsetInternal( subset: BasicExpression, superset: BasicExpression, + expressionHashes: ExpressionHashCache, ): boolean { // If subset is false it is requesting no data, // thus the result set is empty @@ -80,7 +81,7 @@ function isWhereSubsetInternal( } // If expressions are structurally equal, subset relationship holds - if (areExpressionsEqual(subset, superset)) { + if (areExpressionsEqual(subset, superset, expressionHashes)) { return true } @@ -89,7 +90,11 @@ function isWhereSubsetInternal( // Example: (age > 20) ⊆ (age > 10 AND status = 'active') is false (doesn't imply status condition) if (superset.type === `func` && superset.name === `and`) { return superset.args.every((arg) => - isWhereSubsetInternal(subset, arg as BasicExpression), + isWhereSubsetInternal( + subset, + arg as BasicExpression, + expressionHashes, + ), ) } @@ -98,7 +103,11 @@ function isWhereSubsetInternal( // decomposes the subset first: A ⊆ or(C, D) AND B ⊆ or(C, D). if (subset.type === `func` && subset.name === `or`) { return subset.args.every((arg) => - isWhereSubsetInternal(arg as BasicExpression, superset), + isWhereSubsetInternal( + arg as BasicExpression, + superset, + expressionHashes, + ), ) } @@ -107,7 +116,11 @@ function isWhereSubsetInternal( // match a structurally equal disjunct via areExpressionsEqual. if (superset.type === `func` && superset.name === `or`) { return superset.args.some((arg) => - isWhereSubsetInternal(subset, arg as BasicExpression), + isWhereSubsetInternal( + subset, + arg as BasicExpression, + expressionHashes, + ), ) } @@ -115,7 +128,11 @@ function isWhereSubsetInternal( if (subset.type === `func` && subset.name === `and`) { // For (A AND B) ⊆ C, since (A AND B) implies A, we check if any conjunct implies C return subset.args.some((arg) => - isWhereSubsetInternal(arg as BasicExpression, superset), + isWhereSubsetInternal( + arg as BasicExpression, + superset, + expressionHashes, + ), ) } @@ -124,14 +141,22 @@ function isWhereSubsetInternal( if (subset.type === `func` && subset.name === `in`) { const inField = extractInField(subset) if (inField) { - return isWhereSubsetInternal(convertInToOr(inField), superset) + return isWhereSubsetInternal( + convertInToOr(inField), + superset, + expressionHashes, + ) } } if (superset.type === `func` && superset.name === `in`) { const inField = extractInField(superset) if (inField) { - return isWhereSubsetInternal(subset, convertInToOr(inField)) + return isWhereSubsetInternal( + subset, + convertInToOr(inField), + expressionHashes, + ) } } @@ -1088,13 +1113,41 @@ function findPredicateWithOperator( }) } -function areExpressionsEqual(a: BasicExpression, b: BasicExpression): boolean { +const unhashableExpression = Symbol(`unhashableExpression`) +type ExpressionHashCache = WeakMap< + BasicExpression, + string | typeof unhashableExpression +> + +function getCachedExpressionHash( + expression: BasicExpression, + expressionHashes: ExpressionHashCache, +): string | typeof unhashableExpression { + const cachedHash = expressionHashes.get(expression) + if (cachedHash !== undefined) return cachedHash + try { - return getStableExpressionHash(a) === getStableExpressionHash(b) + const hash = getStableExpressionHash(expression) + expressionHashes.set(expression, hash) + return hash } catch (error) { if (!(error instanceof UnhashableQueryIRError)) throw error + expressionHashes.set(expression, unhashableExpression) + return unhashableExpression + } +} + +function areExpressionsEqual( + a: BasicExpression, + b: BasicExpression, + expressionHashes: ExpressionHashCache = new WeakMap(), +): boolean { + const aHash = getCachedExpressionHash(a, expressionHashes) + const bHash = getCachedExpressionHash(b, expressionHashes) + if (aHash === unhashableExpression || bHash === unhashableExpression) { return areExpressionsStructurallyEqual(a, b) } + return aHash === bHash } function areExpressionsStructurallyEqual( diff --git a/packages/db/src/query/subset-dedupe.ts b/packages/db/src/query/subset-dedupe.ts index 37ce49e0bf..4e0a978ec4 100644 --- a/packages/db/src/query/subset-dedupe.ts +++ b/packages/db/src/query/subset-dedupe.ts @@ -4,6 +4,7 @@ import { minusWherePredicates, unionWherePredicates, } from './predicate-utils.js' +import { Func, PropRef, Value } from './ir.js' import type { BasicExpression } from './ir.js' import type { LoadSubsetOptions } from '../types.js' @@ -324,10 +325,34 @@ function createSharedAbortLease( export function cloneOptions(options: LoadSubsetOptions): LoadSubsetOptions { return { ...options, + where: options.where ? cloneBasicExpression(options.where) : undefined, orderBy: options.orderBy?.map((clause) => ({ ...clause, + expression: cloneBasicExpression(clause.expression), compareOptions: { ...clause.compareOptions }, })), - cursor: options.cursor ? { ...options.cursor } : undefined, + cursor: options.cursor + ? { + ...options.cursor, + whereFrom: cloneBasicExpression(options.cursor.whereFrom), + whereCurrent: cloneBasicExpression(options.cursor.whereCurrent), + } + : undefined, + } +} + +function cloneBasicExpression( + expression: BasicExpression, +): BasicExpression { + switch (expression.type) { + case `ref`: + return new PropRef([...expression.path]) + case `val`: + return new Value(expression.value) + case `func`: + return new Func( + expression.name, + expression.args.map((arg) => cloneBasicExpression(arg)), + ) } } diff --git a/packages/db/tests/query/predicate-utils.test.ts b/packages/db/tests/query/predicate-utils.test.ts index 9d832f39f7..26e5938790 100644 --- a/packages/db/tests/query/predicate-utils.test.ts +++ b/packages/db/tests/query/predicate-utils.test.ts @@ -915,6 +915,50 @@ describe(`isPredicateSubset`, () => { expect(isPredicateSubset(subset, superset)).toBe(true) }) + it(`does not normalize distinct comparison operators at a limited boundary`, () => { + const subset: LoadSubsetOptions = { + where: gt(ref(`age`), val(18)), + limit: 10, + } + const superset: LoadSubsetOptions = { + where: gte(ref(`age`), val(18)), + limit: 20, + } + + expect(isPredicateSubset(subset, superset)).toBe(false) + }) + + it(`does not retain expression hashes across comparison operations`, () => { + const subset = gt(ref(`age`), val(18)) + const superset = gt(ref(`age`), val(18)) + + expect(isWhereSubset(subset, superset)).toBe(true) + superset.name = `lt` + expect(isWhereSubset(subset, superset)).toBe(false) + }) + + it(`hashes a repeated expression once per subset comparison`, () => { + let valueReads = 0 + const countedValue = val(1) + Object.defineProperty(countedValue, `value`, { + configurable: true, + get: () => { + valueReads++ + return 1 + }, + }) + const subset = func(`custom-subset`, countedValue) + const superset = func( + `or`, + ...Array.from({ length: 4 }, (_, index) => + func(`custom-superset-${index}`, val(index)), + ), + ) + + expect(isWhereSubset(subset, superset)).toBe(false) + expect(valueReads).toBe(1) + }) + it(`should return false for limited superset with different where clause`, () => { // Even if subset's where is more restrictive, it can't be a subset // of a limited superset with a different where clause. diff --git a/packages/db/tests/query/subset-dedupe.test.ts b/packages/db/tests/query/subset-dedupe.test.ts index 4c5d8d4f96..4ad426b26b 100644 --- a/packages/db/tests/query/subset-dedupe.test.ts +++ b/packages/db/tests/query/subset-dedupe.test.ts @@ -1387,5 +1387,30 @@ describe(`createDeduplicatedLoadSubset`, () => { expect(result).toBe(true) expect(callCount).toBe(1) }) + + it(`does not let caller mutations change a stored cursor boundary`, async () => { + const loadSubset = vi.fn().mockResolvedValue(undefined) + const deduplicated = new DeduplicatedLoadSubset({ loadSubset }) + const mutableBoundary = val(1) + const firstCursor = { + whereFrom: gt(ref(`id`), mutableBoundary), + whereCurrent: eq(ref(`id`), mutableBoundary), + lastKey: 1, + } + + await deduplicated.loadSubset({ cursor: firstCursor, limit: 10 }) + mutableBoundary.value = 2 + + await deduplicated.loadSubset({ + cursor: { + whereFrom: gt(ref(`id`), val(2)), + whereCurrent: eq(ref(`id`), val(2)), + lastKey: 1, + }, + limit: 10, + }) + + expect(loadSubset).toHaveBeenCalledTimes(2) + }) }) }) diff --git a/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts b/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts index 38dc3e9a19..b7d4b395ad 100644 --- a/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts +++ b/packages/query-db-collection/tests/load-subset-lifecycle-oracle.test.ts @@ -349,20 +349,9 @@ async function expectDeferredStartupReadyDoesNotOverrideError(): Promise { async function expectEquivalentPredicatesShareOneLoad( form: `commutative-and` | `commutative-or` | `reversed-equality`, ): Promise { - const queryClient = createQueryClient() - const id = `load-subset-canonical-predicate-${collectionSequence++}` - const queryFn = vi.fn().mockResolvedValue([{ id: `a`, group: `x` }]) - const collection = createCollection( - queryCollectionOptions({ - id, - queryClient, - queryKey: [id], - queryFn, - getKey: (row) => row.id, - startSync: true, - syncMode: `on-demand`, - retry: false, - }), + const { queryClient, collection, queryFn } = createOnDemandCollection( + `load-subset-canonical-predicate`, + [{ id: `a`, group: `x` }], ) const firstComparison = new IR.Func(`eq`, [ new IR.PropRef([`id`]), @@ -407,20 +396,9 @@ async function expectEquivalentComparisonValuesShareOneLoad( firstValue: unknown, secondValue: unknown, ): Promise { - const queryClient = createQueryClient() - const id = `load-subset-comparison-value-${collectionSequence++}` - const queryFn = vi.fn().mockResolvedValue([{ id: `a` }]) - const collection = createCollection( - queryCollectionOptions({ - id, - queryClient, - queryKey: [id], - queryFn, - getKey: (row) => row.id, - startSync: true, - syncMode: `on-demand`, - retry: false, - }), + const { queryClient, collection, queryFn } = createOnDemandCollection( + `load-subset-comparison-value`, + [{ id: `a` }], ) const value = new IR.PropRef([`value`]) @@ -438,6 +416,25 @@ async function expectEquivalentComparisonValuesShareOneLoad( } } +function createOnDemandCollection(idPrefix: string, rows: Array) { + const queryClient = createQueryClient() + const id = `${idPrefix}-${collectionSequence++}` + const queryFn = vi.fn().mockResolvedValue(rows) + const collection = createCollection( + queryCollectionOptions({ + id, + queryClient, + queryKey: [id], + queryFn, + getKey: (row) => row.id, + startSync: true, + syncMode: `on-demand`, + retry: false, + }), + ) + return { queryClient, collection, queryFn } +} + async function expectFinalOwnerCleanupAbortsQuery(): Promise { const queryClient = createQueryClient() const id = `load-subset-cancel-final-owner-${collectionSequence++}` @@ -567,10 +564,12 @@ describe(`loadSubset lifecycle oracle`, () => { await expectDeferredStartupReadyDoesNotOverrideError() }) - it(`commutative predicate forms share one query-db transport load`, async () => { - await expectEquivalentPredicatesShareOneLoad(`commutative-and`) - await expectEquivalentPredicatesShareOneLoad(`commutative-or`) - }) + it.each([`commutative-and`, `commutative-or`] as const)( + `%s predicate forms share one query-db transport load`, + async (form) => { + await expectEquivalentPredicatesShareOneLoad(form) + }, + ) it(`reversed equality operands share one query-db transport load`, async () => { await expectEquivalentPredicatesShareOneLoad(`reversed-equality`) From 3b4c4b443042aafbb79bddb0f3b55e5425d47130 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Mon, 24 Aug 2026 20:39:14 -0600 Subject: [PATCH 08/14] fix(db): preserve cancellation through settlement --- .../src/persisted.ts | 66 ++++++----- .../tests/persisted.test.ts | 108 ++++++++++++++++++ .../electric-db-collection/src/electric.ts | 8 +- .../tests/electric.test.ts | 57 +++++++++ .../trailbase-db-collection/src/trailbase.ts | 1 + .../tests/trailbase.test.ts | 7 +- 6 files changed, 215 insertions(+), 32 deletions(-) diff --git a/packages/db-sqlite-persistence-core/src/persisted.ts b/packages/db-sqlite-persistence-core/src/persisted.ts index fb02f35643..6f7c563d87 100644 --- a/packages/db-sqlite-persistence-core/src/persisted.ts +++ b/packages/db-sqlite-persistence-core/src/persisted.ts @@ -434,7 +434,7 @@ type SyncControlFns = { | { type: `delete`; key: TKey }, ) => void) | null - commit: (() => SyncAppliedReceipt) | null + commit: ((signal?: AbortSignal) => SyncAppliedReceipt) | null truncate: (() => void) | null metadata: SyncMetadataApi | null } @@ -587,6 +587,7 @@ type BufferedSyncTransaction = { > truncate: boolean internal: boolean + signal?: AbortSignal resolveApplied?: () => void } @@ -841,7 +842,9 @@ class PersistedCollectionRuntime< const commit = syncControls.commit this.syncControls = { ...syncControls, - commit: commit ? () => this.trackAppliedReceipt(commit()) : null, + commit: commit + ? (signal) => this.trackAppliedReceipt(commit(signal)) + : null, } } @@ -1348,17 +1351,21 @@ class PersistedCollectionRuntime< } } - private async applyBufferedSyncTransactionUnsafe( + private applyBufferedSyncTransactionUnsafe( transaction: BufferedSyncTransaction, - ): Promise { + ): void { + if (transaction.signal?.aborted) { + transaction.resolveApplied?.() + return + } + const { begin, write, commit, truncate, metadata } = this.syncControls if (!begin || !write || !commit) { transaction.resolveApplied?.() return } - let receiptLinkedToCoreApplication = false - const applyToCollection = (): boolean => { + const applyToCollection = (): SyncAppliedReceipt => { begin() if (transaction.truncate) { @@ -1396,31 +1403,34 @@ class PersistedCollectionRuntime< } } - const applied = commit() + const applied = commit(transaction.signal) if (applied === true) { transaction.resolveApplied?.() - return false } else { void applied.then(() => transaction.resolveApplied?.()) - return true } + return applied } try { if (transaction.internal) { - receiptLinkedToCoreApplication = - this.withInternalApply(applyToCollection) + this.withInternalApply(applyToCollection) return } - receiptLinkedToCoreApplication = applyToCollection() - await this.persistAndBroadcastExternalSyncTransactionUnsafe(transaction) + const applied = applyToCollection() + const persistAfterApplication = async () => { + if (applied !== true) await applied + if (transaction.signal?.aborted) return + await this.persistAndBroadcastExternalSyncTransaction(transaction) + } + void persistAfterApplication().catch((error) => { + console.warn(`Failed to persist buffered sync transaction:`, error) + }) } catch (error) { // A replay failure before commit has no core receipt that cleanup can // settle. Release the wrapper receipt so the source load cannot hang. - if (!receiptLinkedToCoreApplication) { - transaction.resolveApplied?.() - } + transaction.resolveApplied?.() throw error } } @@ -2506,13 +2516,14 @@ function createWrappedSyncConfig< params.truncate() } }, - commit: () => { + commit: (signal?: AbortSignal) => { const openTransaction = transactionStack.pop() if (!openTransaction) { - return params.commit() + return params.commit(signal) } if (openTransaction.queuedBecauseHydrating) { + if (signal?.aborted) return true let resolveApplied!: () => void const applied = new Promise((resolve) => { resolveApplied = resolve @@ -2524,15 +2535,18 @@ function createWrappedSyncConfig< openTransaction.collectionMetadataWrites, truncate: openTransaction.truncate, internal: openTransaction.internal, + signal, resolveApplied, }) return applied } - const applied = params.commit() + const applied = params.commit(signal) if (!openTransaction.internal) { - void runtime - .persistAndBroadcastExternalSyncTransaction({ + const persistAfterApplication = async () => { + if (applied !== true) await applied + if (signal?.aborted) return + await runtime.persistAndBroadcastExternalSyncTransaction({ operations: openTransaction.operations, rowMetadataWrites: openTransaction.rowMetadataWrites, collectionMetadataWrites: @@ -2540,12 +2554,10 @@ function createWrappedSyncConfig< truncate: openTransaction.truncate, internal: false, }) - .catch((error) => { - console.warn( - `Failed to persist wrapped sync transaction:`, - error, - ) - }) + } + void persistAfterApplication().catch((error) => { + console.warn(`Failed to persist wrapped sync transaction:`, error) + }) } return applied }, diff --git a/packages/db-sqlite-persistence-core/tests/persisted.test.ts b/packages/db-sqlite-persistence-core/tests/persisted.test.ts index abcc499f48..8c00cff939 100644 --- a/packages/db-sqlite-persistence-core/tests/persisted.test.ts +++ b/packages/db-sqlite-persistence-core/tests/persisted.test.ts @@ -818,6 +818,53 @@ describe(`persistedCollectionOptions`, () => { ) }) + it(`does not apply or persist a wrapped sync transaction committed with an aborted signal`, async () => { + const adapter = createRecordingAdapter() + let remoteBegin: (() => void) | undefined + let remoteWrite: + | ((message: { type: `insert`; value: Todo }) => void) + | undefined + let remoteCommit: + | ((signal?: AbortSignal) => true | Promise) + | undefined + const collection = createCollection( + persistedCollectionOptions({ + id: `sync-present-aborted-commit`, + getKey: (item) => item.id, + sync: { + sync: ({ begin, write, commit, markReady }) => { + remoteBegin = begin + remoteWrite = write as (message: { + type: `insert` + value: Todo + }) => void + remoteCommit = commit + markReady() + }, + }, + persistence: { adapter }, + }), + ) + + try { + await collection.stateWhenReady() + const abortController = new AbortController() + abortController.abort() + remoteBegin?.() + remoteWrite?.({ + type: `insert`, + value: { id: `aborted`, title: `Must not publish` }, + }) + await remoteCommit?.(abortController.signal) + await flushAsyncWork() + + expect(collection.get(`aborted`)).toBeUndefined() + expect(adapter.applyCommittedTxCalls).toHaveLength(0) + } finally { + await collection.cleanup() + } + }) + it(`preserves row metadata set before a metadata-less insert in the same sync transaction`, async () => { const adapter = createRecordingAdapter() const ownership = { queryCollection: { owners: [`gc:q1`] } } @@ -1099,6 +1146,67 @@ describe(`persistedCollectionOptions`, () => { }) }) + it(`discards a hydration-buffered transaction aborted before replay`, async () => { + const adapter = createRecordingAdapter() + let resolveLoadSubset: (() => void) | undefined + adapter.loadSubset = async () => { + await new Promise((resolve) => { + resolveLoadSubset = resolve + }) + return [] + } + let remoteBegin: (() => void) | undefined + let remoteWrite: + | ((message: { type: `insert`; value: Todo }) => void) + | undefined + let remoteCommit: + | ((signal?: AbortSignal) => true | Promise) + | undefined + const collection = createCollection( + persistedCollectionOptions({ + id: `sync-present-aborted-hydration-queue`, + getKey: (item) => item.id, + sync: { + sync: ({ begin, write, commit, markReady }) => { + remoteBegin = begin + remoteWrite = write as (message: { + type: `insert` + value: Todo + }) => void + remoteCommit = commit + markReady() + }, + }, + persistence: { adapter }, + }), + ) + + try { + const ready = collection.stateWhenReady() + for (let attempt = 0; attempt < 20 && !resolveLoadSubset; attempt++) { + await flushAsyncWork() + } + const abortController = new AbortController() + remoteBegin?.() + remoteWrite?.({ + type: `insert`, + value: { id: `aborted`, title: `Must not replay` }, + }) + const applied = remoteCommit?.(abortController.signal) + abortController.abort() + resolveLoadSubset?.() + await ready + if (applied !== true) await applied + await flushAsyncWork() + + expect(collection.get(`aborted`)).toBeUndefined() + expect(adapter.applyCommittedTxCalls).toHaveLength(0) + } finally { + resolveLoadSubset?.() + await collection.cleanup() + } + }) + it(`settles a hydration-buffered receipt when replay fails`, async () => { const adapter = createRecordingAdapter() let resolveLoadSubset: (() => void) | undefined diff --git a/packages/electric-db-collection/src/electric.ts b/packages/electric-db-collection/src/electric.ts index a148b26da6..fbc62a8f23 100644 --- a/packages/electric-db-collection/src/electric.ts +++ b/packages/electric-db-collection/src/electric.ts @@ -542,7 +542,7 @@ function createLoadSubsetDedupe>({ value: T metadata: Record }) => void - commit: () => SyncAppliedReceipt + commit: (signal?: AbortSignal) => SyncAppliedReceipt getCommitCursor: () => number waitForCommitsAfter: (cursor: number) => Promise collectionId?: string @@ -599,7 +599,7 @@ function createLoadSubsetDedupe>({ metadata: { ...row.headers }, }) } - await commit() + await commit(opts.signal) debug(`${logPrefix}Applied snapshot with ${rows.length} rows`) } } catch (error) { @@ -1503,9 +1503,9 @@ function createElectricSync>( } = params let commitSequence = 0 const pendingAppliedReceipts = new Map>() - const commit = (): SyncAppliedReceipt => { + const commit = (signal?: AbortSignal): SyncAppliedReceipt => { const sequence = ++commitSequence - const applied = commitSyncTransaction() + const applied = commitSyncTransaction(signal) if (applied === true) { return true } diff --git a/packages/electric-db-collection/tests/electric.test.ts b/packages/electric-db-collection/tests/electric.test.ts index bc156fd159..56c518860f 100644 --- a/packages/electric-db-collection/tests/electric.test.ts +++ b/packages/electric-db-collection/tests/electric.test.ts @@ -3003,6 +3003,63 @@ describe(`Electric Integration`, () => { } }) + it(`does not publish a progressive snapshot aborted while its commit is parked`, async () => { + mockFetchSnapshot.mockResolvedValue({ + metadata: {}, + data: [ + { + key: `2`, + value: { id: 2, name: `Obsolete snapshot` }, + headers: { operation: `insert` }, + }, + ], + }) + mockSubscribe.mockImplementation(() => () => {}) + const testCollection = createCollection( + electricCollectionOptions({ + id: `progressive-parked-abort-test`, + shapeOptions: { + url: `http://test-url`, + params: { table: `test_table` }, + }, + syncMode: `progressive`, + getKey: (item: Row) => item.id as number, + startSync: true, + }), + ) + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + const abortController = new AbortController() + + try { + transaction.mutate(() => + testCollection.insert({ id: 3, name: `Local row` }), + ) + const load = testCollection._sync.loadSubset({ + limit: 1, + signal: abortController.signal, + }) + await vi.waitFor(() => expect(mockFetchSnapshot).toHaveBeenCalledOnce()) + await Promise.resolve() + await Promise.resolve() + + expect(testCollection.has(2)).toBe(false) + abortController.abort() + persistence.resolve() + await transaction.isPersisted.promise + if (load instanceof Promise) await load + + expect(testCollection.has(2)).toBe(false) + } finally { + abortController.abort() + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await testCollection.cleanup() + } + }) + it(`should not request snapshots when loadSubset is called in eager mode`, async () => { vi.clearAllMocks() diff --git a/packages/trailbase-db-collection/src/trailbase.ts b/packages/trailbase-db-collection/src/trailbase.ts index d256815721..7afeea7a16 100644 --- a/packages/trailbase-db-collection/src/trailbase.ts +++ b/packages/trailbase-db-collection/src/trailbase.ts @@ -254,6 +254,7 @@ export function trailBaseCollectionOptions< } await commit(opts.signal) + if (cancelled || opts.signal?.aborted) return remaining -= length diff --git a/packages/trailbase-db-collection/tests/trailbase.test.ts b/packages/trailbase-db-collection/tests/trailbase.test.ts index f784749c01..c55189309e 100644 --- a/packages/trailbase-db-collection/tests/trailbase.test.ts +++ b/packages/trailbase-db-collection/tests/trailbase.test.ts @@ -279,7 +279,11 @@ describe(`TrailBase Integration`, () => { it(`does not publish a parked subset page after its request is aborted`, async () => { const recordApi = new MockRecordApi() recordApi.list.mockResolvedValue({ - records: [{ id: 1, updated: 0, data: `obsolete` }], + records: Array.from({ length: 256 }, (_, index) => ({ + id: index + 1, + updated: 0, + data: `obsolete`, + })), }) recordApi.subscribe.mockResolvedValue(new TransformStream().readable) const collection = createCollection( @@ -320,6 +324,7 @@ describe(`TrailBase Integration`, () => { if (load instanceof Promise) await load expect(collection.get(1)).toBeUndefined() + expect(recordApi.list).toHaveBeenCalledOnce() } finally { abortController.abort() resolvePersistence() From a6b5ffaac35c2ab69898add9187681d02bb82d09 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 25 Aug 2026 07:32:52 -0600 Subject: [PATCH 09/14] fix(db): keep demand history immutable --- packages/db/src/query/predicate-utils.ts | 9 +++ packages/db/src/query/subset-dedupe.ts | 70 +++++++++++++++++-- .../db/tests/query/predicate-utils.test.ts | 18 +++++ packages/db/tests/query/subset-dedupe.test.ts | 33 +++++++++ 4 files changed, 125 insertions(+), 5 deletions(-) diff --git a/packages/db/src/query/predicate-utils.ts b/packages/db/src/query/predicate-utils.ts index 27a43f0f16..3241f9e55d 100644 --- a/packages/db/src/query/predicate-utils.ts +++ b/packages/db/src/query/predicate-utils.ts @@ -913,6 +913,15 @@ export function isPredicateSubset( } } + // A cursor-relative request is also a finite window. Even when it has no + // numeric limit, a different predicate can select rows outside that window. + if ( + superset.cursor !== undefined && + !areWhereClausesEqual(subset.where, superset.where) + ) { + return false + } + if (superset.limit !== undefined) { // For limited supersets, where clauses must be equal if (!areWhereClausesEqual(subset.where, superset.where)) { diff --git a/packages/db/src/query/subset-dedupe.ts b/packages/db/src/query/subset-dedupe.ts index 4e0a978ec4..f5b98b2531 100644 --- a/packages/db/src/query/subset-dedupe.ts +++ b/packages/db/src/query/subset-dedupe.ts @@ -325,7 +325,9 @@ function createSharedAbortLease( export function cloneOptions(options: LoadSubsetOptions): LoadSubsetOptions { return { ...options, - where: options.where ? cloneBasicExpression(options.where) : undefined, + where: options.where + ? cloneBasicExpression(options.where, `predicate`) + : undefined, orderBy: options.orderBy?.map((clause) => ({ ...clause, expression: cloneBasicExpression(clause.expression), @@ -334,25 +336,83 @@ export function cloneOptions(options: LoadSubsetOptions): LoadSubsetOptions { cursor: options.cursor ? { ...options.cursor, - whereFrom: cloneBasicExpression(options.cursor.whereFrom), - whereCurrent: cloneBasicExpression(options.cursor.whereCurrent), + whereFrom: cloneBasicExpression( + options.cursor.whereFrom, + `predicate`, + ), + whereCurrent: cloneBasicExpression( + options.cursor.whereCurrent, + `predicate`, + ), } : undefined, } } +type ExpressionCloneContext = `exact` | `predicate` | `comparison` + function cloneBasicExpression( expression: BasicExpression, + context: ExpressionCloneContext = `exact`, ): BasicExpression { switch (expression.type) { case `ref`: return new PropRef([...expression.path]) case `val`: - return new Value(expression.value) + return new Value( + context === `comparison` + ? snapshotComparisonValue(expression.value) + : expression.value, + ) case `func`: return new Func( expression.name, - expression.args.map((arg) => cloneBasicExpression(arg)), + expression.args.map((arg, index) => { + if ( + context === `predicate` && + expression.name === `in` && + index === 1 && + arg.type === `val` && + Array.isArray(arg.value) + ) { + return new Value( + arg.value.map((value) => snapshotComparisonValue(value)), + ) + } + + const argumentContext = + context === `predicate` && isComparisonFunction(expression.name) + ? `comparison` + : context + return cloneBasicExpression(arg, argumentContext) + }), ) } } + +function isComparisonFunction(name: string): boolean { + return ( + name === `eq` || + name === `gt` || + name === `gte` || + name === `lt` || + name === `lte` + ) +} + +function snapshotComparisonValue(value: T): T { + if (value instanceof Date) { + return new Date(value.getTime()) as T + } + + if (typeof Buffer !== `undefined` && value instanceof Buffer) { + return Buffer.from(value) as T + } + + if (value instanceof Uint8Array) { + return value.slice() as T + } + + // Other objects use reference equality in predicate identity and comparison. + return value +} diff --git a/packages/db/tests/query/predicate-utils.test.ts b/packages/db/tests/query/predicate-utils.test.ts index 26e5938790..6471950dee 100644 --- a/packages/db/tests/query/predicate-utils.test.ts +++ b/packages/db/tests/query/predicate-utils.test.ts @@ -928,6 +928,24 @@ describe(`isPredicateSubset`, () => { expect(isPredicateSubset(subset, superset)).toBe(false) }) + it(`requires equal predicates for a cursor-relative superset`, () => { + const cursor = { + whereFrom: gt(ref(`id`), val(10)), + whereCurrent: eq(ref(`id`), val(10)), + lastKey: 10, + } + const subset: LoadSubsetOptions = { + where: gt(ref(`age`), val(18)), + cursor, + } + const superset: LoadSubsetOptions = { + where: gte(ref(`age`), val(18)), + cursor, + } + + expect(isPredicateSubset(subset, superset)).toBe(false) + }) + it(`does not retain expression hashes across comparison operations`, () => { const subset = gt(ref(`age`), val(18)) const superset = gt(ref(`age`), val(18)) diff --git a/packages/db/tests/query/subset-dedupe.test.ts b/packages/db/tests/query/subset-dedupe.test.ts index 4ad426b26b..b76ba2963c 100644 --- a/packages/db/tests/query/subset-dedupe.test.ts +++ b/packages/db/tests/query/subset-dedupe.test.ts @@ -1412,5 +1412,38 @@ describe(`createDeduplicatedLoadSubset`, () => { expect(loadSubset).toHaveBeenCalledTimes(2) }) + + it(`does not let Date mutation change a stored cursor boundary`, async () => { + const loadSubset = vi.fn().mockResolvedValue(undefined) + const deduplicated = new DeduplicatedLoadSubset({ loadSubset }) + const mutableBoundary = new Date(`2025-01-01T00:00:00.000Z`) + + await deduplicated.loadSubset({ + cursor: { + whereFrom: gt(ref(`createdAt`), val(mutableBoundary)), + whereCurrent: eq(ref(`createdAt`), val(mutableBoundary)), + lastKey: 1, + }, + limit: 10, + }) + mutableBoundary.setUTCFullYear(2026) + + await deduplicated.loadSubset({ + cursor: { + whereFrom: gt( + ref(`createdAt`), + val(new Date(`2026-01-01T00:00:00.000Z`)), + ), + whereCurrent: eq( + ref(`createdAt`), + val(new Date(`2026-01-01T00:00:00.000Z`)), + ), + lastKey: 1, + }, + limit: 10, + }) + + expect(loadSubset).toHaveBeenCalledTimes(2) + }) }) }) From 66a67ba78a420fe73c86a4f96a49aa8764bcff9b Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 25 Aug 2026 07:32:52 -0600 Subject: [PATCH 10/14] test(electric): type deferred helper --- packages/electric-db-collection/tests/electric.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/electric-db-collection/tests/electric.test.ts b/packages/electric-db-collection/tests/electric.test.ts index 56c518860f..39cd3a8bfd 100644 --- a/packages/electric-db-collection/tests/electric.test.ts +++ b/packages/electric-db-collection/tests/electric.test.ts @@ -23,7 +23,10 @@ import type { StandardSchemaV1 } from '@standard-schema/spec' const NativeAbortController = globalThis.AbortController -function createDeferred() { +function createDeferred(): { + promise: Promise + resolve: (value: T | PromiseLike) => void +} { let resolve!: (value: T | PromiseLike) => void const promise = new Promise((resolvePromise) => { resolve = resolvePromise From 16ad1c3ccf8c164ab18d33d8f5f8885259954841 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 25 Aug 2026 09:08:27 -0600 Subject: [PATCH 11/14] fix(db): preserve observable query identity --- packages/db/src/query/ir-stable-identity.ts | 11 ++-- .../db/tests/query/ir-stable-identity.test.ts | 55 ++++++++++++++++++- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/packages/db/src/query/ir-stable-identity.ts b/packages/db/src/query/ir-stable-identity.ts index d7fe07e306..1c07d0b748 100644 --- a/packages/db/src/query/ir-stable-identity.ts +++ b/packages/db/src/query/ir-stable-identity.ts @@ -270,7 +270,9 @@ function canonicalizeQueryInScope( if ( !query.select && - (Boolean(query.join?.length) || Boolean(query.groupBy?.length)) + (query.from.type === `unionFrom` || + query.join !== undefined || + query.groupBy !== undefined) ) { // Without an explicit projection, these query shapes return a namespaced // row. Its alias keys are public output and therefore part of identity. @@ -799,9 +801,10 @@ function canonicalizeFunction( ) const unique = sortUniqueStableIdentityValues(flattened) - return unique.length === 1 - ? unique[0]! - : { type: `func`, name, args: unique } + // The evaluator gives `and` and `or` boolean results even when their sole + // operand returns another truthy or falsy value. Keep that coercion in the + // identity because expression result types are erased at runtime. + return { type: `func`, name, args: unique } } if (name === `eq` && args.length === 2) { diff --git a/packages/db/tests/query/ir-stable-identity.test.ts b/packages/db/tests/query/ir-stable-identity.test.ts index 5a85d9ec48..ca745d9de5 100644 --- a/packages/db/tests/query/ir-stable-identity.test.ts +++ b/packages/db/tests/query/ir-stable-identity.test.ts @@ -45,7 +45,10 @@ import { UnionAll, Value, } from '../../src/query/ir.js' -import { compileExpression } from '../../src/query/compiler/evaluators.js' +import { + compileExpression, + toBooleanPredicate, +} from '../../src/query/compiler/evaluators.js' import { isLoadSubsetRequestSubsumedBy } from '../../src/query/predicate-utils.js' import { createRuntimeReferenceIdentityFactory } from '../../src/query/runtime-reference-identity.js' import type { BasicExpression, QueryIR } from '../../src/query/ir.js' @@ -231,7 +234,19 @@ describe(`semantic expression identity`, () => { expect(getStableExpressionHash(nested)).toBe(getStableExpressionHash(flat)) expect(getStableExpressionHash(new Func(`or`, [adult, adult]))).toBe( - getStableExpressionHash(adult), + getStableExpressionHash(new Func(`or`, [adult])), + ) + }) + + it(`keeps a boolean wrapper when duplicate operands coerce their result`, () => { + const bareAge = new PropRef([`user`, `age`]) + const duplicateAnd = new Func(`and`, [bareAge, bareAge]) + const row = { user: { age: 18 } } + + expect(toBooleanPredicate(compileExpression(bareAge)(row))).toBe(false) + expect(toBooleanPredicate(compileExpression(duplicateAnd)(row))).toBe(true) + expect(getStableExpressionHash(duplicateAnd)).not.toBe( + getStableExpressionHash(bareAge), ) }) @@ -1058,6 +1073,42 @@ describe(`stable QueryIR identity smoke test`, () => { ) }) + it(`keeps aliases that define an implicit union-source result shape`, () => { + const usersAndPosts = getQueryIR( + new Query().unionAll({ + user: usersCollection, + post: postsCollection, + }), + ) + const accountsAndArticles = getQueryIR( + new Query().unionAll({ + account: usersCollection, + article: postsCollection, + }), + ) + + expect(usersAndPosts.from.type).toBe(`unionFrom`) + expect(getQueryIdentity(usersAndPosts)).not.toBe( + getQueryIdentity(accountsAndArticles), + ) + }) + + it(`keeps aliases when an empty groupBy still selects a namespaced row`, () => { + const createQuery = (alias: string) => + getQueryIR( + new Query() + .from({ [alias]: usersCollection } as Record< + string, + typeof usersCollection + >) + .groupBy(() => []), + ) + + expect(getQueryIdentity(createQuery(`user`))).not.toBe( + getQueryIdentity(createQuery(`account`)), + ) + }) + it(`keeps output-producing runtime values exact`, () => { const bufferExpression = new Func(`concat`, [new Value(Buffer.from([65]))]) const uint8Expression = new Func(`concat`, [ From 751e3f71887fbc3835d5ced655a469e09a3649e8 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 25 Aug 2026 09:13:51 -0600 Subject: [PATCH 12/14] fix(db): close applied settlement gaps --- .../src/persisted.ts | 56 +++--- .../tests/persisted.test.ts | 71 +++++++- packages/db/src/collection/state.ts | 18 ++ .../query/load-subset-oracle.property.test.ts | 60 +++++++ .../electric-db-collection/src/electric.ts | 20 ++- .../tests/electric.test.ts | 31 ++++ packages/query-db-collection/src/query.ts | 163 +++++++++++++----- .../query-db-collection/tests/query.test.ts | 122 +++++++++++++ packages/rxdb-db-collection/src/rxdb.ts | 12 +- .../rxdb-db-collection/tests/rxdb.test.ts | 59 +++++++ .../trailbase-db-collection/src/trailbase.ts | 8 +- .../tests/trailbase.test.ts | 60 ++++++- 12 files changed, 584 insertions(+), 96 deletions(-) diff --git a/packages/db-sqlite-persistence-core/src/persisted.ts b/packages/db-sqlite-persistence-core/src/persisted.ts index 6f7c563d87..e158472ef9 100644 --- a/packages/db-sqlite-persistence-core/src/persisted.ts +++ b/packages/db-sqlite-persistence-core/src/persisted.ts @@ -589,6 +589,7 @@ type BufferedSyncTransaction = { internal: boolean signal?: AbortSignal resolveApplied?: () => void + rejectApplied?: (error: unknown) => void } type OpenSyncTransaction< @@ -1347,13 +1348,22 @@ class PersistedCollectionRuntime< if (!transaction) { continue } - await this.applyBufferedSyncTransactionUnsafe(transaction) + try { + await this.applyBufferedSyncTransactionUnsafe(transaction) + } catch (error) { + transaction.rejectApplied?.(error) + for (const abandoned of this.queuedHydrationTransactions) { + abandoned.rejectApplied?.(error) + } + this.queuedHydrationTransactions.length = 0 + throw error + } } } - private applyBufferedSyncTransactionUnsafe( + private async applyBufferedSyncTransactionUnsafe( transaction: BufferedSyncTransaction, - ): void { + ): Promise { if (transaction.signal?.aborted) { transaction.resolveApplied?.() return @@ -1403,34 +1413,23 @@ class PersistedCollectionRuntime< } } - const applied = commit(transaction.signal) - if (applied === true) { - transaction.resolveApplied?.() - } else { - void applied.then(() => transaction.resolveApplied?.()) - } - return applied + return commit(transaction.signal) } try { - if (transaction.internal) { - this.withInternalApply(applyToCollection) - return + const applied = transaction.internal + ? this.withInternalApply(applyToCollection) + : applyToCollection() + if (applied !== true) { + await applied } - const applied = applyToCollection() - const persistAfterApplication = async () => { - if (applied !== true) await applied - if (transaction.signal?.aborted) return - await this.persistAndBroadcastExternalSyncTransaction(transaction) + if (!transaction.internal && !transaction.signal?.aborted) { + await this.persistAndBroadcastExternalSyncTransactionUnsafe(transaction) } - void persistAfterApplication().catch((error) => { - console.warn(`Failed to persist buffered sync transaction:`, error) - }) - } catch (error) { - // A replay failure before commit has no core receipt that cleanup can - // settle. Release the wrapper receipt so the source load cannot hang. transaction.resolveApplied?.() + } catch (error) { + transaction.rejectApplied?.(error) throw error } } @@ -2525,8 +2524,10 @@ function createWrappedSyncConfig< if (openTransaction.queuedBecauseHydrating) { if (signal?.aborted) return true let resolveApplied!: () => void - const applied = new Promise((resolve) => { + let rejectApplied!: (error: unknown) => void + const applied = new Promise((resolve, reject) => { resolveApplied = resolve + rejectApplied = reject }) runtime.queueHydrationBufferedTransaction({ operations: openTransaction.operations, @@ -2537,6 +2538,7 @@ function createWrappedSyncConfig< internal: openTransaction.internal, signal, resolveApplied, + rejectApplied, }) return applied } @@ -2555,9 +2557,7 @@ function createWrappedSyncConfig< internal: false, }) } - void persistAfterApplication().catch((error) => { - console.warn(`Failed to persist wrapped sync transaction:`, error) - }) + return persistAfterApplication() } return applied }, diff --git a/packages/db-sqlite-persistence-core/tests/persisted.test.ts b/packages/db-sqlite-persistence-core/tests/persisted.test.ts index 8c00cff939..d1b34d758b 100644 --- a/packages/db-sqlite-persistence-core/tests/persisted.test.ts +++ b/packages/db-sqlite-persistence-core/tests/persisted.test.ts @@ -865,6 +865,50 @@ describe(`persistedCollectionOptions`, () => { } }) + it(`rejects a wrapped sync receipt when persistence fails`, async () => { + const adapter = createRecordingAdapter() + const persistenceError = new Error(`persistence failed`) + adapter.applyCommittedTx = () => Promise.reject(persistenceError) + let remoteBegin: (() => void) | undefined + let remoteWrite: + | ((message: { type: `insert`; value: Todo }) => void) + | undefined + let remoteCommit: (() => true | Promise) | undefined + const collection = createCollection( + persistedCollectionOptions({ + id: `sync-present-persistence-error`, + getKey: (item) => item.id, + sync: { + sync: ({ begin, write, commit, markReady }) => { + remoteBegin = begin + remoteWrite = write as (message: { + type: `insert` + value: Todo + }) => void + remoteCommit = commit + markReady() + }, + }, + persistence: { adapter }, + }), + ) + + try { + await collection.stateWhenReady() + remoteBegin?.() + remoteWrite?.({ + type: `insert`, + value: { id: `failed`, title: `Not durable` }, + }) + + await expect(Promise.resolve(remoteCommit?.())).rejects.toBe( + persistenceError, + ) + } finally { + await collection.cleanup() + } + }) + it(`preserves row metadata set before a metadata-less insert in the same sync transaction`, async () => { const adapter = createRecordingAdapter() const ownership = { queryCollection: { owners: [`gc:q1`] } } @@ -1207,7 +1251,7 @@ describe(`persistedCollectionOptions`, () => { } }) - it(`settles a hydration-buffered receipt when replay fails`, async () => { + it(`rejects every hydration-buffered receipt when replay fails`, async () => { const adapter = createRecordingAdapter() let resolveLoadSubset: (() => void) | undefined adapter.loadSubset = async () => { @@ -1265,19 +1309,28 @@ describe(`persistedCollectionOptions`, () => { type: `insert`, value: { id: `during-hydrate`, title: `During hydrate` }, }) - const receipt = remoteCommit?.() - expect(receipt).toBeInstanceOf(Promise) - let receiptSettled = false - void Promise.resolve(receipt).then(() => { - receiptSettled = true + const failingReceipt = remoteCommit?.() + remoteBegin?.() + remoteWrite?.({ + type: `insert`, + value: { id: `sibling`, title: `Sibling` }, }) + const siblingReceipt = remoteCommit?.() + expect(failingReceipt).toBeInstanceOf(Promise) + expect(siblingReceipt).toBeInstanceOf(Promise) + const failingExpectation = expect( + Promise.resolve(failingReceipt), + ).rejects.toBe(replayError) + const siblingExpectation = expect( + Promise.resolve(siblingReceipt), + ).rejects.toBe(replayError) resolveLoadSubset?.() await readyPromise - await collection.cleanup() - await flushAsyncWork() + await failingExpectation + await siblingExpectation - expect(receiptSettled).toBe(true) + await collection.cleanup() }) it(`marks ready even when persisted startup fails before markReady`, async () => { diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index d5cb986a9c..951040acc5 100644 --- a/packages/db/src/collection/state.ts +++ b/packages/db/src/collection/state.ts @@ -1380,10 +1380,28 @@ export class CollectionStateManager< this.pendingSyncedTransactions.splice(index, 1) transaction.applied.resolve() + const remainingPendingKeys = new Set() + for (const pending of this.pendingSyncedTransactions) { + for (const operation of pending.operations) { + remainingPendingKeys.add(operation.key as TKey) + } + } + for (const operation of transaction.operations) { + const key = operation.key as TKey + if (!remainingPendingKeys.has(key)) { + this.recentlySyncedKeys.delete(key) + this.preSyncVisibleState.delete(key) + } + } + if (this.pendingSyncedTransactions.length === 0) { this.preSyncVisibleState.clear() this.recentlySyncedKeys.clear() this.changes.emitEvents([], true) + } else { + // Recompute after removing the canceled keys so optimistic cleanup is + // no longer suppressed by a sync transaction that will never publish. + this.recomputeOptimisticState(false) } } diff --git a/packages/db/tests/query/load-subset-oracle.property.test.ts b/packages/db/tests/query/load-subset-oracle.property.test.ts index 21d559978d..3c56f26047 100644 --- a/packages/db/tests/query/load-subset-oracle.property.test.ts +++ b/packages/db/tests/query/load-subset-oracle.property.test.ts @@ -1599,6 +1599,62 @@ async function expectAbortedReceiptDoesNotPublishCoverage( } } +async function expectCanceledReceiptReleasesOnlyItsSuppression() { + let begin!: () => void + let write!: (message: { type: `update`; value: PersistedLoadRow }) => void + let commit!: (signal?: AbortSignal) => SyncAppliedReceipt + const source = createCollection({ + id: `load-subset-cancel-suppression-${collectionSequence++}`, + getKey: (row) => row.id, + sync: { + sync: (params) => { + begin = params.begin + write = params.write + commit = params.commit + begin() + write({ type: `update`, value: { id: `first`, projectId: `old` } }) + write({ type: `update`, value: { id: `second`, projectId: `old` } }) + commit() + params.markReady() + }, + }, + }) + await source.preload() + await Promise.resolve() + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `local`, projectId: `pending` })) + expect(transaction.state).toBe(`persisting`) + try { + begin() + write({ type: `update`, value: { id: `first`, projectId: `new` } }) + const canceled = commit() + const canceledTransaction = source._state.pendingSyncedTransactions.at(-1)! + expect(source._state.pendingSyncedTransactions).toHaveLength(1) + begin() + write({ type: `update`, value: { id: `second`, projectId: `new` } }) + expect(source._state.pendingSyncedTransactions).toHaveLength(2) + + source._state.capturePreSyncVisibleState() + expect(source._state.recentlySyncedKeys).toEqual( + new Set([`first`, `second`]), + ) + + source._state.cancelPendingSyncedTransaction(canceledTransaction) + expect(source._state.pendingSyncedTransactions).toHaveLength(1) + expect(source._state.recentlySyncedKeys).toEqual(new Set([`second`])) + expect(source._state.preSyncVisibleState.has(`first`)).toBe(false) + expect(source._state.preSyncVisibleState.has(`second`)).toBe(true) + if (canceled !== true) await canceled + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await source.cleanup() + } +} + async function expectCleanupSettlesReceiptOnce() { let receipt!: Promise let transportCalls = 0 @@ -2590,6 +2646,10 @@ describe(`loadSubset coverage oracle`, () => { expectAbortedReceiptDoesNotPublishCoverage, ) + it(`releases only a canceled receipt's event suppression`, async () => { + await expectCanceledReceiptReleasesOnlyItsSuppression() + }) + it(`settles an abandoned receipt once without publishing coverage`, async () => { await expectCleanupSettlesReceiptOnce() }) diff --git a/packages/electric-db-collection/src/electric.ts b/packages/electric-db-collection/src/electric.ts index fbc62a8f23..6809eb6a3a 100644 --- a/packages/electric-db-collection/src/electric.ts +++ b/packages/electric-db-collection/src/electric.ts @@ -1544,7 +1544,13 @@ function createElectricSync>( // Wrap markReady to wait for test hook in progressive mode let progressiveReadyGate: Promise | null = null - const wrappedMarkReady = (isBuffering: boolean) => { + let streamErrorVersion = 0 + const wrappedMarkReady = ( + isBuffering: boolean, + expectedErrorVersion = streamErrorVersion, + ) => { + if (streamErrorVersion !== expectedErrorVersion) return + // Only create gate if we're in buffering phase (first up-to-date) if ( isBuffering && @@ -1554,7 +1560,9 @@ function createElectricSync>( // Create a new gate promise for this sync cycle progressiveReadyGate = testHooks.beforeMarkingReady() progressiveReadyGate.then(() => { - markReady() + if (streamErrorVersion === expectedErrorVersion) { + markReady() + } }) } else { // No hook, not buffering, or already past first up-to-date @@ -1609,6 +1617,7 @@ function createElectricSync>( (canUsePersistedResume ? persistedResumeState.handle : undefined), signal: abortController.signal, onError: (errorParams) => { + streamErrorVersion++ // Note that Electric sends a 409 error on a `must-refetch` message, but the // ShapeStream handled this and it will not reach this handler, therefor // this handler will not run for a `must-refetch`. @@ -1998,10 +2007,13 @@ function createElectricSync>( applied = commit() } } + const readyErrorVersion = streamErrorVersion if (applied === true) { - wrappedMarkReady(wasBufferingInitialSync) + wrappedMarkReady(wasBufferingInitialSync, readyErrorVersion) } else { - void applied.then(() => wrappedMarkReady(wasBufferingInitialSync)) + void applied.then(() => + wrappedMarkReady(wasBufferingInitialSync, readyErrorVersion), + ) } // Track that we've received the first up-to-date for progressive mode diff --git a/packages/electric-db-collection/tests/electric.test.ts b/packages/electric-db-collection/tests/electric.test.ts index 39cd3a8bfd..c913f8d973 100644 --- a/packages/electric-db-collection/tests/electric.test.ts +++ b/packages/electric-db-collection/tests/electric.test.ts @@ -221,6 +221,37 @@ describe(`Electric Integration`, () => { } }) + it(`does not let a parked ready receipt overwrite a later stream error`, async () => { + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + const streamError = new Error(`stream failed`) + const loggedError = vi.spyOn(console, `error`).mockImplementation(() => {}) + + try { + transaction.mutate(() => collection.insert({ id: 99, name: `Local row` })) + subscriber([{ headers: { control: `up-to-date` } }]) + expect(collection.status).toBe(`loading`) + + const streamOptions = vi.mocked(ShapeStream).mock.calls.at(-1)?.[0] as + | { onError?: (error: unknown) => void } + | undefined + streamOptions?.onError?.(streamError) + expect(collection.status).toBe(`error`) + + persistence.resolve() + await transaction.isPersisted.promise + await Promise.resolve() + + expect(collection.status).toBe(`error`) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + loggedError.mockRestore() + } + }) + it(`should handle incoming insert messages and commit on up-to-date`, () => { // Simulate incoming insert message subscriber([ diff --git a/packages/query-db-collection/src/query.ts b/packages/query-db-collection/src/query.ts index f27a247306..b9362b3497 100644 --- a/packages/query-db-collection/src/query.ts +++ b/packages/query-db-collection/src/query.ts @@ -889,7 +889,9 @@ export function queryCollectionOptions( let startupRetentionSettled = false const retainedQueriesPendingRevalidation = new Set() const pendingResultApplications = new Map>() + const failedResultApplications = new Map() const resultApplicationTokens = new Map() + const resultApplicationControllers = new Map>() const effectivePersistedGcTimes = new Map() const persistedRetentionTimers = new Map< string, @@ -899,7 +901,25 @@ export function queryCollectionOptions( const invalidatePendingResultApplication = (hashedQueryKey: string) => { pendingResultApplications.delete(hashedQueryKey) + failedResultApplications.delete(hashedQueryKey) resultApplicationTokens.delete(hashedQueryKey) + resultApplicationControllers + .get(hashedQueryKey) + ?.forEach((controller) => controller.abort()) + resultApplicationControllers.delete(hashedQueryKey) + } + + const getResultApplicationSettlement = ( + hashedQueryKey: string, + ): true | Promise => { + const pending = pendingResultApplications.get(hashedQueryKey) + if (pending) return pending + + if (failedResultApplications.has(hashedQueryKey)) { + return Promise.reject(failedResultApplications.get(hashedQueryKey)) + } + + return true } const getRowMetadata = (rowKey: string | number) => { @@ -1251,7 +1271,10 @@ export function queryCollectionOptions( const unsubscribe = observer.subscribe((result) => { // Use a microtask in case `subscribe` is called synchronously, before `unsubscribe` is initialized queueMicrotask(() => { - if (result.isSuccess || result.isError) { + if ( + (result.isSuccess && !collection.deferDataRefresh) || + result.isError + ) { unsubscribe() const pending = pendingReadyUnsubscribes.get(hashedQueryKey) pending?.delete(unsubscribe) @@ -1317,14 +1340,21 @@ export function queryCollectionOptions( const currentResult = observer.getCurrentResult() if (currentResult.isSuccess) { - return pendingResultApplications.get(hashedQueryKey) ?? true + if (collection.deferDataRefresh) { + return waitForQueryReady(observer, hashedQueryKey).then(() => { + const settlement = getResultApplicationSettlement(hashedQueryKey) + return settlement === true ? undefined : settlement + }) + } + return getResultApplicationSettlement(hashedQueryKey) } else if (currentResult.isError) { // Error already occurred, reject immediately return Promise.reject(currentResult.error) } else { - return waitForQueryReady(observer, hashedQueryKey).then(() => - pendingResultApplications.get(hashedQueryKey), - ) + return waitForQueryReady(observer, hashedQueryKey).then(() => { + const settlement = getResultApplicationSettlement(hashedQueryKey) + return settlement === true ? undefined : settlement + }) } } @@ -1391,7 +1421,13 @@ export function queryCollectionOptions( if (syncStarted || collection.subscriberCount > 0) { subscribeToQuery(localObserver, hashedQueryKey) } - return pendingResultApplications.get(hashedQueryKey) ?? true + if (collection.deferDataRefresh) { + return waitForQueryReady(localObserver, hashedQueryKey).then(() => { + const settlement = getResultApplicationSettlement(hashedQueryKey) + return settlement === true ? undefined : settlement + }) + } + return getResultApplicationSettlement(hashedQueryKey) } // Create a promise that resolves when the query result is first available @@ -1403,9 +1439,10 @@ export function queryCollectionOptions( subscribeToQuery(localObserver, hashedQueryKey) } - return readyPromise.then(() => - pendingResultApplications.get(hashedQueryKey), - ) + return readyPromise.then(() => { + const settlement = getResultApplicationSettlement(hashedQueryKey) + return settlement === true ? undefined : settlement + }) } type UpdateHandler = Parameters[0] @@ -1420,10 +1457,11 @@ export function queryCollectionOptions( owners: Set } >, + signal?: AbortSignal, ): Promise => { const hashedQueryKey = hashKey(queryKey) - if (collection.status === `cleaned-up`) { + if (collection.status === `cleaned-up` || signal?.aborted) { return } @@ -1473,6 +1511,23 @@ export function queryCollectionOptions( }) let transactionActive = false + const restoreOwnershipTracking = () => { + if (!state.observers.has(hashedQueryKey)) return + + if (previousOwnedRows === undefined) { + queryToRows.delete(hashedQueryKey) + } else { + queryToRows.set(hashedQueryKey, previousOwnedRows) + } + previousOwnersByRow.forEach((owners, key) => { + if (owners === undefined) { + rowToQueries.delete(key) + } else { + rowToQueries.set(key, owners) + } + }) + } + try { // From this point onward the result, including an empty result, is the // authoritative ownership baseline until this query is cleaned up. @@ -1522,7 +1577,7 @@ export function queryCollectionOptions( } }) - const applied = commit() + const applied = commit(signal) transactionActive = false retainedQueriesPendingRevalidation.delete(hashedQueryKey) cancelPersistedRetentionExpiry(hashedQueryKey) @@ -1532,20 +1587,13 @@ export function queryCollectionOptions( if (applied !== true) { await applied } + if (signal?.aborted) { + restoreOwnershipTracking() + return + } markReady() } catch (error) { - if (previousOwnedRows === undefined) { - queryToRows.delete(hashedQueryKey) - } else { - queryToRows.set(hashedQueryKey, previousOwnedRows) - } - previousOwnersByRow.forEach((owners, key) => { - if (owners === undefined) { - rowToQueries.delete(key) - } else { - rowToQueries.set(key, owners) - } - }) + restoreOwnershipTracking() if (transactionActive) { const cancellation = new AbortController() @@ -1564,6 +1612,7 @@ export function queryCollectionOptions( queryKey: QueryKey, result: QueryObserverResult, applicationToken: object, + signal: AbortSignal, ) => { const hashedQueryKey = hashKey(queryKey) const persistedBaseline = @@ -1574,7 +1623,7 @@ export function queryCollectionOptions( ) { return } - await applySuccessfulResult(queryKey, result, persistedBaseline) + await applySuccessfulResult(queryKey, result, persistedBaseline, signal) } const trackResultApplication = ( @@ -1585,31 +1634,52 @@ export function queryCollectionOptions( const finish = () => { if (pendingResultApplications.get(hashedQueryKey) === application) { pendingResultApplications.delete(hashedQueryKey) + return true } + return false } - void application.then(finish, (error) => { - finish() - state.lastError = error - state.errorCount++ - state.lastErrorUpdatedAt = Date.now() - console.error( - `[QueryCollection] Error applying query ${String(hashToQueryKey.get(hashedQueryKey))}:`, - error, - ) - if (collection.status === `loading`) { - markError(error) - } - }) + void application.then( + () => { + if (finish()) failedResultApplications.delete(hashedQueryKey) + }, + (error) => { + if (!finish()) return + failedResultApplications.set(hashedQueryKey, error) + state.lastError = error + state.errorCount++ + state.lastErrorUpdatedAt = Date.now() + console.error( + `[QueryCollection] Error applying query ${String(hashToQueryKey.get(hashedQueryKey))}:`, + error, + ) + if (collection.status === `loading`) { + markError(error) + } + }, + ) } const enqueueResultApplication = ( hashedQueryKey: string, - apply: () => Promise, + apply: (signal: AbortSignal) => Promise, ): void => { + const controller = new AbortController() + const controllers = + resultApplicationControllers.get(hashedQueryKey) ?? new Set() + controllers.add(controller) + resultApplicationControllers.set(hashedQueryKey, controllers) const previousApplication = pendingResultApplications.get(hashedQueryKey) + const run = () => apply(controller.signal) const application = previousApplication - ? previousApplication.then(apply, apply) - : apply() + ? previousApplication.then(run, run) + : run() + const cleanupController = () => { + controllers.delete(controller) + if (controllers.size === 0) { + resultApplicationControllers.delete(hashedQueryKey) + } + } + void application.then(cleanupController, cleanupController) trackResultApplication(hashedQueryKey, application) } @@ -1659,12 +1729,17 @@ export function queryCollectionOptions( const applicationToken = {} resultApplicationTokens.set(hashedQueryKey, applicationToken) - enqueueResultApplication(hashedQueryKey, () => - reconcileSuccessfulResult(queryKey, result, applicationToken), + enqueueResultApplication(hashedQueryKey, (signal) => + reconcileSuccessfulResult( + queryKey, + result, + applicationToken, + signal, + ), ) } else { - enqueueResultApplication(hashedQueryKey, () => - applySuccessfulResult(queryKey, result), + enqueueResultApplication(hashedQueryKey, (signal) => + applySuccessfulResult(queryKey, result, undefined, signal), ) } } else if (result.isError) { diff --git a/packages/query-db-collection/tests/query.test.ts b/packages/query-db-collection/tests/query.test.ts index 1d4ea34853..b6f8266813 100644 --- a/packages/query-db-collection/tests/query.test.ts +++ b/packages/query-db-collection/tests/query.test.ts @@ -719,6 +719,93 @@ describe(`QueryCollection`, () => { } }) + it(`does not publish queued query results after the subset is released`, async () => { + const queryKey = [`released-result-application`] + const queryResult = createDeferred>() + const collection = createCollection( + queryCollectionOptions({ + id: `released-result-application`, + queryClient, + queryKey, + queryFn: () => queryResult.promise, + getKey, + syncMode: `on-demand`, + startSync: true, + }), + ) + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + + try { + transaction.mutate(() => + collection.insert({ id: `local`, name: `Local` }), + ) + collection._sync.loadSubset({}) + queryResult.resolve([{ id: `first`, name: `First` }]) + await flushPromises() + + queryClient.setQueryData(queryKey, [{ id: `second`, name: `Second` }]) + await flushPromises() + collection._sync.unloadSubset({}) + + persistence.resolve() + await transaction.isPersisted.promise + await flushPromises() + + expect(collection.has(`first`)).toBe(false) + expect(collection.has(`second`)).toBe(false) + } finally { + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await collection.cleanup() + } + }) + + it(`keeps a deferred successful result pending until its refetch applies`, async () => { + const barrier = createDeferred() + const queryFn = vi + .fn() + .mockResolvedValue([{ id: `server`, name: `Server` }]) + const collection = createCollection( + queryCollectionOptions({ + id: `deferred-result-application`, + queryClient, + queryKey: [`deferred-result-application`], + queryFn, + getKey, + syncMode: `on-demand`, + startSync: true, + }), + ) + collection.deferDataRefresh = barrier.promise + + try { + const load = collection._sync.loadSubset({}) + let settled = false + void Promise.resolve(load).then(() => { + settled = true + }) + await vi.waitFor(() => expect(queryFn).toHaveBeenCalledOnce()) + await flushPromises() + + expect(settled).toBe(false) + expect(collection.has(`server`)).toBe(false) + + collection.deferDataRefresh = null + barrier.resolve() + if (load !== true) await load + + expect(queryFn).toHaveBeenCalledTimes(2) + expect(collection.has(`server`)).toBe(true) + } finally { + collection.deferDataRefresh = null + barrier.resolve() + await collection.cleanup() + } + }) + it(`applies successive eager results in publication order`, async () => { const queryKey = [`eager-result-publication-order`] const collection = createCollection( @@ -4661,6 +4748,41 @@ describe(`QueryCollection`, () => { }, ) + it(`does not treat a failed application as established coverage`, async () => { + const applicationError = new Error(`application failed`) + const consoleErrorSpy = vi + .spyOn(console, `error`) + .mockImplementation(() => {}) + const demand = { where: eq(`id`, `1`) } + const collection = createCollection( + queryCollectionOptions({ + id: `failed-application-coverage`, + queryClient, + queryKey: [`failed-application-coverage`], + queryFn: vi.fn().mockResolvedValue([{ id: `1`, name: `Item 1` }]), + getKey: () => { + throw applicationError + }, + syncMode: `on-demand`, + startSync: true, + retry: false, + }), + ) + + try { + const firstLoad = collection._sync.loadSubset(demand) + await expect(Promise.resolve(firstLoad)).rejects.toBe(applicationError) + + const repeatedLoad = collection._sync.loadSubset(demand) + await expect(Promise.resolve(repeatedLoad)).rejects.toBe( + applicationError, + ) + } finally { + await collection.cleanup() + consoleErrorSpy.mockRestore() + } + }) + it(`should track error state, count, and support recovery`, async () => { const initialData = [{ id: `1`, name: `Item 1` }] const updatedData = [{ id: `1`, name: `Updated Item 1` }] diff --git a/packages/rxdb-db-collection/src/rxdb.ts b/packages/rxdb-db-collection/src/rxdb.ts index 4a0e44d551..bd8d1bd89d 100644 --- a/packages/rxdb-db-collection/src/rxdb.ts +++ b/packages/rxdb-db-collection/src/rxdb.ts @@ -258,10 +258,13 @@ export function rxdbCollectionOptions( return } - // Keep buffering until every older startup batch is applied. Events - // that arrive while one receipt is parked join the next FIFO batch. - while (buffer.length) { - const pending = buffer.splice(0) + // Take one finite snapshot of changes observed during the initial + // fetch, then route newer events through the normal live path. The + // core transaction queue preserves their order without letting a + // continuous event stream postpone readiness forever. + const pending = buffer.splice(0) + buffering = false + if (pending.length > 0) { begin() for (const msg of pending) write(msg) await commit() @@ -269,7 +272,6 @@ export function rxdbCollectionOptions( return } } - buffering = false if (!isCleanedUp()) { markReady() diff --git a/packages/rxdb-db-collection/tests/rxdb.test.ts b/packages/rxdb-db-collection/tests/rxdb.test.ts index 02c63aa1d3..a8dfbe82dd 100644 --- a/packages/rxdb-db-collection/tests/rxdb.test.ts +++ b/packages/rxdb-db-collection/tests/rxdb.test.ts @@ -214,6 +214,65 @@ describe(`RxDB Integration`, () => { } }) + it(`does not let later live traffic extend the startup readiness boundary`, async () => { + const db = await getDatababase() + const rxCollection: RxCollection = db.test + const initialQueryStarted = createDeferred() + const releaseInitialQuery = createDeferred() + const bufferedApplied = createDeferred() + const laterLiveApplied = createDeferred() + const storageQuery = rxCollection.storageInstance.query.bind( + rxCollection.storageInstance, + ) + const query = vi + .spyOn(rxCollection.storageInstance, `query`) + .mockImplementationOnce(async (preparedQuery) => { + const result = await storageQuery(preparedQuery) + initialQueryStarted.resolve() + await releaseInitialQuery.promise + return result + }) + const options = rxdbCollectionOptions({ rxCollection }) + const begin = vi.fn() + const write = vi.fn() + const commit = vi + .fn() + .mockReturnValueOnce(true) + .mockReturnValueOnce(bufferedApplied.promise) + .mockReturnValueOnce(laterLiveApplied.promise) + const markReady = vi.fn() + const markError = vi.fn() + const cleanup = options.sync.sync({ + begin, + write, + commit, + markReady, + markError, + collection: { status: `loading` }, + } as never) + + try { + await initialQueryStarted.promise + await rxCollection.insert({ id: `buffered`, name: `Buffered` }) + releaseInitialQuery.resolve() + await vi.waitFor(() => expect(commit).toHaveBeenCalledTimes(2)) + + await rxCollection.insert({ id: `later`, name: `Later` }) + await vi.waitFor(() => expect(commit).toHaveBeenCalledTimes(3)) + expect(markReady).not.toHaveBeenCalled() + + bufferedApplied.resolve() + await vi.waitFor(() => expect(markReady).toHaveBeenCalledOnce()) + } finally { + releaseInitialQuery.resolve() + bufferedApplied.resolve() + laterLiveApplied.resolve() + if (typeof cleanup === `function`) cleanup() + query.mockRestore() + await db.remove() + } + }) + it(`should initialize and fetch initial data`, async () => { const initialItems = getTestData(2) diff --git a/packages/trailbase-db-collection/src/trailbase.ts b/packages/trailbase-db-collection/src/trailbase.ts index 7afeea7a16..631c594d0b 100644 --- a/packages/trailbase-db-collection/src/trailbase.ts +++ b/packages/trailbase-db-collection/src/trailbase.ts @@ -218,6 +218,7 @@ export function trailBaseCollectionOptions< if (remaining <= 0) { return } + const appliedPages: Array> = [] while (true) { const limit = Math.min(remaining, 256) @@ -253,7 +254,10 @@ export function trailBaseCollectionOptions< }) } - await commit(opts.signal) + const applied = commit(opts.signal) + if (applied !== true) { + appliedPages.push(applied) + } if (cancelled || opts.signal?.aborted) return remaining -= length @@ -276,6 +280,8 @@ export function trailBaseCollectionOptions< cursor = response.cursor } } + + await Promise.all(appliedPages) } // Afterwards subscribe. diff --git a/packages/trailbase-db-collection/tests/trailbase.test.ts b/packages/trailbase-db-collection/tests/trailbase.test.ts index c55189309e..7370ec1c12 100644 --- a/packages/trailbase-db-collection/tests/trailbase.test.ts +++ b/packages/trailbase-db-collection/tests/trailbase.test.ts @@ -279,11 +279,7 @@ describe(`TrailBase Integration`, () => { it(`does not publish a parked subset page after its request is aborted`, async () => { const recordApi = new MockRecordApi() recordApi.list.mockResolvedValue({ - records: Array.from({ length: 256 }, (_, index) => ({ - id: index + 1, - updated: 0, - data: `obsolete`, - })), + records: [{ id: 1, updated: 0, data: `obsolete` }], }) recordApi.subscribe.mockResolvedValue(new TransformStream().readable) const collection = createCollection( @@ -333,6 +329,60 @@ describe(`TrailBase Integration`, () => { } }) + it(`fetches later subset pages while earlier pages wait to apply`, async () => { + const recordApi = new MockRecordApi() + recordApi.list.mockImplementation(async () => { + const start = recordApi.list.mock.calls.length === 1 ? 1 : 257 + const count = start === 1 ? 256 : 1 + return { + records: Array.from({ length: count }, (_, index) => ({ + id: start + index, + updated: 0, + data: `remote`, + })), + cursor: `page-${start}`, + } + }) + recordApi.subscribe.mockResolvedValue(new TransformStream().readable) + const collection = createCollection( + trailBaseCollectionOptions({ + recordApi, + getKey: (item: Data) => item.id ?? -1, + startSync: true, + syncMode: `on-demand`, + parse: {}, + serialize: {}, + }), + ) + let resolvePersistence!: () => void + const persistence = new Promise((resolve) => { + resolvePersistence = resolve + }) + const transaction = createTransaction({ mutationFn: () => persistence }) + + try { + await vi.waitFor(() => expect(collection.status).toBe(`ready`)) + transaction.mutate(() => + collection.insert({ id: 999, updated: 0, data: `local` }), + ) + const load = collection._sync.loadSubset({ limit: 257 }) + + await vi.waitFor(() => expect(recordApi.list).toHaveBeenCalledTimes(2)) + expect(collection.get(1)).toBeUndefined() + + resolvePersistence() + await transaction.isPersisted.promise + if (load instanceof Promise) await load + + expect(collection.get(1)?.data).toBe(`remote`) + expect(collection.get(257)?.data).toBe(`remote`) + } finally { + resolvePersistence() + await transaction.isPersisted.promise.catch(() => undefined) + await collection.cleanup() + } + }) + it(`initial fetch, receive update and cancel`, async () => { const records: Array = [ { From fb2a4f05befc289c56acf4ac175e670ecedc4ad8 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 25 Aug 2026 10:17:14 -0600 Subject: [PATCH 13/14] fix(db): distinguish canceled sync receipts --- .changeset/settle-subset-after-publication.md | 6 +- docs/guides/mutations.md | 6 ++ .../src/persisted.ts | 24 +++-- .../tests/persisted.test.ts | 80 +++++++++++++++- .../db-core/mutations-optimistic/SKILL.md | 5 + packages/db/src/client.ts | 1 + packages/db/src/collection/state.ts | 19 +++- packages/db/src/collection/sync.ts | 15 ++- packages/db/src/errors.ts | 8 ++ packages/db/src/query/live/ARCHITECTURE.md | 18 +++- packages/db/src/types.ts | 19 +++- .../collection-subscribe-changes.test.ts | 1 + .../query/load-subset-oracle.property.test.ts | 92 ++++++++++++++++--- .../electric-db-collection/src/electric.ts | 9 +- .../tests/trailbase.test.ts | 4 +- 15 files changed, 262 insertions(+), 45 deletions(-) diff --git a/.changeset/settle-subset-after-publication.md b/.changeset/settle-subset-after-publication.md index 834be1fb4c..29e5040c81 100644 --- a/.changeset/settle-subset-after-publication.md +++ b/.changeset/settle-subset-after-publication.md @@ -8,4 +8,8 @@ '@tanstack/trailbase-db-collection': patch --- -Settle subset loads only after their committed rows and events are visible. Preserve causal publication, cancellation, and error handling across the affected sync adapters. +Settle subset loads only after their committed rows and events are visible. A +commit receipt now rejects with `AbortError` when cancellation wins before +application and ignores later aborts. Preserve causal publication, +cancellation, persistence, and error handling across the affected sync +adapters. diff --git a/docs/guides/mutations.md b/docs/guides/mutations.md index 7a269aa36c..2faf25b88a 100644 --- a/docs/guides/mutations.md +++ b/docs/guides/mutations.md @@ -432,6 +432,12 @@ const todoCollection = createCollection({ > [!IMPORTANT] > Operation handlers must not resolve until the server changes have synced back to the collection. Different collection types provide different patterns to ensure this happens correctly. +> +> Do not call or await `collection.preload()`, live-query `preload()`, or a +> direct `loadSubset()` inside a mutation handler. The optimistic mutation is +> already applied when the handler starts. A preload may need a sync commit +> that is queued behind that same handler, which creates a deadlock. Use the +> collection adapter's documented mutation acknowledgement pattern instead. ### Collection-Specific Handler Patterns diff --git a/packages/db-sqlite-persistence-core/src/persisted.ts b/packages/db-sqlite-persistence-core/src/persisted.ts index e158472ef9..14358dd3e9 100644 --- a/packages/db-sqlite-persistence-core/src/persisted.ts +++ b/packages/db-sqlite-persistence-core/src/persisted.ts @@ -1,4 +1,5 @@ import { + SyncTransactionAbortedError, compileSingleRowExpression, safeRandomUUID, toBooleanPredicate, @@ -855,7 +856,8 @@ class PersistedCollectionRuntime< return true } this.pendingAppliedReceipts.set(sequence, receipt) - void receipt.then(() => this.pendingAppliedReceipts.delete(sequence)) + const removeReceipt = () => this.pendingAppliedReceipts.delete(sequence) + void receipt.then(removeReceipt, removeReceipt) return receipt } @@ -1191,7 +1193,7 @@ class PersistedCollectionRuntime< this.pendingRemoteSubsetEnsures.clear() this.activeSubsets.clear() for (const transaction of this.queuedHydrationTransactions) { - transaction.resolveApplied?.() + transaction.rejectApplied?.(new SyncTransactionAbortedError()) } this.queuedHydrationTransactions.length = 0 this.queuedTxCommitted.length = 0 @@ -1365,13 +1367,13 @@ class PersistedCollectionRuntime< transaction: BufferedSyncTransaction, ): Promise { if (transaction.signal?.aborted) { - transaction.resolveApplied?.() + transaction.rejectApplied?.(new SyncTransactionAbortedError()) return } const { begin, write, commit, truncate, metadata } = this.syncControls if (!begin || !write || !commit) { - transaction.resolveApplied?.() + transaction.rejectApplied?.(new SyncTransactionAbortedError()) return } @@ -1424,7 +1426,7 @@ class PersistedCollectionRuntime< await applied } - if (!transaction.internal && !transaction.signal?.aborted) { + if (!transaction.internal) { await this.persistAndBroadcastExternalSyncTransactionUnsafe(transaction) } transaction.resolveApplied?.() @@ -2522,13 +2524,18 @@ function createWrappedSyncConfig< } if (openTransaction.queuedBecauseHydrating) { - if (signal?.aborted) return true + if (signal?.aborted) { + const aborted = Promise.reject(new SyncTransactionAbortedError()) + void aborted.catch(() => undefined) + return aborted + } let resolveApplied!: () => void let rejectApplied!: (error: unknown) => void const applied = new Promise((resolve, reject) => { resolveApplied = resolve rejectApplied = reject }) + void applied.catch(() => undefined) runtime.queueHydrationBufferedTransaction({ operations: openTransaction.operations, rowMetadataWrites: openTransaction.rowMetadataWrites, @@ -2547,7 +2554,6 @@ function createWrappedSyncConfig< if (!openTransaction.internal) { const persistAfterApplication = async () => { if (applied !== true) await applied - if (signal?.aborted) return await runtime.persistAndBroadcastExternalSyncTransaction({ operations: openTransaction.operations, rowMetadataWrites: openTransaction.rowMetadataWrites, @@ -2557,7 +2563,9 @@ function createWrappedSyncConfig< internal: false, }) } - return persistAfterApplication() + const persisted = persistAfterApplication() + void persisted.catch(() => undefined) + return persisted } return applied }, diff --git a/packages/db-sqlite-persistence-core/tests/persisted.test.ts b/packages/db-sqlite-persistence-core/tests/persisted.test.ts index d1b34d758b..606f0d75e7 100644 --- a/packages/db-sqlite-persistence-core/tests/persisted.test.ts +++ b/packages/db-sqlite-persistence-core/tests/persisted.test.ts @@ -855,7 +855,9 @@ describe(`persistedCollectionOptions`, () => { type: `insert`, value: { id: `aborted`, title: `Must not publish` }, }) - await remoteCommit?.(abortController.signal) + await expect( + remoteCommit?.(abortController.signal), + ).rejects.toMatchObject({ name: `AbortError` }) await flushAsyncWork() expect(collection.get(`aborted`)).toBeUndefined() @@ -865,6 +867,78 @@ describe(`persistedCollectionOptions`, () => { } }) + it(`persists a wrapped sync transaction when abort follows application`, async () => { + const adapter = createRecordingAdapter() + let remoteBegin: (() => void) | undefined + let remoteWrite: + | ((message: { type: `insert`; value: Todo }) => void) + | undefined + let remoteCommit: + | ((signal?: AbortSignal) => true | Promise) + | undefined + const collection = createCollection( + persistedCollectionOptions({ + id: `sync-present-abort-after-application`, + getKey: (item) => item.id, + sync: { + sync: ({ begin, write, commit, markReady }) => { + remoteBegin = begin + remoteWrite = write as (message: { + type: `insert` + value: Todo + }) => void + remoteCommit = commit + markReady() + }, + }, + persistence: { adapter }, + }), + ) + let releaseMutation!: () => void + const mutationGate = new Promise((resolve) => { + releaseMutation = resolve + }) + const transaction = createTransaction({ + mutationFn: () => mutationGate, + }) + + try { + await collection.stateWhenReady() + transaction.mutate(() => { + collection.insert({ id: `local`, title: `Optimistic gate` }) + }) + + const abortController = new AbortController() + const subscription = collection.subscribeChanges((changes) => { + if (changes.some((change) => change.key === `remote`)) { + abortController.abort() + } + }) + remoteBegin?.() + remoteWrite?.({ + type: `insert`, + value: { id: `remote`, title: `Already visible` }, + }) + const receipt = remoteCommit?.(abortController.signal) + expect(receipt).toBeInstanceOf(Promise) + + releaseMutation() + await transaction.isPersisted.promise + await receipt + subscription.unsubscribe() + + expect(stripVirtualProps(collection.get(`remote`))).toEqual({ + id: `remote`, + title: `Already visible`, + }) + expect(adapter.applyCommittedTxCalls).toHaveLength(1) + } finally { + releaseMutation() + await transaction.isPersisted.promise.catch(() => undefined) + await collection.cleanup() + } + }) + it(`rejects a wrapped sync receipt when persistence fails`, async () => { const adapter = createRecordingAdapter() const persistenceError = new Error(`persistence failed`) @@ -1240,7 +1314,9 @@ describe(`persistedCollectionOptions`, () => { abortController.abort() resolveLoadSubset?.() await ready - if (applied !== true) await applied + if (applied !== true) { + await expect(applied).rejects.toMatchObject({ name: `AbortError` }) + } await flushAsyncWork() expect(collection.get(`aborted`)).toBeUndefined() diff --git a/packages/db/skills/db-core/mutations-optimistic/SKILL.md b/packages/db/skills/db-core/mutations-optimistic/SKILL.md index c249ef8602..b3dda1471d 100644 --- a/packages/db/skills/db-core/mutations-optimistic/SKILL.md +++ b/packages/db/skills/db-core/mutations-optimistic/SKILL.md @@ -93,6 +93,11 @@ settlement or catch rollback errors. For a non-empty transaction, this normally means its `mutationFn` returned; it proves upload, confirmation, or read-back only when that function waits for the backend observation before returning. +Do not start or await collection preloads, live-query preloads, or direct +`loadSubset()` calls inside `mutationFn`. Sync commits queue behind mutation +persistence, so the preload can wait on the mutation that is waiting on it. +Use the collection adapter's documented mutation acknowledgement pattern. + --- ## Core Patterns diff --git a/packages/db/src/client.ts b/packages/db/src/client.ts index e954eb35e5..2aa4651bc7 100644 --- a/packages/db/src/client.ts +++ b/packages/db/src/client.ts @@ -867,6 +867,7 @@ export class DbClient { if (rows.length > 0) { collection._state.pendingSyncedTransactions.push({ committed: true, + applicationStarted: false, layoutChanged: false, operations: rows.map((row) => ({ type: collection._state.syncedData.has(row.key) diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index 951040acc5..3783422013 100644 --- a/packages/db/src/collection/state.ts +++ b/packages/db/src/collection/state.ts @@ -1,6 +1,7 @@ import { deepEquals } from '../utils' import { SortedMap } from '../SortedMap' import { enrichRowWithVirtualProps } from '../virtual-props.js' +import { SyncTransactionAbortedError } from '../errors.js' import { DIRECT_TRANSACTION_METADATA_KEY } from './transaction-metadata.js' import type { VirtualOrigin, @@ -26,13 +27,14 @@ interface PendingSyncedTransaction< TKey extends string | number = string | number, > { committed: boolean + applicationStarted: boolean layoutChanged: boolean operations: Array> truncate?: boolean deletedKeys: Set rowMetadataWrites: Map collectionMetadataWrites: Map - /** Resolves after this transaction's writes and events are visible. */ + /** Resolves after application and rejects if canceled before application. */ applied: Deferred optimisticSnapshot?: { upserts: Map @@ -884,6 +886,13 @@ export class CollectionStateManager< // non-immediate transactions would be applied later and could overwrite newer state. // Processing all committed transactions together preserves causal ordering. if (!hasPersistingTransaction || hasTruncateSync || hasImmediateSync) { + // Application is now the point of no return. Event listeners run before + // the receipts resolve, so a signal aborted from one of those listeners + // must not cancel writes that are already becoming visible. + for (const transaction of committedSyncedTransactions) { + transaction.applicationStarted = true + } + // Set flag to prevent redundant optimistic state recalculations this.isCommittingSyncTransactions = true @@ -1374,11 +1383,13 @@ export class CollectionStateManager< public cancelPendingSyncedTransaction( transaction: PendingSyncedTransaction, ): void { + if (transaction.applicationStarted) return + const index = this.pendingSyncedTransactions.indexOf(transaction) if (index === -1) return this.pendingSyncedTransactions.splice(index, 1) - transaction.applied.resolve() + transaction.applied.reject(new SyncTransactionAbortedError()) const remainingPendingKeys = new Set() for (const pending of this.pendingSyncedTransactions) { @@ -1482,9 +1493,7 @@ export class CollectionStateManager< */ public cleanup(): void { for (const transaction of this.pendingSyncedTransactions) { - // Applied receipts never reject. Cleanup abandons the collection and - // releases callers that may have retained and ignored a receipt. - transaction.applied.resolve() + transaction.applied.reject(new SyncTransactionAbortedError()) } this.syncedData.clear() this.syncedMetadata.clear() diff --git a/packages/db/src/collection/sync.ts b/packages/db/src/collection/sync.ts index 4c7539417a..a14a357ddd 100644 --- a/packages/db/src/collection/sync.ts +++ b/packages/db/src/collection/sync.ts @@ -126,15 +126,21 @@ export class CollectionSyncManager< collection: this.collection, begin: (options?: { immediate?: boolean }) => { if (!isCurrentSync()) return + const applied = createDeferred() + // A source may ignore a stream receipt. Keep cancellation from + // becoming an unhandled rejection while preserving the original + // promise's rejection for callers that do await it. + void applied.promise.catch(() => undefined) this.state.pendingSyncedTransactions.push({ committed: false, + applicationStarted: false, layoutChanged: false, operations: [], deletedKeys: new Set(), rowMetadataWrites: new Map(), collectionMetadataWrites: new Map(), immediate: options?.immediate, - applied: createDeferred(), + applied, }) }, write: ( @@ -247,7 +253,7 @@ export class CollectionSyncManager< if (signal?.aborted) { this.state.cancelPendingSyncedTransaction(pendingTransaction) - return true + return pendingTransaction.applied.promise } pendingTransaction.committed = true @@ -265,9 +271,10 @@ export class CollectionSyncManager< const receipt = pendingTransaction.applied.promise if (signal) { - void receipt.then(() => { + const removeAbortListener = () => { signal.removeEventListener(`abort`, cancel) - }) + } + void receipt.then(removeAbortListener, removeAbortListener) } return receipt }, diff --git a/packages/db/src/errors.ts b/packages/db/src/errors.ts index 25d5b4db85..12c6753d3b 100644 --- a/packages/db/src/errors.ts +++ b/packages/db/src/errors.ts @@ -729,6 +729,14 @@ export class SyncCleanupError extends TanStackDBError { } } +/** A sync transaction was canceled before its writes became visible. */ +export class SyncTransactionAbortedError extends Error { + constructor() { + super(`Sync transaction was aborted before application`) + this.name = `AbortError` + } +} + // Query Optimizer Errors export class QueryOptimizerError extends TanStackDBError { constructor(message: string) { diff --git a/packages/db/src/query/live/ARCHITECTURE.md b/packages/db/src/query/live/ARCHITECTURE.md index 6a546c1c7d..794bfcc4a5 100644 --- a/packages/db/src/query/live/ARCHITECTURE.md +++ b/packages/db/src/query/live/ARCHITECTURE.md @@ -455,16 +455,26 @@ ways to satisfy that contract; they are not materializer state. Every sync `commit()` returns an applied receipt: `true` when that transaction's writes and events are already visible, or a promise when the transaction is parked in the causal queue. The promise resolves only after the -writes and events become visible, or after collection cleanup abandons the -transaction. A successful `loadSubset` implementation must await or return -every receipt for the transactions that establish its result. A source must -not add priority merely to make a subset load settle. +writes and events become visible. It rejects with `AbortError` if request +cancellation or collection cleanup abandons the transaction first. An abort +after application has no effect. Application becomes irrevocable before change +events are emitted, so an abort raised by a publication observer is already +late. A successful `loadSubset` implementation must await or return every +receipt for the transactions that establish its result. A source must not add +priority merely to make a subset load settle. Existing immediate bootstrap and persistence-hydration paths, plus truncate, retain their queue-bypass contract; if one applies a parked subset transaction as part of that prefix, the subset receipt settles only after the writes are visible. Rejected, canceled, and obsolete acquisitions establish no coverage. Sources must honor cancellation before publishing request-scoped rows. +A transaction `mutationFn` must not start or await collection or live-query +preloads. User persistence owns the causal queue while that function runs, so a +preload that waits for a queued sync commit can wait on the mutation that is +waiting on the preload. Use an adapter's documented mutation acknowledgement +helper instead; it must confirm the optimistic write without starting new +collection demand. + This project uses a single graph-run order rather than multi-dimensional timely-dataflow frontiers. Do not introduce a general timestamp or frontier framework unless a source contract proves that the generation and up-to-date diff --git a/packages/db/src/types.ts b/packages/db/src/types.ts index f49797e784..29db572da0 100644 --- a/packages/db/src/types.ts +++ b/packages/db/src/types.ts @@ -126,6 +126,12 @@ export type MutationFnParams> = { transaction: TransactionWithMutations } +/** + * Persists an optimistic transaction. Do not start or await collection or + * live-query preloads here. Sync commits queue behind this function, so waiting + * for preload work that needs one of those commits can deadlock the mutation. + * Use the collection adapter's mutation acknowledgement helper instead. + */ export type MutationFn> = ( params: MutationFnParams, ) => Promise @@ -340,8 +346,10 @@ export type LoadSubsetOptions = { export type LoadSubsetFn = (options: LoadSubsetOptions) => true | Promise /** - * Confirms whether a committed sync transaction is already visible or is - * waiting for its turn in the collection's causal queue. + * Confirms whether a committed sync transaction is visible or is waiting for + * its turn in the collection's causal queue. A pending receipt rejects with an + * error named `AbortError` if cancellation wins before application. Once the + * writes are visible, later cancellation has no effect. */ export type SyncAppliedReceipt = true | Promise @@ -370,10 +378,11 @@ export interface SyncConfig< /** * Commit the active sync transaction in FIFO order. * Returns `true` when the writes and events are already visible. Otherwise - * returns a receipt that resolves after they become visible, or after - * collection cleanup or an optional request abort abandons the transaction. + * returns a receipt that resolves after they become visible. If collection + * cleanup or an optional request abort abandons the transaction first, the + * receipt rejects with an error named `AbortError`. * Pass a signal only for request-scoped work that must not publish after - * cancellation. The receipt never rejects. + * cancellation. Aborting after application has no effect. */ commit: (signal?: AbortSignal) => SyncAppliedReceipt /** Signal that a usable initial or recovered snapshot is available. */ diff --git a/packages/db/tests/collection-subscribe-changes.test.ts b/packages/db/tests/collection-subscribe-changes.test.ts index 810acdcd18..c059a2ad38 100644 --- a/packages/db/tests/collection-subscribe-changes.test.ts +++ b/packages/db/tests/collection-subscribe-changes.test.ts @@ -731,6 +731,7 @@ describe(`Collection.subscribeChanges`, () => { begin: () => { syncCollection._state.pendingSyncedTransactions.push({ committed: false, + applicationStarted: false, operations: [], }) }, diff --git a/packages/db/tests/query/load-subset-oracle.property.test.ts b/packages/db/tests/query/load-subset-oracle.property.test.ts index d42080e6da..ff535c18fc 100644 --- a/packages/db/tests/query/load-subset-oracle.property.test.ts +++ b/packages/db/tests/query/load-subset-oracle.property.test.ts @@ -1563,7 +1563,7 @@ async function expectLaterImmediateCommitSettlesAppliedSubset() { } async function expectAbortedReceiptDoesNotPublishCoverage( - abortPhase: `before-commit` | `after-commit`, + abortPhase: `before-commit` | `while-parked`, ) { let transportCalls = 0 const committed = createDeferred() @@ -1616,7 +1616,7 @@ async function expectAbortedReceiptDoesNotPublishCoverage( const first = requirePendingAppliedReceipt( source._sync.loadSubset({ signal: controller.signal }), ) - if (abortPhase === `after-commit`) { + if (abortPhase === `while-parked`) { await committed.promise } controller.abort() @@ -1624,7 +1624,11 @@ async function expectAbortedReceiptDoesNotPublishCoverage( try { persistence.resolve() await transaction.isPersisted.promise - await first + if (abortPhase === `while-parked`) { + await expect(first).rejects.toMatchObject({ name: `AbortError` }) + } else { + await first + } expect(transportCalls).toBe(1) expect(source.get(`row`)).toBeUndefined() @@ -1639,6 +1643,58 @@ async function expectAbortedReceiptDoesNotPublishCoverage( } } +async function expectAbortDuringPublicationDoesNotCancelReceipt() { + const controller = new AbortController() + const source = createCollection({ + id: `load-subset-applied-publication-abort-${collectionSequence++}`, + getKey: (row) => row.id, + syncMode: `on-demand`, + sync: { + sync: ({ begin, write, commit, markReady }) => { + markReady() + return { + loadSubset: ({ signal }) => { + begin() + write({ + type: `insert`, + value: { id: `row`, projectId: `p1` }, + }) + return commit(signal) + }, + } + }, + }, + }) + source.startSyncImmediate() + + const persistence = createDeferred() + const transaction = createTransaction({ + mutationFn: () => persistence.promise, + }) + transaction.mutate(() => source.insert({ id: `local`, projectId: `pending` })) + const subscription = source.subscribeChanges((changes) => { + if (changes.some((change) => change.key === `row`)) { + controller.abort() + } + }) + const load = requirePendingAppliedReceipt( + source._sync.loadSubset({ signal: controller.signal }), + ) + + try { + persistence.resolve() + await transaction.isPersisted.promise + await expect(load).resolves.toBeUndefined() + expect(controller.signal.aborted).toBe(true) + expect(source.get(`row`)).toEqual(expect.objectContaining({ id: `row` })) + } finally { + subscription.unsubscribe() + persistence.resolve() + await transaction.isPersisted.promise.catch(() => undefined) + await source.cleanup() + } +} + async function expectCanceledReceiptReleasesOnlyItsSuppression() { let begin!: () => void let write!: (message: { type: `update`; value: PersistedLoadRow }) => void @@ -1687,7 +1743,9 @@ async function expectCanceledReceiptReleasesOnlyItsSuppression() { expect(source._state.recentlySyncedKeys).toEqual(new Set([`second`])) expect(source._state.preSyncVisibleState.has(`first`)).toBe(false) expect(source._state.preSyncVisibleState.has(`second`)).toBe(true) - if (canceled !== true) await canceled + if (canceled !== true) { + await expect(canceled).rejects.toMatchObject({ name: `AbortError` }) + } } finally { persistence.resolve() await transaction.isPersisted.promise.catch(() => undefined) @@ -1695,7 +1753,7 @@ async function expectCanceledReceiptReleasesOnlyItsSuppression() { } } -async function expectCleanupSettlesReceiptOnce() { +async function expectCleanupRejectsReceiptOnce() { let receipt!: Promise let transportCalls = 0 const deduplicated = new DeduplicatedLoadSubset({ @@ -1745,12 +1803,18 @@ async function expectCleanupSettlesReceiptOnce() { transaction.mutate(() => source.insert({ id: `local`, projectId: `p2` })) const load = requirePendingAppliedReceipt(source._sync.loadSubset({})) let settlements = 0 - void receipt.then(() => { - settlements += 1 - }) + void receipt.then( + () => { + settlements += 1 + }, + () => { + settlements += 1 + }, + ) await source.cleanup() - await Promise.all([load, receipt]) + await expect(load).rejects.toMatchObject({ name: `AbortError` }) + await expect(receipt).rejects.toMatchObject({ name: `AbortError` }) expect(settlements).toBe(1) persistence.resolve() @@ -2693,17 +2757,21 @@ describe(`loadSubset coverage oracle`, () => { await expectLaterImmediateCommitSettlesAppliedSubset() }) - it.each([`before-commit`, `after-commit`] as const)( + it.each([`before-commit`, `while-parked`] as const)( `does not publish coverage when a parked receipt is aborted %s`, expectAbortedReceiptDoesNotPublishCoverage, ) + it(`ignores an abort raised after application starts publishing`, async () => { + await expectAbortDuringPublicationDoesNotCancelReceipt() + }) + it(`releases only a canceled receipt's event suppression`, async () => { await expectCanceledReceiptReleasesOnlyItsSuppression() }) - it(`settles an abandoned receipt once without publishing coverage`, async () => { - await expectCleanupSettlesReceiptOnce() + it(`rejects an abandoned receipt once without publishing coverage`, async () => { + await expectCleanupRejectsReceiptOnce() }) it(`publishes synced source rows while a derived mutation persists`, async () => { diff --git a/packages/electric-db-collection/src/electric.ts b/packages/electric-db-collection/src/electric.ts index 6809eb6a3a..39e6540a6a 100644 --- a/packages/electric-db-collection/src/electric.ts +++ b/packages/electric-db-collection/src/electric.ts @@ -1510,7 +1510,8 @@ function createElectricSync>( return true } pendingAppliedReceipts.set(sequence, applied) - void applied.then(() => pendingAppliedReceipts.delete(sequence)) + const removeReceipt = () => pendingAppliedReceipts.delete(sequence) + void applied.then(removeReceipt, removeReceipt) return applied } const waitForCommitsAfter = async (cursor: number): Promise => { @@ -2011,8 +2012,10 @@ function createElectricSync>( if (applied === true) { wrappedMarkReady(wasBufferingInitialSync, readyErrorVersion) } else { - void applied.then(() => - wrappedMarkReady(wasBufferingInitialSync, readyErrorVersion), + void applied.then( + () => + wrappedMarkReady(wasBufferingInitialSync, readyErrorVersion), + () => undefined, ) } diff --git a/packages/trailbase-db-collection/tests/trailbase.test.ts b/packages/trailbase-db-collection/tests/trailbase.test.ts index 7370ec1c12..8ed7ddc71a 100644 --- a/packages/trailbase-db-collection/tests/trailbase.test.ts +++ b/packages/trailbase-db-collection/tests/trailbase.test.ts @@ -317,7 +317,9 @@ describe(`TrailBase Integration`, () => { abortController.abort() resolvePersistence() await transaction.isPersisted.promise - if (load instanceof Promise) await load + if (load instanceof Promise) { + await expect(load).rejects.toMatchObject({ name: `AbortError` }) + } expect(collection.get(1)).toBeUndefined() expect(recordApi.list).toHaveBeenCalledOnce() From b716366dfe4c807155d13044b56578218d509dec Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 25 Aug 2026 10:45:28 -0600 Subject: [PATCH 14/14] test(db): tighten applied receipt coverage --- .../collection-subscribe-changes.test.ts | 62 +++++++++---------- .../tests/trailbase.test.ts | 5 +- 2 files changed, 31 insertions(+), 36 deletions(-) diff --git a/packages/db/tests/collection-subscribe-changes.test.ts b/packages/db/tests/collection-subscribe-changes.test.ts index c059a2ad38..08dce91992 100644 --- a/packages/db/tests/collection-subscribe-changes.test.ts +++ b/packages/db/tests/collection-subscribe-changes.test.ts @@ -695,8 +695,9 @@ describe(`Collection.subscribeChanges`, () => { expect(callback).not.toHaveBeenCalled() }) - it(`should correctly handle filtered updates that transition between filter states`, () => { + it(`should correctly handle filtered updates that transition between filter states`, async () => { const callback = vi.fn() + const emitter = mitt() // Create collection with items that have a status field const collection = createCollection<{ @@ -708,6 +709,21 @@ describe(`Collection.subscribeChanges`, () => { getKey: (item) => item.id, sync: { sync: ({ begin, write, commit }) => { + // Feed persisted mutations back through the real sync transaction + // path so this test also observes applied-receipt failures. + // @ts-expect-error don't trust Mitt's typing and this works. + emitter.on(`*`, (_, changes: Array) => { + begin() + changes.forEach((change) => { + write({ + type: change.type, + // @ts-expect-error TODO type changes + value: change.modified, + }) + }) + commit() + }) + // Start with some initial data begin() write({ @@ -723,39 +739,8 @@ describe(`Collection.subscribeChanges`, () => { }, }) - const mutationFn: MutationFn = async () => { - // Simulate sync by writing the mutations back - const syncCollection = collection as any - syncCollection.config.sync.sync({ - collection: syncCollection, - begin: () => { - syncCollection._state.pendingSyncedTransactions.push({ - committed: false, - applicationStarted: false, - operations: [], - }) - }, - write: (messageWithoutKey: any) => { - const pendingTransaction = - syncCollection._state.pendingSyncedTransactions[ - syncCollection._state.pendingSyncedTransactions.length - 1 - ] - const key = syncCollection.getKeyFromItem(messageWithoutKey.value) - const message = { ...messageWithoutKey, key } - pendingTransaction.operations.push(message) - }, - commit: () => { - const pendingTransaction = - syncCollection._state.pendingSyncedTransactions[ - syncCollection._state.pendingSyncedTransactions.length - 1 - ] - pendingTransaction.committed = true - syncCollection.commitPendingTransactions() - }, - markReady: () => { - syncCollection.markReady() - }, - }) + const mutationFn: MutationFn = ({ transaction }) => { + emitter.emit(`sync`, transaction.mutations) return Promise.resolve() } @@ -859,6 +844,15 @@ describe(`Collection.subscribeChanges`, () => { // Should not emit any events for inactive items expect(callback).not.toHaveBeenCalled() + // Keep the immediate optimistic assertions isolated above, then prove that + // every auto-commit also completes through applied-receipt settlement. + await Promise.all([ + tx1.isPersisted.promise, + tx2.isPersisted.promise, + tx3.isPersisted.promise, + tx4.isPersisted.promise, + ]) + // Clean up subscription.unsubscribe() }) diff --git a/packages/trailbase-db-collection/tests/trailbase.test.ts b/packages/trailbase-db-collection/tests/trailbase.test.ts index 8ed7ddc71a..43b28db28e 100644 --- a/packages/trailbase-db-collection/tests/trailbase.test.ts +++ b/packages/trailbase-db-collection/tests/trailbase.test.ts @@ -317,9 +317,10 @@ describe(`TrailBase Integration`, () => { abortController.abort() resolvePersistence() await transaction.isPersisted.promise - if (load instanceof Promise) { - await expect(load).rejects.toMatchObject({ name: `AbortError` }) + if (load === true) { + throw new Error(`Expected a pending applied receipt`) } + await expect(load).rejects.toMatchObject({ name: `AbortError` }) expect(collection.get(1)).toBeUndefined() expect(recordApi.list).toHaveBeenCalledOnce()