Skip to content

Commit fe84352

Browse files
authored
fix(fumadb): mirror aggregate review hardening (#62)
## Summary Mirrors the review-hardening deltas from upstream [RhysSullivan#1119](RhysSullivan#1119) onto `dev`. `dev` already contains the broader FumaDB aggregate and keyset query feature, so this PR only carries the remaining drift from the upstream review loop. ## Changes - Preserve memory and Drizzle parity for empty composite filters by compiling empty JSON `or` filters to a constant false SQL predicate. - Add regression coverage for empty `or` and empty `and` filters across the aggregate harness. - Replace nested plugin-storage operator selection with an explicit typed JSON compare-operator map. - Document SQLite percentile behavior on the public FumaDB and plugin-storage stats inputs. ## Intentional Differences From Upstream RhysSullivan#1119 - This PR does not re-add the full aggregate and keyset implementation because `dev` already has that feature surface. - This PR does not touch the OpenAPI storage facade mock because `dev` already has the mock shape needed by the expanded collection facade. - This PR has no changeset because it only mirrors fixes to an existing `dev` feature surface. ## Tests - `bun run bootstrap` - `bun run --cwd packages/core/fumadb test -- src/query/aggregate.test.ts src/query/table-policy.test.ts` - `bun run --cwd packages/core/sdk test -- src/plugin-storage-aggregate.test.ts src/plugin-storage.test.ts` - `bun run --cwd packages/core/fumadb typecheck` - `bun run --cwd packages/core/sdk typecheck` - `bun run typecheck` - `./node_modules/.bin/oxfmt --check packages/core/fumadb/src/adapters/drizzle/query.ts packages/core/fumadb/src/query/aggregate.test.ts packages/core/fumadb/src/query/aggregate.ts packages/core/sdk/src/executor.ts packages/core/sdk/src/plugin-storage.ts`
1 parent 50bed06 commit fe84352

5 files changed

Lines changed: 56 additions & 11 deletions

File tree

packages/core/fumadb/src/adapters/drizzle/query.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@ export function fromDrizzle(
362362
);
363363
}
364364
if (filter.kind === "or") {
365+
if (filter.items.length === 0) return Drizzle.sql`1 = 0`;
365366
return Drizzle.or(
366367
...filter.items.map((item) => buildJsonFilter(jsonColumn, item)),
367368
);

packages/core/fumadb/src/query/aggregate.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,24 @@ const runSuite = (name: string, makeHarness: () => Promise<Harness>) => {
128128
).toBe(2);
129129
});
130130

131+
it("matches empty composite filter semantics", async () => {
132+
const { orm } = harness;
133+
expect(
134+
await orm.jsonCount("events", {
135+
column: "data",
136+
where: inT1,
137+
filter: { kind: "or", items: [] },
138+
}),
139+
).toBe(0);
140+
expect(
141+
await orm.jsonCount("events", {
142+
column: "data",
143+
where: inT1,
144+
filter: { kind: "and", items: [] },
145+
}),
146+
).toBe(4);
147+
});
148+
131149
it("jsonGroupCount counts distinct values", async () => {
132150
const rows = await harness.orm.jsonGroupCount("events", {
133151
column: "data",

packages/core/fumadb/src/query/aggregate.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,13 @@ export interface JsonTimeBucketRow {
9191
export interface JsonStatsAdapterOptions extends JsonAdapterBase {
9292
/** Numeric path the stats are computed over. */
9393
readonly path: JsonPath;
94-
/** Percentile fractions in [0, 1] (e.g. 0.5, 0.95). */
94+
/**
95+
* Percentile fractions in [0, 1] (e.g. 0.5, 0.95).
96+
*
97+
* SQLite adapters compute percentiles from all matching numeric values after
98+
* projection because SQLite has no native percentile aggregate. Count, min,
99+
* and max still run as SQL aggregates.
100+
*/
95101
readonly percentiles?: readonly number[];
96102
}
97103

@@ -155,6 +161,13 @@ export interface JsonTimeBucketOptions<TColumns extends Record<string, AnyColumn
155161
export interface JsonStatsOptions<TColumns extends Record<string, AnyColumn>>
156162
extends JsonPublicBase<TColumns> {
157163
readonly path: JsonPath;
164+
/**
165+
* Percentile fractions in [0, 1] (e.g. 0.5, 0.95).
166+
*
167+
* SQLite adapters compute percentiles from all matching numeric values after
168+
* projection because SQLite has no native percentile aggregate. Count, min,
169+
* and max still run as SQL aggregates.
170+
*/
158171
readonly percentiles?: readonly number[];
159172
}
160173

packages/core/sdk/src/executor.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
withQueryContext,
88
type Condition,
99
type ConditionBuilder,
10+
type JsonCompareOperator,
1011
type JsonFilter,
1112
type JsonGroupCountRow,
1213
type JsonKeysetCursor,
@@ -1116,6 +1117,17 @@ const isPluginStorageRecord = (value: unknown): value is Readonly<Record<string,
11161117

11171118
const pluginStorageWhereOperators = ["eq", "in", "gt", "gte", "lt", "lte"] as const;
11181119

1120+
const pluginStorageJsonCompareOperators = {
1121+
eq: "=",
1122+
gt: ">",
1123+
gte: ">=",
1124+
lt: "<",
1125+
lte: "<=",
1126+
} satisfies Record<
1127+
Exclude<(typeof pluginStorageWhereOperators)[number], "in">,
1128+
JsonCompareOperator
1129+
>;
1130+
11191131
const isPluginStorageWhereFilter = (value: unknown): value is Readonly<Record<string, unknown>> =>
11201132
isPluginStorageRecord(value) && pluginStorageWhereOperators.some((operator) => operator in value);
11211133

@@ -1212,16 +1224,11 @@ const pluginStorageWhereToJsonFilter = (
12121224
});
12131225
continue;
12141226
}
1227+
if (!(operator in pluginStorageJsonCompareOperators)) continue;
12151228
const compareOperator =
1216-
operator === "gt"
1217-
? ">"
1218-
: operator === "gte"
1219-
? ">="
1220-
: operator === "lt"
1221-
? "<"
1222-
: operator === "lte"
1223-
? "<="
1224-
: "=";
1229+
pluginStorageJsonCompareOperators[
1230+
operator as keyof typeof pluginStorageJsonCompareOperators
1231+
];
12251232
items.push({
12261233
kind: "compare",
12271234
path,

packages/core/sdk/src/plugin-storage.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,13 @@ export interface PluginStorageStatsInput<
225225
> extends PluginStorageAggregateFilter<TDefinition> {
226226
/** Numeric field the stats are computed over. */
227227
readonly field: PluginStorageCollectionIndexedField<TDefinition>;
228-
/** Percentile fractions in [0, 1] (e.g. 0.5, 0.95). */
228+
/**
229+
* Percentile fractions in [0, 1] (e.g. 0.5, 0.95).
230+
*
231+
* SQLite-backed storage computes percentiles from all matching numeric values
232+
* after projection because SQLite has no native percentile aggregate. Count,
233+
* min, and max still run as SQL aggregates.
234+
*/
229235
readonly percentiles?: readonly number[];
230236
}
231237

0 commit comments

Comments
 (0)