diff --git a/.changeset/aggregate-per-aggregation-filter.md b/.changeset/aggregate-per-aggregation-filter.md new file mode 100644 index 0000000000..1710444432 --- /dev/null +++ b/.changeset/aggregate-per-aggregation-filter.md @@ -0,0 +1,34 @@ +--- +"@objectstack/spec": minor +"@objectstack/objectql": minor +"@objectstack/driver-sql": patch +"@objectstack/driver-turso": patch +"@objectstack/driver-mongodb": patch +"@objectstack/driver-memory": patch +--- + +`engine.aggregate` honours a per-aggregation `filter` (#10576, the contract +half of #10413). `AggregationNodeSchema.filter` — declared since #4286 but +marked experimental and enforced by nothing — is now live with SQL +`FILTER (WHERE …)` semantics: the predicate narrows the SOURCE rows that one +aggregation reads while sibling aggregations in the same call keep seeing +every row of the group, so a measure-scoped filter (`stage: 'closed_won'`) +can finally reach the engine instead of being silently dropped (the #10413 +wrong-numbers defect on the ObjectQL analytics path). The +`StrategyContext.executeAggregate` bridge (`@objectstack/spec/contracts`) +gains the same optional `filter` on its aggregation entries so analytics +strategies can lower measure filters into it (#10413 phase 2 consumes this +seam next). + +Execution is the correct-first two-tier shape date bucketing and HAVING use: +the engine lowers filtered aggregations in memory for every driver (unknown +operators refuse loudly with `INVALID_FILTER`/400 naming the aggregation +position; a group emptied by its filter answers the ruled empty-group values +— count/sum 0, avg/min/max null). No driver compiles conditional aggregation +natively today, so each native aggregate face (driver-sql — inherited by +driver-sqlite-wasm and Turso local —, the Turso remote transport, +driver-mongodb's pipeline builder, driver-memory's `performAggregation`) +refuses a directly-delivered per-aggregation filter with +`NOT_IMPLEMENTED`/501 instead of silently aggregating the unfiltered rows. +Aggregations without a `filter` are byte-identically unchanged, including +their native pushdown path. diff --git a/content/docs/references/data/query.mdx b/content/docs/references/data/query.mdx index c50481fc0d..f954194b91 100644 --- a/content/docs/references/data/query.mdx +++ b/content/docs/references/data/query.mdx @@ -49,7 +49,7 @@ const result = AggregationFunction.parse(data); | **field** | `string` | optional | Field to aggregate (optional for COUNT(*)) | | **alias** | `string` | ✅ | Result column alias | | **distinct** | `never` | optional | [REMOVED] `query.aggregations[].distinct` was removed in @objectstack/spec 17 (#6815, ADR-0049) — exactly ONE of the six faces that read an aggregation honoured it. The objectql in-memory fallback deduplicated the values before applying the function, while `driver-sql`, `driver-turso`, `driver-mongodb`, `driver-memory` and the service-analytics SQL builder all ignored it — so `{ function: 'sum', field: 'amount', distinct: true }` answered a DEDUPLICATED sum when the engine fell back in memory and an ordinary sum on every SQL datasource: one query, two numbers, chosen by which backend happened to serve it. Both answers are plausible, so nothing surfaced the divergence. Delete the key. For a deduplicated COUNT the live spelling is the `count_distinct` aggregation function, which every SQL face compiles to `COUNT(DISTINCT field)` (#6409) and the in-memory fallback computes identically. `SUM(DISTINCT …)` / `AVG(DISTINCT …)` get no replacement: no backend ever computed them here, and a per-row measure that needs deduplicating is a modelling problem to fix in the data, not a flag on the read. | -| **filter** | `any` | optional | [EXPERIMENTAL — not enforced] Per-aggregation filter (SQL FILTER (WHERE …)). Neither the SQL builders nor the in-memory fallback applies it (#4286); filter the whole query with `where` instead. | +| **filter** | `any` | optional | Per-aggregation filter (SQL FILTER (WHERE …) semantics): narrows the source rows THIS aggregation reads, leaving sibling aggregations unfiltered. Enforced by engine.aggregate (#10576): lowered in memory for drivers without native conditional aggregation; a driver reached directly refuses rather than silently dropping it. | --- diff --git a/packages/drivers/driver-memory/src/filter-refusal.ts b/packages/drivers/driver-memory/src/filter-refusal.ts index e40ba334c0..9ef3e4e645 100644 --- a/packages/drivers/driver-memory/src/filter-refusal.ts +++ b/packages/drivers/driver-memory/src/filter-refusal.ts @@ -54,6 +54,36 @@ export function unsupportedFilterError(message: string): Error { return err; } +/** + * [#10576] An aggregation entry carries a per-aggregation `filter` + * (`AggregationNodeSchema.filter`, the contract half of #10413) — the twin of + * `driver-sql`'s `unsupportedAggregationFilterError`, first sentence for first + * sentence, and the same NOT_IMPLEMENTED/501 class for the same reason (#5907, + * ADR-0112): the spec declares the key, this driver's `performAggregation` + * evaluates no per-aggregation predicate, so it is a capability gap in the + * backend rather than a mistake in the query. Building the evaluation here is + * a capability investment this refusal deliberately is not (#5499 freeze). + * Refused rather than silently aggregating the UNFILTERED rows — the #10413 + * defect. Unreachable through `engine.aggregate` (the engine lowers filtered + * aggregations in memory for every driver); this fires only for a caller that + * reaches the driver's own aggregation faces directly (`find()` with + * aggregations, or `aggregate(AST)`). + */ +export function refusePerAggregationFilter(alias: string): never { + const err = new Error( + `Per-aggregation \`filter\` on "${alias}" is not supported by this backend (driver-memory). ` + + `The query is spelled correctly and @objectstack/spec AggregationNodeSchema declares the key — ` + + `this backend compiles no conditional-aggregate (SQL FILTER (WHERE …) / CASE WHEN) expression ` + + `for it, so it is refused rather than silently aggregating the UNFILTERED rows (#10413), which ` + + `is why it answers NOT_IMPLEMENTED/501 rather than a 400. \`engine.aggregate\` lowers filtered ` + + `aggregations in memory for every driver without native support — route the query through the ` + + `engine, or drop the \`filter\` key.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; + err.status = 501; + throw err; +} + /** * [#5158] A `FilterArray` reached the driver unlowered. * diff --git a/packages/drivers/driver-memory/src/memory-aggregation-filter-refusal.test.ts b/packages/drivers/driver-memory/src/memory-aggregation-filter-refusal.test.ts new file mode 100644 index 0000000000..556c39c6ec --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-aggregation-filter-refusal.test.ts @@ -0,0 +1,86 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10576] A per-aggregation `filter` (`AggregationNodeSchema.filter`, the + * contract half of #10413) reaching this driver's own aggregation faces is + * refused with the ADR-0112 envelope — never silently answered with the + * UNFILTERED aggregate, which is the #10413 defect at the driver seam. + * + * This driver has TWO doors into `performAggregation` — `aggregate(AST)` (the + * one objectql's engine uses) and `find()` with aggregations — so both are + * pinned: a guard on one door alone re-opens the drop through the other. The + * engine itself never pushes a filtered aggregation down (it lowers in + * memory); the refusal exists for direct callers. Evaluating the predicate + * here instead would be a capability build-out, which the #5499 family freeze + * rules out — the refusal path is the sanctioned scope. + * + * Every case asserts `code` AND `status`, never merely "it threw" (#6144). + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import type { DriverQuery } from '@objectstack/spec/contracts'; +import { InMemoryDriver } from './memory-driver.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +const TABLE = 'deal'; + +describe('[#10576] InMemoryDriver refuses a per-aggregation filter it does not evaluate', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver(); + await driver.connect(); + await driver.create(TABLE, { id: '1', stage: 'closed_won', amount: 500 }); + await driver.create(TABLE, { id: '2', stage: 'open', amount: 900 }); + }); + + const filteredQuery = (): DriverQuery => ({ + aggregations: [{ function: 'count', alias: 'won_count', filter: { stage: 'closed_won' } }], + }) as unknown as DriverQuery; + + it('the aggregate(AST) door answers NOT_IMPLEMENTED / 501, naming the aggregation and the engine lowering', async () => { + let thrown: WireBearingError | undefined; + try { + await driver.aggregate(TABLE, filteredQuery()); + } catch (e) { + thrown = e as WireBearingError; + } + expect(thrown, 'a filter this face does not evaluate must not be silently dropped').toBeDefined(); + expect(thrown!.code).toBe('NOT_IMPLEMENTED'); + expect(thrown!.status).toBe(501); + expect(thrown!.message).toContain( + 'Per-aggregation `filter` on "won_count" is not supported by this backend (driver-memory).', + ); + expect(thrown!.message).toContain('`engine.aggregate` lowers filtered aggregations in memory'); + }); + + it('the find()-with-aggregations door refuses identically — one guard covers both doors', async () => { + let thrown: WireBearingError | undefined; + try { + await driver.find(TABLE, filteredQuery()); + } catch (e) { + thrown = e as WireBearingError; + } + expect(thrown).toBeDefined(); + expect(thrown!.code).toBe('NOT_IMPLEMENTED'); + expect(thrown!.status).toBe(501); + expect(thrown!.message).toContain('Per-aggregation `filter` on "won_count"'); + }); + + it('control: the same aggregation WITHOUT a filter still computes through both doors', async () => { + const bare = { aggregations: [{ function: 'count', alias: 'n' }] } as unknown as DriverQuery; + await expect(driver.aggregate(TABLE, bare)).resolves.toEqual([{ n: 2 }]); + await expect(driver.find(TABLE, bare)).resolves.toEqual([{ n: 2 }]); + }); + + it('control: an EMPTY filter object is vacuous (the where/having convention) and computes', async () => { + const vacuous = { + aggregations: [{ function: 'sum', field: 'amount', alias: 'total', filter: {} }], + } as unknown as DriverQuery; + await expect(driver.aggregate(TABLE, vacuous)).resolves.toEqual([{ total: 1400 }]); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index 28fc3dae19..5a6de1ee66 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -28,6 +28,8 @@ import { // [#7536] The `$like`/`$ilike` comparand refusals, beside their siblings. likePatternComparandError, danglingLikeEscapeError, + // [#10576] The per-aggregation `filter` refusal — this driver evaluates none. + refusePerAggregationFilter, } from './filter-refusal.js'; import { coerceTemporalValue, @@ -1159,6 +1161,19 @@ export class InMemoryDriver implements IDataDriver { private performAggregation(records: any[], query: DriverQuery): any[] { const { groupBy, aggregations } = query; + // [#10576] A per-aggregation `filter` this face does not evaluate is + // refused before any group is built — silently answering the UNFILTERED + // aggregate is the #10413 defect. Guarded HERE because both of this + // driver's aggregation doors (`find()` with aggregations and + // `aggregate(AST)`) funnel through this method. `{}` is the vacuous + // filter, same convention as `where` / `having`. See + // {@link refusePerAggregationFilter}. + for (const agg of aggregations ?? []) { + const f = (agg as { filter?: unknown }).filter; + if (f && typeof f === 'object' && Object.keys(f).length > 0) { + refusePerAggregationFilter((agg as any).alias ?? (agg as any).field ?? '(unaliased)'); + } + } const groups: Map = new Map(); const normalizeGroupBy = (node: any): { field: string; alias: string } => { diff --git a/packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts b/packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts index 89ebc1bca9..e76c4ec239 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-aggregation-translation.test.ts @@ -330,3 +330,40 @@ describe('the in-process pipeline evaluator discriminates', () => { expect(counted).toBe(1); }); }); + +/** + * [#10576] A per-aggregation `filter` (`AggregationNodeSchema.filter`, the + * contract half of #10413) reaching this builder is refused with the ADR-0112 + * envelope — never silently accumulated over the UNFILTERED rows, which is the + * #10413 defect at the driver seam. `AggregationInput.filter` has declared the + * key since #6850 and nothing ever read it; a declared key a builder ignores + * is exactly the silent drop this card closes. The engine never pushes a + * filtered aggregation down (it lowers in memory) — the refusal exists for the + * direct caller of this exported builder. + */ +describe('[#10576] per-aggregation filter refuses instead of silently dropping', () => { + it('answers NOT_IMPLEMENTED / 501, naming the aggregation and the engine lowering', () => { + let thrown: (Error & { code?: string; status?: number }) | undefined; + try { + buildAggregationPipeline({ + aggregations: [{ function: 'count', alias: 'won_count', filter: { stage: 'closed_won' } }], + }); + } catch (err) { + thrown = err as Error & { code?: string; status?: number }; + } + expect(thrown, 'a filter this builder cannot lower must not be silently dropped').toBeDefined(); + expect(thrown!.code).toBe('NOT_IMPLEMENTED'); + expect(thrown!.status).toBe(501); + expect(thrown!.message).toContain( + 'Per-aggregation `filter` on "won_count" is not supported by this backend (driver-mongodb).', + ); + expect(thrown!.message).toContain('`engine.aggregate` lowers filtered aggregations in memory'); + }); + + it('control: an EMPTY filter object is vacuous (the where/having convention) and still lowers', () => { + const pipeline = buildAggregationPipeline({ + aggregations: [{ function: 'count', alias: 'n', filter: {} }], + }); + expect(pipeline[0]).toHaveProperty('$group'); + }); +}); diff --git a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts index 0b3b50f36d..876623b8f5 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-aggregation.ts @@ -303,6 +303,33 @@ function refuseDateBucketedGroupBy(granularity: string): never { throw err; } +/** + * [#10576] An aggregation entry carries a per-aggregation `filter` + * (`AggregationNodeSchema.filter`, the contract half of #10413) — the twin of + * `driver-sql`'s `unsupportedAggregationFilterError`, first sentence for first + * sentence, and the same NOT_IMPLEMENTED/501 class for the same reason (#5907, + * ADR-0112): the spec declares the key, this builder emits no conditional + * accumulator for it, so it is a capability gap in the backend rather than a + * mistake in the query. Refused rather than silently accumulating the + * UNFILTERED rows — the #10413 defect. Unreachable through `engine.aggregate` + * (the engine lowers filtered aggregations in memory for every driver); this + * fires only for a caller that drives the builder or driver directly. + */ +function refusePerAggregationFilter(alias: string): never { + const err = new Error( + `Per-aggregation \`filter\` on "${alias}" is not supported by this backend (driver-mongodb). ` + + `The query is spelled correctly and @objectstack/spec AggregationNodeSchema declares the key — ` + + `this backend compiles no conditional-aggregate (SQL FILTER (WHERE …) / CASE WHEN) expression ` + + `for it, so it is refused rather than silently aggregating the UNFILTERED rows (#10413), which ` + + `is why it answers NOT_IMPLEMENTED/501 rather than a 400. \`engine.aggregate\` lowers filtered ` + + `aggregations in memory for every driver without native support — route the query through the ` + + `engine, or drop the \`filter\` key.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; + err.status = 501; + throw err; +} + /** * [#6850] A `groupBy` entry that is neither half of the declared union. * @@ -386,6 +413,17 @@ export function buildAggregationPipeline(opts: { // Build accumulators from aggregation descriptors for (const agg of opts.aggregations) { + // [#10576] A per-aggregation `filter` (`AggregationNodeSchema.filter`, + // the contract half of #10413) has no lowering in this builder — a + // `$cond`-wrapped accumulator would be one, but building it is a + // capability investment this refusal deliberately is not. Refused before + // a pipeline exists rather than silently accumulating the UNFILTERED + // rows (the #10413 defect). Unreachable through `engine.aggregate`, + // which lowers filtered aggregations in memory for every driver; `{}` is + // the vacuous filter, same convention as `where` / `having`. + if (agg.filter && typeof agg.filter === 'object' && Object.keys(agg.filter).length > 0) { + refusePerAggregationFilter(agg.alias ?? agg.field ?? '(unaliased)'); + } groupAccumulators[agg.alias] = buildAccumulator(agg); } diff --git a/packages/drivers/driver-sql/src/sql-driver-aggregation-filter-refusal.test.ts b/packages/drivers/driver-sql/src/sql-driver-aggregation-filter-refusal.test.ts new file mode 100644 index 0000000000..8da022152e --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-aggregation-filter-refusal.test.ts @@ -0,0 +1,93 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10576] A per-aggregation `filter` (`AggregationNodeSchema.filter`, the + * contract half of #10413) reaching this compiler DIRECTLY is refused with a + * wire identity — never silently dropped. + * + * Before #10576 this driver's `aggregate()` never read `agg.filter`: the + * statement it built aggregated EVERY row and reported success, which is the + * #10413 defect ("won deals" counting every opportunity) at the driver seam. + * The engine now lowers filtered aggregations in memory and never pushes one + * down here — so the only caller that can arrive with the key is a direct + * one, and the honest answers are exactly two: compile a conditional + * aggregate, or refuse loudly. This backend refuses (NOT_IMPLEMENTED/501, the + * #5907 class for "declared by the spec, not compiled by this face"). + * + * Every case asserts `code` AND `status`, never merely "it threw" (#6144): a + * bare-throw assertion is green before and after the envelope exists. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { SqlDriver } from './index.js'; +import type { QueryAST } from '@objectstack/spec/data'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +describe('[#10576] SqlDriver refuses a per-aggregation filter it cannot compile', () => { + let driver: SqlDriver; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.initObjects([ + { + name: 'deal', + fields: { + id: { type: 'text', name: 'id' }, + stage: { type: 'text', name: 'stage' }, + amount: { type: 'number', name: 'amount' }, + }, + } as any, + ]); + await driver.create('deal', { id: '1', stage: 'closed_won', amount: 500 }); + await driver.create('deal', { id: '2', stage: 'open', amount: 900 }); + }); + + it('refuses NOT_IMPLEMENTED/501, naming the aggregation and the engine lowering', async () => { + const ast = { + object: 'deal', + aggregations: [ + { function: 'count', alias: 'won_count', filter: { stage: 'closed_won' } }, + ], + } as unknown as QueryAST; + + let thrown: WireBearingError | undefined; + try { + await driver.aggregate('deal', ast); + } catch (e) { + thrown = e as WireBearingError; + } + + expect(thrown).toBeDefined(); + expect(thrown!.code).toBe('NOT_IMPLEMENTED'); + expect(thrown!.status).toBe(501); + expect(thrown!.message).toContain( + 'Per-aggregation `filter` on "won_count" is not supported by this backend (driver-sql).', + ); + // The remedy is named: the engine's in-memory lowering serves this query. + expect(thrown!.message).toContain('`engine.aggregate` lowers filtered aggregations in memory'); + }); + + it('control: the same aggregation WITHOUT a filter still computes (only the refusal was added)', async () => { + const rows = await driver.aggregate('deal', { + object: 'deal', + aggregations: [{ function: 'count', alias: 'n' }], + } as unknown as QueryAST); + expect(rows).toEqual([{ n: 2 }]); + }); + + it('control: an EMPTY filter object is vacuous (the where/having convention) and computes', async () => { + const rows = await driver.aggregate('deal', { + object: 'deal', + aggregations: [{ function: 'sum', field: 'amount', alias: 'total', filter: {} }], + } as unknown as QueryAST); + expect(rows).toEqual([{ total: 1400 }]); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 51faa25379..4d4eb2b837 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -1342,6 +1342,38 @@ function uncompilableAggregateFunctionError(func: string): Error { return err; } +/** + * [#10576] An aggregation entry carries a per-aggregation `filter` + * (`AggregationNodeSchema.filter`, the contract half of #10413) and this + * backend compiles no conditional-aggregate expression (SQL `FILTER (WHERE …)` + * / `CASE WHEN`) for it — so it is refused rather than silently aggregating + * the UNFILTERED rows, which is exactly the wrong-numbers defect #10413 + * measured ("won deals" counting every row). Same class and envelope as + * {@link uncompilableAggregateFunctionError}: the query is spelled correctly + * and the spec declares the key, so this is a capability gap in the backend + * (NOT_IMPLEMENTED/501), not a mistake in the query (400). + * + * Unreachable through `engine.aggregate`, deliberately: the engine routes any + * call whose aggregations carry a filter through its in-memory lowering + * (`applyInMemoryAggregation`), which honours the predicate on every driver. + * This guard is the direct-caller half of "no silent drop, anywhere" — the + * refusal names the remedy so a direct caller knows the engine path works. + */ +function unsupportedAggregationFilterError(alias: string, backend: string): Error { + const err = new Error( + `Per-aggregation \`filter\` on "${alias}" is not supported by this backend (${backend}). ` + + `The query is spelled correctly and @objectstack/spec AggregationNodeSchema declares the key — ` + + `this backend compiles no conditional-aggregate (SQL FILTER (WHERE …) / CASE WHEN) expression ` + + `for it, so it is refused rather than silently aggregating the UNFILTERED rows (#10413), which ` + + `is why it answers NOT_IMPLEMENTED/501 rather than a 400. \`engine.aggregate\` lowers filtered ` + + `aggregations in memory for every driver without native support — route the query through the ` + + `engine, or drop the \`filter\` key.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; + err.status = 501; + return err; +} + /** * [#6409] `count_distinct` written with no `field` — nothing to deduplicate. * @@ -7052,6 +7084,15 @@ export class SqlDriver implements IDataDriver { const aggregates = query.aggregations; if (aggregates) { for (const agg of aggregates) { + // [#10576] A per-aggregation `filter` this compiler cannot express is + // refused before any statement is built — silently emitting the + // UNFILTERED aggregate is the #10413 defect. Unreachable through + // `engine.aggregate` (the engine lowers filtered aggregations in + // memory); this is the direct-caller guard. `{}` is the vacuous filter, + // same convention as `where` / `having`. + if (agg.filter && Object.keys(agg.filter).length > 0) { + throw unsupportedAggregationFilterError(agg.alias ?? agg.field ?? '(unaliased)', 'driver-sql'); + } const funcName = agg.function; const lowering = this.mapAggregateFunc(funcName); // Spec: `field` is optional for COUNT (means COUNT(*)). diff --git a/packages/drivers/driver-turso/src/remote-transport-aggregate-function-refusal.test.ts b/packages/drivers/driver-turso/src/remote-transport-aggregate-function-refusal.test.ts index 604737f850..d36fc7fa46 100644 --- a/packages/drivers/driver-turso/src/remote-transport-aggregate-function-refusal.test.ts +++ b/packages/drivers/driver-turso/src/remote-transport-aggregate-function-refusal.test.ts @@ -537,3 +537,51 @@ describe('[#5907] RemoteTransport refuses an aggregate function it cannot compil }); }); }); + +/** + * [#10576] The per-aggregation `filter` (`AggregationNodeSchema.filter`, the + * contract half of #10413) — a key BOTH faces of this driver decline to + * compile, so both must refuse it identically rather than silently aggregating + * the unfiltered rows (the #10413 defect at the driver seam). The engine never + * pushes a filtered aggregation down (it lowers in memory); these pins are the + * direct-caller guard, and the parity pin keeps #6203's lesson: one driver, + * two faces, one answer. + */ +describe('[#10576] per-aggregation filter is refused identically by both faces', () => { + const filteredAst = (): QueryAST => ({ + object: 'deal', + aggregations: [{ function: 'count', alias: 'won_count', filter: { stage: 'closed_won' } }], + }) as unknown as QueryAST; + + it('REMOTE refuses NOT_IMPLEMENTED/501 without a round trip, naming the aggregation and the engine lowering', async () => { + const err = await refusalOfAst('count(filter)', filteredAst()); + expect(err.code).toBe('NOT_IMPLEMENTED'); + expect(err.status).toBe(501); + expect(err.message).toContain( + 'Per-aggregation `filter` on "won_count" is not supported by this backend (Turso remote transport).', + ); + expect(err.message).toContain('`engine.aggregate` lowers filtered aggregations in memory'); + }); + + it('LOCAL (SqlDriver face) refuses the same class with the same first-sentence shape', async () => { + const remote = await refusalOfAst('count(filter)', filteredAst()); + const local = await localRefusalOf('count(filter)', filteredAst()); + expect(local.code).toBe('NOT_IMPLEMENTED'); + expect(local.status).toBe(501); + expect(local.message).toContain( + 'Per-aggregation `filter` on "won_count" is not supported by this backend (driver-sql).', + ); + // First sentence for first sentence: the two faces differ only in the + // backend name they print. + expect(remote.message.replace('(Turso remote transport)', '(driver-sql)')).toBe(local.message); + }); + + it('control: an EMPTY filter object is vacuous and still compiles on the remote face', async () => { + const { t, calls } = transportWithCapturingClient(); + await t.aggregate('deal', { + object: 'deal', + aggregations: [{ function: 'count', field: 'stage', alias: 'n', filter: {} }], + } as unknown as QueryAST); + expect(calls.map((c) => c.sql)).toEqual(['SELECT count("stage") AS "n" FROM "deal"']); + }); +}); diff --git a/packages/drivers/driver-turso/src/remote-transport.ts b/packages/drivers/driver-turso/src/remote-transport.ts index 475c480feb..939e3e13b6 100644 --- a/packages/drivers/driver-turso/src/remote-transport.ts +++ b/packages/drivers/driver-turso/src/remote-transport.ts @@ -902,6 +902,33 @@ function refuseAggregateFunction(func: string): never { : undeclaredAggregateFunctionError(func); } +/** + * [#10576] An aggregation entry carries a per-aggregation `filter` + * (`AggregationNodeSchema.filter`, the contract half of #10413) — the twin of + * `driver-sql`'s `unsupportedAggregationFilterError`, first sentence for first + * sentence, and the same NOT_IMPLEMENTED/501 class for the same reason: the + * spec declares the key, this transport compiles no conditional-aggregate + * expression for it, so it is a capability gap in the backend rather than a + * mistake in the query. Refused rather than silently aggregating the + * UNFILTERED rows — the #10413 defect. Unreachable through `engine.aggregate` + * (the engine lowers filtered aggregations in memory for every driver); this + * fires only for a caller that reached the transport directly. + */ +function refusePerAggregationFilter(alias: string): never { + const err = new Error( + `Per-aggregation \`filter\` on "${alias}" is not supported by this backend (Turso remote transport). ` + + `The query is spelled correctly and @objectstack/spec AggregationNodeSchema declares the key — ` + + `this backend compiles no conditional-aggregate (SQL FILTER (WHERE …) / CASE WHEN) expression ` + + `for it, so it is refused rather than silently aggregating the UNFILTERED rows (#10413), which ` + + `is why it answers NOT_IMPLEMENTED/501 rather than a 400. \`engine.aggregate\` lowers filtered ` + + `aggregations in memory for every driver without native support — route the query through the ` + + `engine, or drop the \`filter\` key.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.NOT_IMPLEMENTED; + err.status = 501; + throw err; +} + /** * [#6212] A `groupBy` entry asks for a date BUCKET — the twin of `driver-sql`'s * `refuseDateBucketedGroupBy`, first sentence for first sentence, and the same @@ -1288,6 +1315,14 @@ export class RemoteTransport { // declared; its only writers were the two driver packages' own fixtures. const aggregations = query?.aggregations || []; for (const agg of aggregations) { + // [#10576] A per-aggregation `filter` this transport cannot compile is + // refused before any statement is built — silently emitting the + // UNFILTERED aggregate is the #10413 defect. `{}` is the vacuous filter, + // same convention as `where` / `having`. See + // {@link refusePerAggregationFilter}. + if (agg.filter && Object.keys(agg.filter).length > 0) { + refusePerAggregationFilter(agg.alias ?? agg.field ?? '(unaliased)'); + } // [#5907] The caller's spelling is what the refusal quotes back and what // the declared-vocabulary check is judged against. // diff --git a/packages/objectql/src/engine-aggregate-filter.test.ts b/packages/objectql/src/engine-aggregate-filter.test.ts new file mode 100644 index 0000000000..474a8c6b71 --- /dev/null +++ b/packages/objectql/src/engine-aggregate-filter.test.ts @@ -0,0 +1,297 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `engine.aggregate({ aggregations: [{ …, filter }] })` — ENFORCED since +// #10576, the contract half of #10413's ruling (maintainer, 2026-08-21, +// verbatim 「其他接受」 accepting option A: 「给引擎聚合契约加逐聚合过滤,一次修 +// 对所有驱动」). +// +// The defect being closed: the ObjectQL analytics path handed per-measure +// filters (`stage: 'closed_won'`) toward `engine.aggregate` and the contract +// had nowhere to put them, so "won deals" counted EVERY row — silently, with +// the dashboard door on the same deployment answering the filtered numbers +// (#10413's two-door disagreement). These tests reproduce that measurement's +// shape at the objectql level: the same dataset, the same three measures, and +// the numbers CHANGING once the filter is honoured. +// +// Execution model pinned here (the correct-first two-tier shape date bucketing +// and HAVING use): +// * any aggregation carrying a non-empty `filter` forces the in-memory path +// — no driver compiles a conditional aggregate today, and pushing the +// entry down would aggregate the unfiltered rows; +// * aggregations WITHOUT a filter keep the native pushdown path untouched +// (the widening must not move the existing acceptance face); +// * an unknown operator inside a per-aggregation filter REFUSES with the +// ADR-0112 `INVALID_FILTER`/400 envelope, naming the aggregation position +// — ignoring it would silently answer the unfiltered aggregate, which is +// the very defect this key closes. + +import { describe, it, expect } from 'vitest'; +import type { EngineAggregateOptions } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; + +// The #10413 measurement's dataset shape: opportunities with a stage and an +// amount. 6 rows, 2 closed_won worth 700 total. +const OPPORTUNITIES = [ + { stage: 'closed_won', amount: 500, region: 'east' }, + { stage: 'closed_won', amount: 200, region: 'west' }, + { stage: 'open', amount: 900, region: 'east' }, + { stage: 'open', amount: 300, region: 'west' }, + { stage: 'closed_lost', amount: 50, region: 'east' }, + { stage: 'closed_lost', amount: 20, region: 'west' }, +]; + +/** A driver WITH native aggregate() — counts its calls so the fork is visible. */ +function makeNativeDriver(rows: any[]) { + let nativeAggregateCalls = 0; + let findCalls = 0; + const driver: any = { + name: 'native-agg-mock', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { findCalls += 1; return rows.slice(); }, + async findOne() { return rows[0] ?? null; }, + async create(_o: string, d: any) { return d; }, + async update(_o: string, _id: string, d: any) { return d; }, + async delete() { return true; }, + async count() { return rows.length; }, + async bulkCreate(_o: string, r: any[]) { return r; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + async aggregate(_object: string, ast: any) { + nativeAggregateCalls += 1; + // A native driver that DROPS the per-aggregation filter — the pre-#10576 + // behaviour of every real driver. If the engine ever pushes a filtered + // aggregation down here, the totals below come back unfiltered and the + // reproduction test reads the wrong numbers. + const out: Record = {}; + for (const agg of ast.aggregations ?? []) { + if (agg.function === 'count') out[agg.alias] = rows.length; + if (agg.function === 'sum') out[agg.alias] = rows.reduce((a: number, r: any) => a + r[agg.field], 0); + } + return [out]; + }, + }; + return { driver, nativeCalls: () => nativeAggregateCalls, finds: () => findCalls }; +} + +/** A driver WITHOUT aggregate() — the engine's find() + in-memory lowering. */ +function makeRawDriver(rows: any[]) { + const driver: any = { + name: 'raw-mock', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return rows.slice(); }, + async findOne() { return rows[0] ?? null; }, + async create(_o: string, d: any) { return d; }, + async update(_o: string, _id: string, d: any) { return d; }, + async delete() { return true; }, + async count() { return rows.length; }, + async bulkCreate(_o: string, r: any[]) { return r; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return driver; +} + +async function makeEngine(driver: any) { + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + // Receiver-cast rather than argument-cast: `registerObject`'s later + // parameters are irrelevant to these tests, and the argument-cast spelling + // still bills the package's TEST_DEBT ratchet a TS2554 arity error. + (engine.registry as any).registerObject({ + name: 'crm_opportunity', + fields: { + stage: { type: 'text' }, + amount: { type: 'number' }, + region: { type: 'text' }, + }, + }); + return engine; +} + +// The #10413 reproduction's three measures, lowered to the contract this card +// widens: per-aggregation `filter` on the two "won" measures. +const REPRO_AGGREGATIONS: NonNullable = [ + { function: 'count', alias: 'opp_count' }, + { function: 'count', alias: 'won_count', filter: { stage: 'closed_won' } }, + { function: 'sum', field: 'amount', alias: 'won_amount', filter: { stage: 'closed_won' } }, +]; + +describe('engine.aggregate — per-aggregation filter (#10576, the #10413 contract half)', () => { + it('reproduces #10413: per-measure stage filters reach engine.aggregate and CHANGE the numbers', async () => { + const engine = await makeEngine(makeRawDriver(OPPORTUNITIES)); + + const rows = await engine.aggregate('crm_opportunity', { + aggregations: REPRO_AGGREGATIONS, + } satisfies EngineAggregateOptions); + + // Pre-#10576 (the measured defect): won_count === opp_count === 6 and + // won_amount summed every row (1970). Honoured, the numbers move. + expect(rows).toEqual([{ opp_count: 6, won_count: 2, won_amount: 700 }]); + }); + + it('a filtered aggregation forces the in-memory lowering even on a native-aggregate driver', async () => { + const { driver, nativeCalls, finds } = makeNativeDriver(OPPORTUNITIES); + const engine = await makeEngine(driver); + + const rows = await engine.aggregate('crm_opportunity', { + aggregations: REPRO_AGGREGATIONS, + } satisfies EngineAggregateOptions); + + // The driver's own aggregate() drops the filter (as every real driver + // did), so the ONLY way these numbers are right is that the engine never + // called it: filtered aggregations take find() + in-memory. + expect(nativeCalls()).toBe(0); + expect(finds()).toBe(1); + expect(rows).toEqual([{ opp_count: 6, won_count: 2, won_amount: 700 }]); + }); + + it('positive pin: aggregations WITHOUT filter keep the native pushdown path, results unchanged', async () => { + const { driver, nativeCalls } = makeNativeDriver(OPPORTUNITIES); + const engine = await makeEngine(driver); + + const rows = await engine.aggregate('crm_opportunity', { + aggregations: [ + { function: 'count', alias: 'opp_count' }, + { function: 'sum', field: 'amount', alias: 'total_amount' }, + ], + } satisfies EngineAggregateOptions); + + expect(nativeCalls()).toBe(1); // pushdown exactly as before the widening + expect(rows).toEqual([{ opp_count: 6, total_amount: 1970 }]); + }); + + it('positive pin: an EMPTY filter object is vacuous (same convention as where/having) and does not break pushdown', async () => { + const { driver, nativeCalls } = makeNativeDriver(OPPORTUNITIES); + const engine = await makeEngine(driver); + + const rows = await engine.aggregate('crm_opportunity', { + aggregations: [{ function: 'count', alias: 'opp_count', filter: {} }], + } satisfies EngineAggregateOptions); + + expect(nativeCalls()).toBe(1); + expect(rows).toEqual([{ opp_count: 6 }]); + }); + + it('composes with groupBy: the filter narrows each bucket for ITS aggregation only', async () => { + const engine = await makeEngine(makeRawDriver(OPPORTUNITIES)); + + const rows = await engine.aggregate('crm_opportunity', { + groupBy: ['region'], + aggregations: [ + { function: 'count', alias: 'opp_count' }, + { function: 'sum', field: 'amount', alias: 'won_amount', filter: { stage: 'closed_won' } }, + ], + } satisfies EngineAggregateOptions); + + const byRegion = Object.fromEntries(rows.map((r: any) => [r.region, r])); + expect(byRegion.east).toEqual({ region: 'east', opp_count: 3, won_amount: 500 }); + expect(byRegion.west).toEqual({ region: 'west', opp_count: 3, won_amount: 200 }); + }); + + it('a group the filter empties answers the ruled empty-group values: count/sum 0, avg/min/max null', async () => { + const engine = await makeEngine(makeRawDriver(OPPORTUNITIES)); + + const rows = await engine.aggregate('crm_opportunity', { + groupBy: ['region'], + aggregations: [ + // No row has this stage, so every bucket's filtered set is empty. + { function: 'count', alias: 'n', filter: { stage: 'no_such_stage' } }, + { function: 'sum', field: 'amount', alias: 'total', filter: { stage: 'no_such_stage' } }, + { function: 'avg', field: 'amount', alias: 'mean', filter: { stage: 'no_such_stage' } }, + { function: 'max', field: 'amount', alias: 'top', filter: { stage: 'no_such_stage' } }, + ], + } satisfies EngineAggregateOptions); + + // `emptyGroupValueFor` (spec data/aggregation-policy.ts): counting or + // summing no rows is a measured 0; averaging/maximising them has no answer. + for (const row of rows) { + expect(row.n).toBe(0); + expect(row.total).toBe(0); + expect(row.mean).toBeNull(); + expect(row.top).toBeNull(); + } + }); + + it('the filter composes the where vocabulary ($in, $gte, $and) over source rows', async () => { + const engine = await makeEngine(makeRawDriver(OPPORTUNITIES)); + + const rows = await engine.aggregate('crm_opportunity', { + aggregations: [{ + function: 'count', + alias: 'big_closed', + filter: { $and: [{ stage: { $in: ['closed_won', 'closed_lost'] } }, { amount: { $gte: 50 } }] }, + }], + } satisfies EngineAggregateOptions); + + // closed_won 500, closed_won 200, closed_lost 50 — the 20 is excluded. + expect(rows).toEqual([{ big_closed: 3 }]); + }); + + it('an unknown operator in a per-aggregation filter REFUSES with INVALID_FILTER/400, naming the position', async () => { + const engine = await makeEngine(makeRawDriver(OPPORTUNITIES)); + + let thrown: (Error & { code?: string; status?: number }) | undefined; + try { + await engine.aggregate('crm_opportunity', { + aggregations: [ + { function: 'count', alias: 'opp_count' }, + { function: 'count', alias: 'bad', filter: { amount: { $median: 3 } } }, + ], + } as unknown as EngineAggregateOptions); + } catch (e) { + thrown = e as Error & { code?: string; status?: number }; + } + + // The named envelope, not a bare throw (#6142/#6050: a suite that only + // asserts THREW stays green while the envelope is missing). + expect(thrown).toBeDefined(); + expect(thrown!.code).toBe('INVALID_FILTER'); + expect(thrown!.status).toBe(400); + expect(thrown!.message).toMatch(/Unsupported operator '\$median' in `aggregations\[1\]\.filter`/); + expect(thrown!.message).toMatch(/refused rather than ignored/); + }); + + it('a retired operator ($regex) in a per-aggregation filter gets the retirement prescription, same envelope', async () => { + const engine = await makeEngine(makeRawDriver(OPPORTUNITIES)); + + let thrown: (Error & { code?: string; status?: number }) | undefined; + try { + await engine.aggregate('crm_opportunity', { + aggregations: [{ function: 'count', alias: 'bad', filter: { stage: { $regex: 'won' } } }], + } as unknown as EngineAggregateOptions); + } catch (e) { + thrown = e as Error & { code?: string; status?: number }; + } + + expect(thrown).toBeDefined(); + expect(thrown!.code).toBe('INVALID_FILTER'); + expect(thrown!.status).toBe(400); + expect(thrown!.message).toMatch(/Filter operator '\$regex' in `aggregations\[0\]\.filter` is RETIRED/); + }); + + it('the comparand-shape door covers the new filter position: a scalar $in is refused before any driver runs', async () => { + const engine = await makeEngine(makeRawDriver(OPPORTUNITIES)); + + let thrown: (Error & { code?: string; status?: number }) | undefined; + try { + await engine.aggregate('crm_opportunity', { + aggregations: [{ function: 'count', alias: 'bad', filter: { stage: { $in: 'closed_won' } } }], + } as unknown as EngineAggregateOptions); + } catch (e) { + thrown = e as Error & { code?: string; status?: number }; + } + + expect(thrown).toBeDefined(); + expect(thrown!.code).toBe('INVALID_FILTER'); + expect(thrown!.status).toBe(400); + // The path names WHICH aggregation carries the offending comparand. + expect(thrown!.message).toContain('aggregations[0].filter'); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 35119baa82..40da9584dd 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -11543,6 +11543,20 @@ export class ObjectQL implements IObjectQLEngine { rejectUnknownEngineOptions(object, 'aggregate', query, ENGINE_AGGREGATE_OPTION_KEYS); query = lowerWhereFilterArray(object, 'aggregate', query, this._registry.getObject(object)); this.rejectCredentialAggregation(object, query); + // [#10576] The per-aggregation `filter` (`AggregationNodeSchema.filter`, + // the contract half of #10413) is a second filter position on this verb, + // so it walks through the same refusal doors `where` does at this seam: + // the comparand-shape gate (#5869 — a scalar `$in` etc. is refused, not + // silently matched against nothing) and the materializable-field gate + // (#8296 — a typo'd column would otherwise select ZERO rows for that one + // aggregation, silently, which is the same wrong-number shape #10413 + // measured). The path names which aggregation carries the offending key. + for (const [i, agg] of (Array.isArray(query.aggregations) ? query.aggregations : []).entries()) { + const aggFilter = (agg as { filter?: unknown })?.filter; + if (aggFilter == null) continue; + assertListComparandShapes(object, 'aggregate', aggFilter, `aggregations[${i}].filter`); + assertFilterIsMaterializable(object, 'aggregate', this._registry.getObject(object), aggFilter); + } const driver = this.getDriver(object); this.logger.debug(`Aggregate on ${object} using ${driver.name}`, query); @@ -11567,6 +11581,25 @@ export class ObjectQL implements IObjectQLEngine { context: mergeReadContext(query?.context, options?.context), }; this.resolveWhereTokens(opCtx.ast as QueryAST, opCtx.context); + // [#10576] Filter tokens (`{userId}`-style placeholders, #3810) resolve + // in per-aggregation filters exactly as they do in `where` — a filter + // position is a filter position, and an unresolved placeholder would be + // compared as a literal string, silently selecting zero rows for that one + // aggregation. Copy-on-write like `withResolvedWhere`: the entry objects + // belong to the caller (view metadata / flow config get reused), so a + // resolved filter lands on a copied entry, never written back through. + { + const astAggs = (opCtx.ast as QueryAST).aggregations; + if (Array.isArray(astAggs) && astAggs.some((a) => (a as { filter?: unknown })?.filter != null)) { + const tokenCtx = filterTokenContextFrom(opCtx.context); + (opCtx.ast as QueryAST).aggregations = astAggs.map((a) => { + const f = (a as { filter?: unknown })?.filter; + if (f == null) return a; + const resolved = resolveFilterTokens(f as any, tokenCtx); + return resolved === f ? a : { ...(a as object), filter: resolved } as typeof a; + }); + } + } await this.executeWithMiddleware(opCtx, async () => { const ast = opCtx.ast as QueryAST; @@ -11600,7 +11633,23 @@ export class ObjectQL implements IObjectQLEngine { const tz = query.timezone; const hasDateBucket = structuredItems.some((g) => !!g?.dateGranularity); const tzRequiresInMemory = !!tz && tz !== 'UTC' && hasDateBucket; - if (typeof drv.aggregate === 'function' && allStructuredSupported && !tzRequiresInMemory) { + // [#10576] Per-aggregation filters (`aggregations[].filter`, the + // contract half of #10413) force the in-memory path: no driver compiles + // a conditional aggregate (SQL `FILTER (WHERE …)` / `CASE WHEN`) from + // this key today, and pushing it down would aggregate the UNFILTERED + // rows — the silent drop #10413 measured. Same correct-first two-tier + // shape as date bucketing and HAVING above: a driver that grows native + // support must advertise a capability flag, at which point this fork + // becomes its fallback tier. The driver faces also refuse the key + // loudly (NOT_IMPLEMENTED/501) if reached directly, so neither seam can + // drop it in silence. `{}` counts as no filter (the vacuous-filter + // convention `where` / `having` already follow). + const hasAggregationFilter = (Array.isArray(ast.aggregations) ? ast.aggregations : []) + .some((a) => { + const f = (a as { filter?: unknown })?.filter; + return f != null && typeof f === 'object' && Object.keys(f).length > 0; + }); + if (typeof drv.aggregate === 'function' && allStructuredSupported && !tzRequiresInMemory && !hasAggregationFilter) { // HAVING is engine-owned (#4286): applied AFTER aggregation, over // the aggregated row's own columns (aggregation aliases + groupBy // projections), identically on both paths. No driver implements it diff --git a/packages/objectql/src/having-filter.ts b/packages/objectql/src/having-filter.ts index 144f6b8f2f..afd1278a07 100644 --- a/packages/objectql/src/having-filter.ts +++ b/packages/objectql/src/having-filter.ts @@ -62,6 +62,47 @@ import { invalidFilterError } from './filter-comparand-shape.js'; const LOGICAL_OPERATORS = ['$and', '$or', '$not'] as const; +/** + * [#10576] Which clause this evaluator is judging — the wording seam that lets + * one walker serve two positions honestly. + * + * This module was written for `having` and its refusals said so in prose + * ("Unsupported operator '$x' in `having`. HAVING filters the aggregated + * rows…"). The per-aggregation `filter` (`AggregationNodeSchema.filter`, the + * contract half of #10413) evaluates the SAME operator vocabulary with the + * same #5298 null semantics, but over a different row population — the raw + * source rows of one aggregation, not the aggregated result — so reusing the + * walker verbatim would refuse an aggregation-filter mistake with a sentence + * about a clause the caller never wrote. The clause carries the two strings + * that differ; everything the two positions genuinely share (operators, + * envelope, null semantics, comparand gates) stays single-sourced. + */ +interface FilterClause { + /** Root label refusals use for position (`having`, `aggregations[2].filter`). */ + root: string; + /** One-sentence semantics printed before the supported-operator list. */ + semantics: string; +} + +const HAVING_CLAUSE: FilterClause = { + root: 'having', + semantics: 'HAVING filters the aggregated rows (aggregation aliases + groupBy projections)', +}; + +/** + * [#10576] The clause for one `aggregations[i].filter` position. The position + * index is baked into `root` so a refusal names WHICH aggregation carries the + * offending key — a call can hold several, each with its own filter. + */ +export function aggregationFilterClause(index: number): FilterClause { + return { + root: `aggregations[${index}].filter`, + semantics: + 'A per-aggregation `filter` narrows the SOURCE rows that one aggregation reads ' + + '(raw column namespace, the `where` operator vocabulary)', + }; +} + // [#5702] `$regex` is GONE from this vocabulary. Its arm below ran a real // `RegExp` over the aggregated value and answered an ILLEGAL pattern with // `return false` — "no rows", silently — which is the pair of defects #4706 @@ -116,6 +157,7 @@ function unknownOperator( op: string, where: 'logical' | 'condition', siblings: readonly string[] = [], + clause: FilterClause = HAVING_CLAUSE, ): Error { // [#5702] A RETIRED spelling gets the spec's prescription rather than the // vocabulary list — its author wrote a name this face ANSWERED until #4706, @@ -131,7 +173,7 @@ function unknownOperator( + `whole shape, so this is ONE mistake with ONE fix, not one per key.` : ''; return invalidFilterError( - `Filter operator '${op}' in \`having\` is RETIRED and is no longer evaluated.${replacement} ` + `Filter operator '${op}' in \`${clause.root}\` is RETIRED and is no longer evaluated.${replacement} ` + `${retired.why}${also}`, ); } @@ -139,8 +181,7 @@ function unknownOperator( ? `${LOGICAL_OPERATORS.join(', ')} (or a column condition)` : CONDITION_OPERATORS.join(', '); return invalidFilterError( - `Unsupported operator '${op}' in \`having\`. HAVING filters the aggregated rows ` - + `(aggregation aliases + groupBy projections) and supports: ${supported}. ` + `Unsupported operator '${op}' in \`${clause.root}\`. ${clause.semantics} and supports: ${supported}. ` + `An unknown operator is refused rather than ignored — ignoring it would silently ` + `return unfiltered aggregates (#4286, ADR-0078).`, ); @@ -230,34 +271,70 @@ export function applyHaving(rows: any[], having: FilterCondition | null | undefi * refusal can NAME where the offending key sits. It defaults, so this stays the * two-argument function every existing caller (and `applyHaving` below) uses. */ -export function matchesHaving(row: Record, cond: any, path = 'having'): boolean { +export function matchesHaving( + row: Record, + cond: any, + path = 'having', + clause: FilterClause = HAVING_CLAUSE, +): boolean { if (!cond || typeof cond !== 'object') return true; for (const [key, value] of Object.entries(cond)) { const here = `${path}.${key}`; if (key === '$and') { const branches = Array.isArray(value) ? value : [value]; - if (!branches.every((c, i) => matchesHaving(row, c, `${here}[${i}]`))) return false; + if (!branches.every((c, i) => matchesHaving(row, c, `${here}[${i}]`, clause))) return false; continue; } if (key === '$or') { const branches = Array.isArray(value) ? value : [value]; - if (!branches.some((c, i) => matchesHaving(row, c, `${here}[${i}]`))) return false; + if (!branches.some((c, i) => matchesHaving(row, c, `${here}[${i}]`, clause))) return false; continue; } if (key === '$not') { - if (matchesHaving(row, value, here)) return false; + if (matchesHaving(row, value, here, clause)) return false; continue; } - if (key.startsWith('$')) throw unknownOperator(key, 'logical'); + if (key.startsWith('$')) throw unknownOperator(key, 'logical', [], clause); // Aggregated rows are flat (aliases + group projections) — direct access, - // no dotted-path resolution. - if (!checkCondition(row?.[key], value, key, here)) return false; + // no dotted-path resolution. [#10576] The per-aggregation filter walks the + // same way on purpose: it reads `driver.find()` rows, which are flat too. + if (!checkCondition(row?.[key], value, key, here, clause)) return false; } return true; } +/** + * [#10576] Evaluate one raw source row against one `aggregations[i].filter` + * predicate — the per-aggregation filter of `AggregationNodeSchema` (the + * contract half of #10413's ruling), applied by the in-memory aggregation + * fallback before the aggregation function reads the row. + * + * The SAME walker as `matchesHaving`, deliberately: the two positions share + * one operator vocabulary, one ADR-0112 refusal envelope, and the #5298 + * null-safe negation semantics — a predicate moved between a driver `where`, + * a `having`, and a per-aggregation `filter` must select rows by one rule. + * Only the refusal WORDING differs (see {@link aggregationFilterClause}): an + * unknown operator here is refused naming the aggregation position it sits in, + * because ignoring it would silently answer the UNFILTERED aggregate — the + * precise #10413 defect this key exists to close. + */ +export function matchesAggregationFilter( + row: Record, + filter: FilterCondition, + index: number, +): boolean { + const clause = aggregationFilterClause(index); + return matchesHaving(row, filter, clause.root, clause); +} + /** One column's condition — implicit equality or an operator object. */ -function checkCondition(value: any, condition: any, field: string, path: string): boolean { +function checkCondition( + value: any, + condition: any, + field: string, + path: string, + clause: FilterClause = HAVING_CLAUSE, +): boolean { // Implicit equality (primitives, null, Date, array exact-match) — loose `==` // to mirror the Filter Protocol's memory evaluation. if ( @@ -355,7 +432,7 @@ function checkCondition(value: any, condition: any, field: string, path: string) // answered as "this row does not match", i.e. a silent empty result rather // than an error. Retired by #4706; refused by `default:` below. default: - throw unknownOperator(op, 'condition', keys); + throw unknownOperator(op, 'condition', keys, clause); } } return true; diff --git a/packages/objectql/src/in-memory-aggregation.test.ts b/packages/objectql/src/in-memory-aggregation.test.ts index cb7b5d210d..7c87b76f77 100644 --- a/packages/objectql/src/in-memory-aggregation.test.ts +++ b/packages/objectql/src/in-memory-aggregation.test.ts @@ -400,3 +400,95 @@ describe('count-all `*` sentinel (regression #1982)', () => { ]); }); }); + +// [#10576] Per-aggregation `filter` — the contract half of #10413. This module +// is the documented LOWERING for every driver: `engine.aggregate` routes any +// call whose aggregations carry a filter through here, so these pure-function +// pins are what "one fix, all drivers" rests on. +describe('applyInMemoryAggregation — per-aggregation filter (#10576)', () => { + const opps = [ + { stage: 'closed_won', amount: 500, region: 'east' }, + { stage: 'closed_won', amount: 200, region: 'west' }, + { stage: 'open', amount: 900, region: 'east' }, + { stage: 'closed_lost', amount: 50, region: 'west' }, + ]; + + it('narrows the source rows for the ONE aggregation carrying the filter; siblings see every row', () => { + const out = applyInMemoryAggregation(opps, { + aggregations: [ + { function: 'count', alias: 'opp_count' }, + { function: 'count', alias: 'won_count', filter: { stage: 'closed_won' } }, + { function: 'sum', field: 'amount', alias: 'won_amount', filter: { stage: 'closed_won' } }, + ], + } as any); + expect(out).toEqual([{ opp_count: 4, won_count: 2, won_amount: 700 }]); + }); + + it('applies per bucket under groupBy', () => { + const out = applyInMemoryAggregation(opps, { + groupBy: ['region'], + aggregations: [ + { function: 'count', alias: 'n' }, + { function: 'sum', field: 'amount', alias: 'won', filter: { stage: 'closed_won' } }, + ], + } as any).sort((a, b) => String(a.region).localeCompare(String(b.region))); + expect(out).toEqual([ + { region: 'east', n: 2, won: 500 }, + { region: 'west', n: 2, won: 200 }, + ]); + }); + + it('an empty filter object is vacuous — byte-identical to no filter', () => { + const filtered = applyInMemoryAggregation(opps, { + aggregations: [{ function: 'count', alias: 'n', filter: {} }], + } as any); + const bare = applyInMemoryAggregation(opps, { + aggregations: [{ function: 'count', alias: 'n' }], + } as any); + expect(filtered).toEqual(bare); + }); + + it('a filter that excludes every row answers the ruled empty-group values (aggregation-policy.ts)', () => { + const out = applyInMemoryAggregation(opps, { + aggregations: [ + { function: 'count', alias: 'n', filter: { stage: 'nope' } }, + { function: 'count_distinct', field: 'stage', alias: 'kinds', filter: { stage: 'nope' } }, + { function: 'sum', field: 'amount', alias: 'total', filter: { stage: 'nope' } }, + { function: 'avg', field: 'amount', alias: 'mean', filter: { stage: 'nope' } }, + { function: 'min', field: 'amount', alias: 'lo', filter: { stage: 'nope' } }, + { function: 'max', field: 'amount', alias: 'hi', filter: { stage: 'nope' } }, + ], + } as any); + expect(out).toEqual([{ n: 0, kinds: 0, total: 0, mean: null, lo: null, hi: null }]); + }); + + it('an unknown operator REFUSES with the ADR-0112 envelope, naming the aggregation position', () => { + let thrown: (Error & { code?: string; status?: number }) | undefined; + try { + applyInMemoryAggregation(opps, { + aggregations: [ + { function: 'count', alias: 'ok' }, + { function: 'count', alias: 'bad', filter: { amount: { $median: 3 } } }, + ], + } as any); + } catch (e) { + thrown = e as Error & { code?: string; status?: number }; + } + expect(thrown).toBeDefined(); + expect(thrown!.code).toBe('INVALID_FILTER'); + expect(thrown!.status).toBe(400); + expect(thrown!.message).toMatch(/Unsupported operator '\$median' in `aggregations\[1\]\.filter`/); + }); + + it('null-safe negation (#5298 semantics): $ne is satisfied by a row whose column has no value', () => { + const withNulls = [ + { stage: 'closed_won', amount: 10 }, + { stage: null, amount: 20 }, + { amount: 30 }, + ]; + const out = applyInMemoryAggregation(withNulls, { + aggregations: [{ function: 'sum', field: 'amount', alias: 'not_won', filter: { stage: { $ne: 'closed_won' } } }], + } as any); + expect(out).toEqual([{ not_won: 50 }]); + }); +}); diff --git a/packages/objectql/src/in-memory-aggregation.ts b/packages/objectql/src/in-memory-aggregation.ts index c3483092e7..8ccc36d053 100644 --- a/packages/objectql/src/in-memory-aggregation.ts +++ b/packages/objectql/src/in-memory-aggregation.ts @@ -32,10 +32,23 @@ // dedupe limb is deleted rather than left unreachable, for the same reason // the retired function arms were. `count_distinct` is unaffected: it // deduplicates inside its own arm, and every SQL face compiles it (#6409). -// * `filter: FilterCondition` on aggregations is **not** evaluated here — -// the engine routes filtered aggregations through the driver where -// possible; the in-memory fallback ignores the per-aggregation filter and -// logs a warning if one is present. +// * `filter: FilterCondition` on aggregations IS evaluated here — ENFORCED +// since #10576 (the contract half of #10413: the ObjectQL analytics path +// handed per-measure filters to `engine.aggregate` and they were dropped +// with no error, so "won deals" counted every row). The predicate narrows +// the SOURCE rows the one aggregation reads — SQL `FILTER (WHERE …)` +// semantics — while sibling aggregations in the same call keep the full +// bucket. Evaluation is `matchesAggregationFilter` (having-filter.ts): the +// same operator vocabulary, ADR-0112 refusal envelope and #5298 null-safe +// negation as `having`, over raw rows instead of aggregated ones; an +// unknown operator REFUSES rather than silently answering the unfiltered +// aggregate. This module used to claim it "logs a warning if one is +// present" — it never did; the key was silently ignored on every path, +// which is exactly the defect #10413 measured. NOTE this fallback is the +// documented LOWERING for every driver: `engine.aggregate` routes any call +// whose aggregations carry a filter through here (no driver advertises +// native conditional aggregation today), and the driver faces refuse +// NOT_IMPLEMENTED/501 if reached directly with one. // // Date bucketing uses ISO-8601 conventions (weeks start Monday). // @@ -73,6 +86,7 @@ import { calendarPartsInTzOrUtc } from '@objectstack/core'; import type { QueryAST, GroupByNode, AggregationNode, DateGranularityValue } from '@objectstack/spec/data'; +import { matchesAggregationFilter } from './having-filter.js'; /** * Group + aggregate raw rows according to the AST's `groupBy` / @@ -153,11 +167,24 @@ function projectGroupValue(row: any, g: GroupByNode, timezone?: string): unknown return v ?? null; } -function aggregateBucket(rows: any[], aggregations: AggregationNode[]): Record { +function aggregateBucket(allRows: any[], aggregations: AggregationNode[]): Record { const out: Record = {}; - for (const agg of aggregations) { + for (const [index, agg] of aggregations.entries()) { const alias = agg.alias; const fn = agg.function; + // [#10576] Per-aggregation filter — SQL `FILTER (WHERE …)` semantics: THIS + // aggregation reads only the bucket rows its predicate selects; siblings + // keep the full bucket. `{}` is the vacuous filter (same convention as + // `where` / `having`). When the predicate excludes every row of a bucket + // the arms below already answer the platform's ruled empty-group values — + // count/count_distinct/sum → 0, avg/min/max → null — matching + // `emptyGroupValueFor` (spec data/aggregation-policy.ts), which exists for + // precisely this case (a measure-scoped filter emptying a group the grid + // still lists, objectui#3136). + const aggFilter = agg.filter; + const rows = aggFilter && Object.keys(aggFilter).length > 0 + ? allRows.filter((row) => matchesAggregationFilter(row, aggFilter, index)) + : allRows; if (fn === 'count') { // `*` is the count-all sentinel: the Cube `count` measure and a dataset // `count` with no field both compile to `sql: '*'` (→ SQL `COUNT(*)`). diff --git a/packages/spec/liveness/query.json b/packages/spec/liveness/query.json index 98daa82bc1..fcff03ea75 100644 --- a/packages/spec/liveness/query.json +++ b/packages/spec/liveness/query.json @@ -60,8 +60,14 @@ "note": "REMOVED 2026-07-31 (#4286) — tombstoned at the schema (retiredKey carries the prescription; authoring it is a tsc error and a parse error). No conversion strips it: QueryAST is a request surface, never stored in stack metadata, so the removal is the protocol-17 semantic migration `query-joins-retired`. The entry stays because retiredKey keeps the key in the walked shape (the rls.priority precedent). Related records are read through `expand` (batch $in resolution); the orphaned JoinNode/JoinType/JoinStrategy cluster left with the key." }, "aggregations": { - "note": "Container. `filter` resolves `experimental` from its `[EXPERIMENTAL — not enforced]` describe marker (#4286 — a SQL FILTER (WHERE …) affordance neither the SQL builders nor the in-memory fallback applies; the FLS predicate guard walks it, predicate-guard.ts:89).", + "note": "Container. `filter` was `experimental` (resolved from its `[EXPERIMENTAL — not enforced]` describe marker, #4286) until #10576 took ADR-0049's enforce leg on it — see its own child entry below.", "children": { + "filter": { + "status": "live", + "verifiedAt": "2026-08-21", + "evidence": "packages/objectql/src/in-memory-aggregation.ts aggregateBucket narrows the source rows per aggregation via matchesAggregationFilter (having-filter.ts); packages/objectql/src/engine.ts aggregate() forces the in-memory lowering when any aggregation carries a non-empty filter; the four native driver faces refuse the key NOT_IMPLEMENTED/501 when reached directly (driver-sql sql-driver.ts, driver-turso remote-transport.ts, driver-mongodb mongodb-aggregation.ts, driver-memory memory-driver.ts performAggregation); the FLS predicate guard walks it (plugin-security predicate-guard.ts:89); pinned by packages/objectql/src/engine-aggregate-filter.test.ts", + "note": "ENFORCED 2026-08-21 (#10576, the contract half of #10413's ruling — maintainer 「其他接受」 accepting option A 「给引擎聚合契约加逐聚合过滤,一次修对所有驱动」). SQL FILTER (WHERE …) semantics: the predicate narrows the SOURCE rows the one aggregation reads while siblings keep the full group. Engine-owned via the in-memory lowering (the dateGranularity/having correct-first two-tier shape — a driver that grows native conditional aggregation must advertise a capability flag); a driver face reached directly refuses rather than silently aggregating the unfiltered rows, which was #10413's measured defect on the ObjectQL analytics path." + }, "function": { "status": "live", "verifiedAt": "2026-07-31", diff --git a/packages/spec/liveness/state-counts.md b/packages/spec/liveness/state-counts.md index f681ad3f08..357175c96d 100644 --- a/packages/spec/liveness/state-counts.md +++ b/packages/spec/liveness/state-counts.md @@ -43,7 +43,7 @@ for both corollaries. | `report` | 21 | 0 | 0 | 0 | 21 | | `dashboard` | 34 | 0 | 7 | 0 | 41 | | `webhook` | 19 | 0 | 0 | 0 | 19 | -| `query` | 15 | 1 | 5 | 0 | 21 | +| `query` | 16 | 0 | 5 | 0 | 21 | | `datasource` | 30 | 0 | 0 | 0 | 30 | | `app` | 47 | 0 | 9 | 0 | 56 | | `book` | 20 | 0 | 1 | 0 | 21 | @@ -57,4 +57,4 @@ for both corollaries. | `api` | 25 | 0 | 0 | 2 | 27 | | `capability` | 12 | 0 | 0 | 0 | 12 | | `qa` | 4 | 0 | 5 | 0 | 9 | -| **total** | **797** | **6** | **55** | **11** | **869** | +| **total** | **798** | **5** | **55** | **11** | **869** | diff --git a/packages/spec/src/contracts/analytics-service.ts b/packages/spec/src/contracts/analytics-service.ts index 1f87905315..cabd3884f3 100644 --- a/packages/spec/src/contracts/analytics-service.ts +++ b/packages/spec/src/contracts/analytics-service.ts @@ -285,7 +285,19 @@ export interface StrategyContext { */ executeAggregate?(objectName: string, options: { groupBy?: string[]; - aggregations?: Array<{ field: string; method: string; alias: string }>; + /** + * One entry per aggregate to compute. `filter` (#10576, the contract + * half of #10413's ruling) is a per-aggregation predicate over the + * SOURCE rows — SQL `FILTER (WHERE …)` semantics — so a strategy can + * lower a measure-scoped filter (`stage: 'closed_won'`) into the one + * aggregation it belongs to instead of dropping it (the #10413 silent + * drop) or scoping the WHOLE call via the sibling `filter` below. + * Bridges forward it to `engine.aggregate`'s `aggregations[].filter` + * (`AggregationNodeSchema.filter`), which the engine honours on every + * driver by lowering in memory when the driver has no native + * conditional aggregation. + */ + aggregations?: Array<{ field: string; method: string; alias: string; filter?: FilterCondition }>; filter?: Record; /** * Reference timezone (IANA name) for date bucketing (ADR-0053 Phase 2). diff --git a/packages/spec/src/data/query.zod.ts b/packages/spec/src/data/query.zod.ts index 386a0eeb0b..8e8ffe15b2 100644 --- a/packages/spec/src/data/query.zod.ts +++ b/packages/spec/src/data/query.zod.ts @@ -269,7 +269,20 @@ export const AggregationNodeSchema = lazySchema(() => z.object({ * every face computes. See {@link AGGREGATION_DISTINCT_REMOVED}. */ distinct: retiredKey(AGGREGATION_DISTINCT_REMOVED), - filter: FilterConditionSchema.optional().describe('[EXPERIMENTAL — not enforced] Per-aggregation filter (SQL FILTER (WHERE …)). Neither the SQL builders nor the in-memory fallback applies it (#4286); filter the whole query with `where` instead.'), + /** + * Per-aggregation filter (SQL `FILTER (WHERE …)` semantics) — ENFORCED since + * #10576 (the contract half of #10413's ruling: 「给引擎聚合契约加逐聚合过滤, + * 一次修对所有驱动」). The predicate narrows the SOURCE rows this one + * aggregation reads — raw column namespace, the `where` operator vocabulary — + * while sibling aggregations in the same call keep seeing every row of the + * group. `engine.aggregate` lowers filtered aggregations in memory for every + * driver (correct-first, the date-bucketing two-tier shape); a driver face + * reached directly with one refuses NOT_IMPLEMENTED/501 rather than silently + * aggregating the unfiltered rows — the silent drop was #10413's defect. + * Over a group whose rows the filter excludes entirely, count/sum answer 0 + * and avg/min/max answer null (`emptyGroupValueFor`, aggregation-policy.ts). + */ + filter: FilterConditionSchema.optional().describe('Per-aggregation filter (SQL FILTER (WHERE …) semantics): narrows the source rows THIS aggregation reads, leaving sibling aggregations unfiltered. Enforced by engine.aggregate (#10576): lowered in memory for drivers without native conditional aggregation; a driver reached directly refuses rather than silently dropping it.'), })); // ─── Joins: REMOVED (#4286, ADR-0049) ────────────────────────────────────────