diff --git a/.changeset/plugin-storage-postgres-jsonb.md b/.changeset/plugin-storage-postgres-jsonb.md new file mode 100644 index 0000000000..9eac796720 --- /dev/null +++ b/.changeset/plugin-storage-postgres-jsonb.md @@ -0,0 +1,7 @@ +--- +"emdash": patch +--- + +Fixes plugin storage `query()`, `count()`, ordering, and index creation on Postgres. Queries and unique/expression indexes on stored JSON fields previously failed with an "operator does not exist" error; numeric range/equality/`in` filters compared values as text (so `stock >= 10` also matched `9`), causing over-counting; and `orderBy` on a numeric field sorted lexically (`10, 100, 9`) instead of numerically. Numeric guards now compare and order numerically, and counts are returned as numbers. + +Numeric filters are now type-guarded on both dialects: a stored value that isn't a JSON number is excluded from a numeric comparison (evaluates to no-match) instead of being compared as text — previously such a value could match, and on Postgres an untyped cast would have thrown. Numeric predicates on Postgres are served by a sequential scan (the per-field expression index is text-typed, since field types aren't known when indexes are created). SQLite behavior is otherwise unchanged. diff --git a/packages/core/src/database/dialect-helpers.ts b/packages/core/src/database/dialect-helpers.ts index da81aabe0c..55b11b85d0 100644 --- a/packages/core/src/database/dialect-helpers.ts +++ b/packages/core/src/database/dialect-helpers.ts @@ -229,3 +229,74 @@ export function jsonExtractExpr(db: Kysely, column: string, path: string): } return `json_extract(${column}, '$.${path}')`; } + +/** + * SQL expression for extracting a field from the plugin-storage `data` column. + * + * Unlike `jsonExtractExpr` (for real `json`/`jsonb` content columns), + * `_plugin_storage.data` is a plain `text` column. On Postgres the JSON + * operator `->>` has no overload for `text` — `text ->> 'x'` raises + * `operator does not exist: text ->> unknown` — so the column must be cast to + * `jsonb` first. The extracted value is still `text`, so a numeric comparison + * (`stock >= 10`) would compare lexically (`'9' >= '10'` is TRUE) and silently + * over-count / oversell; pass `{ numeric: true }` for a numeric comparison. + * + * The numeric form is a **type-guarded** cast, not a bare `::numeric`. A bare + * cast throws `invalid input syntax for type numeric` on Postgres the moment a + * single scanned row stores a non-number in that field (documents are + * schemaless), aborting the whole query — and it would diverge from SQLite, + * which silently coerces. Guarding with `jsonb_typeof`/`json_type` makes the + * comparison **total and parity-correct on both dialects**: a non-number + * stored value yields `NULL` (no match) instead of an error. + * + * The field name is validated (`/^[a-zA-Z][a-zA-Z0-9_]*$/`) before + * interpolation, so the casts wrap only a safe identifier and add no injection + * surface. + * + * sqlite text: json_extract(data, '$.field') + * sqlite numeric: CASE WHEN json_type(data, '$.field') IN ('integer', 'real') + * THEN json_extract(data, '$.field') END + * postgres text: (data::jsonb)->>'field' + * postgres numeric: CASE WHEN jsonb_typeof((data::jsonb)->'field') = 'number' + * THEN ((data::jsonb)->>'field')::numeric END + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- accepts any Kysely instance +export function pluginDataExtractExpr( + db: Kysely, + field: string, + options?: { numeric?: boolean }, +): string { + validateJsonFieldName(field, "plugin storage field name"); + if (isPostgres(db)) { + const text = `(data::jsonb)->>'${field}'`; + if (!options?.numeric) return text; + return `CASE WHEN jsonb_typeof((data::jsonb)->'${field}') = 'number' THEN (${text})::numeric END`; + } + const extract = `json_extract(data, '$.${field}')`; + if (!options?.numeric) return extract; + return `CASE WHEN json_type(data, '$.${field}') IN ('integer', 'real') THEN ${extract} END`; +} + +/** + * SQL expression for ordering plugin-storage rows by a `data` field. + * + * `ORDER BY` has no bound operand to infer numeric-vs-text from, so extracting + * as text (`->>'field'`) would sort a numeric field lexically on Postgres + * (`[10, 100, 9]`) while SQLite's `json_extract` sorts it numerically — + * a cross-dialect divergence. Ordering over the **jsonb-native value** + * (`->'field'`, note the single arrow) fixes this: the jsonb btree ordering is + * numeric among numbers, lexical among strings, and total across heterogeneous + * values (it never throws). SQLite's `json_extract` already orders numerically, + * so it is unchanged. + * + * sqlite: json_extract(data, '$.field') + * postgres: (data::jsonb)->'field' + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- accepts any Kysely instance +export function pluginDataOrderExpr(db: Kysely, field: string): string { + validateJsonFieldName(field, "plugin storage order field name"); + if (isPostgres(db)) { + return `(data::jsonb)->'${field}'`; + } + return `json_extract(data, '$.${field}')`; +} diff --git a/packages/core/src/database/repositories/plugin-storage.ts b/packages/core/src/database/repositories/plugin-storage.ts index 7815285fea..e27eb16a39 100644 --- a/packages/core/src/database/repositories/plugin-storage.ts +++ b/packages/core/src/database/repositories/plugin-storage.ts @@ -7,7 +7,7 @@ * @see PLUGIN-SYSTEM.md § Plugin Storage > Full API Reference */ -import type { Kysely } from "kysely"; +import type { Kysely, RawBuilder, SqlBool } from "kysely"; import { sql } from "kysely"; import { @@ -15,7 +15,7 @@ import { validateWhereClause, validateOrderByClause, getIndexedFields, - jsonExtract, + jsonOrderExtract, } from "../../plugins/storage-query.js"; import type { StorageCollection, @@ -27,6 +27,37 @@ import { withTransaction } from "../transaction.js"; import type { Database } from "../types.js"; import { encodeCursor, decodeCursor } from "./types.js"; +/** + * Turn a `buildWhereClause` result (`?`-placeholder SQL + ordered params) into a + * single boolean expression suitable for Kysely's `.where()`. + * + * The `?` placeholders are spliced back into value fragments (`sql`${param}``, + * which parameterizes safely) interleaved with the raw SQL between them. The + * whole condition is returned as a boolean expression and passed directly to + * `.where()` — it must NOT be wrapped in an `= 1` comparison. Postgres parses + * ` = 1` as a chained comparison (`a >= $1 = 1`), a syntax error, and even + * parenthesized `(a >= $1) = 1` is `boolean = integer`, which Postgres rejects. + * A bare boolean expression is valid on both SQLite and Postgres. + */ +function buildRawWhereExpression(whereResult: { + sql: string; + params: unknown[]; +}): RawBuilder { + const parts: RawBuilder[] = []; + let paramIndex = 0; + const sqlParts = whereResult.sql.split("?"); + for (let i = 0; i < sqlParts.length; i++) { + if (i > 0) { + parts.push(sql`${whereResult.params[paramIndex++]}`); + } + const chunk = sqlParts[i]; + if (chunk) { + parts.push(sql.raw(chunk)); + } + } + return sql`${sql.join(parts, sql.raw(""))}`; +} + /** * Plugin Storage Repository * @@ -211,19 +242,7 @@ export class PluginStorageRepository implements StorageCollection[] = []; - let paramIndex = 0; - const sqlParts = whereResult.sql.split("?"); - for (let i = 0; i < sqlParts.length; i++) { - if (i > 0) { - whereSqlParts.push(sql`${whereResult.params[paramIndex++]}`); - } - if (sqlParts[i]) { - whereSqlParts.push(sql.raw(sqlParts[i])); - } - } - query = query.where(({ eb }) => eb(sql.join(whereSqlParts, sql.raw("")), "=", sql.raw("1"))); + query = query.where(buildRawWhereExpression(whereResult)); } // Handle cursor-based pagination — throws on invalid cursor. @@ -237,7 +256,9 @@ export class PluginStorageRepository implements StorageCollection 0) { for (const [field, direction] of Object.entries(orderBy)) { - const extract = jsonExtract(this.db, field); + // Order over the jsonb-native value on Postgres so numeric fields sort + // numerically, not lexically. See pluginDataOrderExpr. + const extract = jsonOrderExtract(this.db, field); const orderExpr = direction === "desc" ? sql`${sql.raw(extract)} desc` : sql`${sql.raw(extract)} asc`; query = query.orderBy(orderExpr); @@ -289,26 +310,14 @@ export class PluginStorageRepository implements StorageCollection 0) { const whereResult = buildWhereClause(this.db, where); if (whereResult.sql) { - // Use sql template to add the raw WHERE conditions with params - const whereSqlParts: ReturnType[] = []; - let paramIndex = 0; - const sqlParts = whereResult.sql.split("?"); - for (let i = 0; i < sqlParts.length; i++) { - if (i > 0) { - whereSqlParts.push(sql`${whereResult.params[paramIndex++]}`); - } - if (sqlParts[i]) { - whereSqlParts.push(sql.raw(sqlParts[i])); - } - } - query = query.where(({ eb }) => - eb(sql.join(whereSqlParts, sql.raw("")), "=", sql.raw("1")), - ); + query = query.where(buildRawWhereExpression(whereResult)); } } const result = await query.executeTakeFirst(); - return result?.count ?? 0; + // Postgres returns COUNT(*) (bigint) as a string via node-postgres; coerce + // so this always satisfies its Promise contract on both dialects. + return Number(result?.count ?? 0); } } diff --git a/packages/core/src/plugins/storage-indexes.ts b/packages/core/src/plugins/storage-indexes.ts index bfb32ea875..43ec2e4eb5 100644 --- a/packages/core/src/plugins/storage-indexes.ts +++ b/packages/core/src/plugins/storage-indexes.ts @@ -9,7 +9,7 @@ import type { Kysely, RawBuilder } from "kysely"; import { sql } from "kysely"; -import { jsonExtractExpr, isPostgres } from "../database/dialect-helpers.js"; +import { pluginDataExtractExpr, isPostgres } from "../database/dialect-helpers.js"; import type { Database } from "../database/types.js"; import { validateIdentifier, @@ -58,13 +58,25 @@ export function generateCreateIndexSql( // Build the indexed expressions // Fields are validated above, safe to interpolate into json path + // + // The index expression is TEXT (`->>`), matching equality/startsWith/text + // predicates. Field TYPES are not known at index-creation time — the plugin + // manifest declares field names, not types — so we cannot build a numeric + // index speculatively. Consequence on Postgres: a numeric predicate compiles + // to a type-guarded `::numeric` expression (see pluginDataExtractExpr) that a + // text btree can't satisfy, so numeric range/equality guards fall back to a + // sequential scan. Acceptable for the small per-plugin/collection document + // sets this store targets; revisit with a typed-field manifest if needed. const expressions = fields .map((field) => { + // Index the extracted value as text (no numeric cast): uniqueness is + // defined on the textual JSON value, and on Postgres the `data` column + // is `text`, so pluginDataExtractExpr adds the required `::jsonb` cast. if (isPostgres(db)) { // Postgres expression indexes need parens around the expression - return `(${jsonExtractExpr(db, "data", field)})`; + return `(${pluginDataExtractExpr(db, field)})`; } - return jsonExtractExpr(db, "data", field); + return pluginDataExtractExpr(db, field); }) .join(", "); diff --git a/packages/core/src/plugins/storage-query.ts b/packages/core/src/plugins/storage-query.ts index c1744c193e..4d765114c2 100644 --- a/packages/core/src/plugins/storage-query.ts +++ b/packages/core/src/plugins/storage-query.ts @@ -8,8 +8,7 @@ import type { Kysely } from "kysely"; -import { jsonExtractExpr } from "../database/dialect-helpers.js"; -import { validateJsonFieldName } from "../database/validate.js"; +import { pluginDataExtractExpr, pluginDataOrderExpr } from "../database/dialect-helpers.js"; import type { WhereClause, WhereValue, RangeFilter, InFilter, StartsWithFilter } from "./types.js"; /** @@ -117,15 +116,33 @@ export function validateOrderByClause( } /** - * SQL expression for extracting JSON field. + * SQL expression for extracting a queryable field from the `_plugin_storage.data` + * column. * - * Validates the field name before interpolation to prevent SQL injection - * via crafted JSON path expressions. + * Delegates to `pluginDataExtractExpr`, which validates the field name before + * interpolation and applies the dialect-correct extraction: a `::jsonb` cast on + * Postgres (the `data` column is `text`) plus an optional `::numeric` cast so + * numeric comparisons don't fall back to lexical text ordering. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any -- accepts any Kysely instance -export function jsonExtract(db: Kysely, field: string): string { - validateJsonFieldName(field, "query field name"); - return jsonExtractExpr(db, "data", field); +export function jsonExtract( + db: Kysely, + field: string, + options?: { numeric?: boolean }, +): string { + return pluginDataExtractExpr(db, field, options); +} + +/** + * SQL expression for ordering by a `_plugin_storage.data` field. + * + * Delegates to `pluginDataOrderExpr`, which orders over the jsonb-native value + * on Postgres so numeric fields sort numerically (not lexically) while staying + * total across heterogeneous data. SQLite keeps `json_extract` (already numeric). + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- accepts any Kysely instance +export function jsonOrderExtract(db: Kysely, field: string): string { + return pluginDataOrderExpr(db, field); } /** @@ -137,33 +154,44 @@ export function buildCondition( field: string, value: WhereValue, ): { sql: string; params: unknown[] } { - const extract = jsonExtract(db, field); + // Numeric-vs-text is decided per condition from the JS type of the bound + // value. On Postgres a text extract compared to a bound number would sort + // lexically (`'9' >= '10'` is TRUE); a `::numeric` cast on the extract fixes + // it. String/boolean operands keep text comparison. SQLite is unaffected — + // `json_extract` already returns a typed value. + const extractFor = (numeric: boolean): string => jsonExtract(db, field, { numeric }); if (value === null) { - return { sql: `${extract} IS NULL`, params: [] }; + return { sql: `${extractFor(false)} IS NULL`, params: [] }; } - if (typeof value === "string" || typeof value === "number") { - return { sql: `${extract} = ?`, params: [value] }; + if (typeof value === "number") { + return { sql: `${extractFor(true)} = ?`, params: [value] }; + } + + if (typeof value === "string") { + return { sql: `${extractFor(false)} = ?`, params: [value] }; } if (typeof value === "boolean") { // JSON booleans are stored as true/false strings - return { sql: `${extract} = ?`, params: [value] }; + return { sql: `${extractFor(false)} = ?`, params: [value] }; } if (isInFilter(value)) { + const numeric = value.in.length > 0 && value.in.every((v) => typeof v === "number"); const placeholders = value.in.map(() => "?").join(", "); return { - sql: `${extract} IN (${placeholders})`, + sql: `${extractFor(numeric)} IN (${placeholders})`, params: value.in, }; } if (isStartsWithFilter(value)) { - // ESCAPE '\' works on both SQLite and PostgreSQL. + // ESCAPE '\' works on both SQLite and PostgreSQL. startsWith is a string + // operation, so always compare as text. return { - sql: `${extract} LIKE ? ESCAPE '\\'`, + sql: `${extractFor(false)} LIKE ? ESCAPE '\\'`, params: [`${escapeLikePattern(value.startsWith)}%`], }; } @@ -172,22 +200,17 @@ export function buildCondition( const conditions: string[] = []; const params: unknown[] = []; - if (value.gt !== undefined) { - conditions.push(`${extract} > ?`); - params.push(value.gt); - } - if (value.gte !== undefined) { - conditions.push(`${extract} >= ?`); - params.push(value.gte); - } - if (value.lt !== undefined) { - conditions.push(`${extract} < ?`); - params.push(value.lt); - } - if (value.lte !== undefined) { - conditions.push(`${extract} <= ?`); - params.push(value.lte); - } + // Each bound is cast to numeric only when its own operand is a number, so + // a mixed range (e.g. a string lower bound) stays correct per side. + const pushBound = (op: string, bound: string | number): void => { + conditions.push(`${extractFor(typeof bound === "number")} ${op} ?`); + params.push(bound); + }; + + if (value.gt !== undefined) pushBound(">", value.gt); + if (value.gte !== undefined) pushBound(">=", value.gte); + if (value.lt !== undefined) pushBound("<", value.lt); + if (value.lte !== undefined) pushBound("<=", value.lte); return { sql: conditions.join(" AND "), @@ -239,7 +262,7 @@ export function buildOrderByClause( const clauses: string[] = []; for (const [field, direction] of Object.entries(orderBy)) { - clauses.push(`${jsonExtract(db, field)} ${direction.toUpperCase()}`); + clauses.push(`${jsonOrderExtract(db, field)} ${direction.toUpperCase()}`); } if (clauses.length === 0) { diff --git a/packages/core/tests/integration/plugins/storage-postgres-query.test.ts b/packages/core/tests/integration/plugins/storage-postgres-query.test.ts new file mode 100644 index 0000000000..32f2af22d9 --- /dev/null +++ b/packages/core/tests/integration/plugins/storage-postgres-query.test.ts @@ -0,0 +1,244 @@ +/** + * Plugin storage query/count/index correctness across dialects. + * + * `_plugin_storage.data` is a plain `text` column. On Postgres the JSON + * operator `->>` needs an explicit `::jsonb` cast (`text ->> 'x'` is a parse + * error), and the extracted value is `text` — so a numeric range guard would + * compare lexically (`'9' >= '10'` is TRUE) and over-count / oversell. + * + * These assertions run on SQLite (always) and Postgres (when EMDASH_TEST_PG is + * set). The numeric-range case is the load-bearing one: it fails on Postgres + * before the jsonb + numeric-cast fix and passes after, while SQLite — whose + * `json_extract` already returns a typed numeric — stays green throughout. + */ + +import type { Kysely } from "kysely"; +import { it, expect, beforeEach, afterEach } from "vitest"; + +import { PluginStorageRepository } from "../../../src/database/repositories/plugin-storage.js"; +import type { Database } from "../../../src/database/types.js"; +import { createStorageIndexes } from "../../../src/plugins/storage-indexes.js"; +import { + describeEachDialect, + setupForDialect, + teardownForDialect, + type DialectTestContext, +} from "../../utils/test-db.js"; + +interface Product { + sku: string; + stock: number; + tier: number; + name: string; +} + +describeEachDialect("Plugin storage query correctness", (dialect) => { + let ctx: DialectTestContext; + let db: Kysely; + + beforeEach(async () => { + ctx = await setupForDialect(dialect); + db = ctx.db; + }); + + afterEach(async () => { + await teardownForDialect(ctx); + }); + + function productsRepo(): PluginStorageRepository { + return new PluginStorageRepository(db, "shop", "products", [ + "sku", + "stock", + "tier", + "name", + ]); + } + + async function seedProducts(): Promise> { + const repo = productsRepo(); + await repo.putMany([ + { id: "p9", data: { sku: "A9", stock: 9, tier: 1, name: "Alpha" } }, + { id: "p10", data: { sku: "B10", stock: 10, tier: 2, name: "Bravo" } }, + { id: "p100", data: { sku: "C100", stock: 100, tier: 3, name: "Charlie" } }, + ]); + return repo; + } + + it("equality guard on a string field returns the right rows", async () => { + const repo = await seedProducts(); + const result = await repo.query({ where: { sku: "B10" } }); + expect(result.items.map((i) => i.id)).toEqual(["p10"]); + }); + + it("numeric RangeFilter compares numerically across the lexical boundary", async () => { + // '9' >= '10' is TRUE lexically but false numerically. A correct + // implementation returns exactly {10, 100}, never 9 — and, ordered + // numerically, in the exact sequence [10, 100] (not lexical [10, 100] + // happens to coincide here, so assert the returned order directly, no + // .toSorted() masking). + const repo = await seedProducts(); + const result = await repo.query({ where: { stock: { gte: 10 } }, orderBy: { stock: "asc" } }); + expect(result.items.map((i) => i.id)).toEqual(["p10", "p100"]); + expect(result.items.map((i) => i.data.stock)).toEqual([10, 100]); + }); + + it("orderBy on a numeric field sorts numerically, not lexically", async () => { + // Lexically the ids/values sort as [10, 100, 9]; numerically [9, 10, 100]. + // This is the orderBy-specific defect: text extraction sorts wrong on PG. + const repo = await seedProducts(); + const asc = await repo.query({ orderBy: { stock: "asc" } }); + expect(asc.items.map((i) => i.data.stock)).toEqual([9, 10, 100]); + expect(asc.items.map((i) => i.id)).toEqual(["p9", "p10", "p100"]); + + const desc = await repo.query({ orderBy: { stock: "desc" } }); + expect(desc.items.map((i) => i.data.stock)).toEqual([100, 10, 9]); + }); + + it("numeric RangeFilter with an upper bound stays numeric", async () => { + const repo = await seedProducts(); + // lexically '100' < '9' and '100' < '10'; numerically 100 is largest. + const result = await repo.query({ where: { stock: { lt: 100 } } }); + expect(result.items.map((i) => i.id).toSorted()).toEqual(["p10", "p9"]); + }); + + it("in filter with numeric values compares numerically", async () => { + const repo = await seedProducts(); + const result = await repo.query({ where: { stock: { in: [9, 100] } } }); + expect(result.items.map((i) => i.id).toSorted()).toEqual(["p100", "p9"]); + }); + + it("startsWith on a string field matches by prefix", async () => { + const repo = await seedProducts(); + const result = await repo.query({ where: { sku: { startsWith: "C" } } }); + expect(result.items.map((i) => i.id)).toEqual(["p100"]); + }); + + it("count with a numeric range guard is numerically correct", async () => { + const repo = await seedProducts(); + // Lexical comparison would count 9 as >= 10 and return 3. + expect(await repo.count({ stock: { gte: 10 } })).toBe(2); + }); + + it("count with an in filter of numeric values is correct", async () => { + const repo = await seedProducts(); + expect(await repo.count({ stock: { in: [10, 100] } })).toBe(2); + }); + + it("creates a UNIQUE index on a declared field and enforces uniqueness", async () => { + // Proves the index-expression path parses on Postgres (the `->>` on a + // text column would otherwise raise "operator does not exist"). + const result = await createStorageIndexes(db, "shop", "products", [], { + uniqueIndexes: ["sku"], + }); + expect(result.errors).toEqual([]); + expect(result.created).toContain("uidx_plugin_shop_products_sku"); + + const repo = productsRepo(); + await repo.put("first", { sku: "DUP", stock: 1, tier: 1, name: "First" }); + await expect( + repo.put("second", { sku: "DUP", stock: 2, tier: 1, name: "Second" }), + ).rejects.toThrow(); + }); + + it("creates a non-unique expression index that queries parse against", async () => { + const result = await createStorageIndexes(db, "shop", "products", ["stock"]); + expect(result.errors).toEqual([]); + expect(result.created).toContain("idx_plugin_shop_products_stock"); + + const repo = await seedProducts(); + // Query should run against the indexed expression without a parse error. + expect(await repo.count({ stock: { gte: 10 } })).toBe(2); + }); + + it("orderBy on an indexed field parses and returns all rows", async () => { + const repo = await seedProducts(); + const result = await repo.query({ orderBy: { name: "asc" } }); + expect(result.items.map((i) => i.data.name)).toEqual(["Alpha", "Bravo", "Charlie"]); + }); + + it("stores the data column as plain text (not json/jsonb)", async () => { + // Guards the premise of the bug: data is text, so PG needs the ::jsonb cast. + await seedProducts(); + const row = await db + .selectFrom("_plugin_storage") + .select("data") + .where("id", "=", "p9") + .executeTakeFirstOrThrow(); + expect(typeof row.data).toBe("string"); + expect(JSON.parse(row.data)).toMatchObject({ stock: 9 }); + }); + + it("numeric guard excludes non-number stored values without throwing", async () => { + // A schemaless store can hold a string where a numeric guard is applied. + // A bare ::numeric cast throws on Postgres; the type-guarded expression + // must return the same result on both dialects: only the numeric row. + const repo = productsRepo(); + await repo.putMany([ + { id: "num", data: { sku: "N", stock: 5, tier: 1, name: "Numeric" } }, + // stock is a string here — must be excluded, not error. + { id: "str", data: { sku: "S", stock: "abc" as unknown as number, tier: 1, name: "Str" } }, + ]); + + const rows = await repo.query({ where: { stock: { gte: 1 } } }); + expect(rows.items.map((i) => i.id)).toEqual(["num"]); + expect(await repo.count({ stock: { gte: 1 } })).toBe(1); + }); + + it("boolean equality matches stored JSON booleans", async () => { + // On Postgres the extract is text ('true'/'false'); the bound JS boolean + // must still match. better-sqlite3 rejects boolean bind params entirely + // (a pre-existing limitation of the boolean path, unrelated to this fix), + // so this is only exercisable on Postgres. + if (dialect !== "postgres") return; + + const repo = new PluginStorageRepository<{ active: boolean; name: string }>( + db, + "shop", + "flags", + ["active"], + ); + await repo.putMany([ + { id: "on", data: { active: true, name: "On" } }, + { id: "off", data: { active: false, name: "Off" } }, + ]); + + expect((await repo.query({ where: { active: true } })).items.map((i) => i.id)).toEqual(["on"]); + expect((await repo.query({ where: { active: false } })).items.map((i) => i.id)).toEqual([ + "off", + ]); + }); + + it("numeric guards handle negative, zero, and float values numerically", async () => { + const repo = productsRepo(); + await repo.putMany([ + { id: "neg", data: { sku: "NEG", stock: -5, tier: 1, name: "Neg" } }, + { id: "zero", data: { sku: "ZERO", stock: 0, tier: 1, name: "Zero" } }, + { id: "frac", data: { sku: "FRAC", stock: 3.5, tier: 1, name: "Frac" } }, + { id: "ten", data: { sku: "TEN", stock: 10, tier: 1, name: "Ten" } }, + ]); + + const nonNegative = await repo.query({ + where: { stock: { gte: 0 } }, + orderBy: { stock: "asc" }, + }); + expect(nonNegative.items.map((i) => i.data.stock)).toEqual([0, 3.5, 10]); + + expect((await repo.query({ where: { stock: { lt: 0 } } })).items.map((i) => i.id)).toEqual([ + "neg", + ]); + + const fractional = await repo.query({ where: { stock: { gt: 3, lt: 4 } } }); + expect(fractional.items.map((i) => i.id)).toEqual(["frac"]); + }); + + it("null field filter matches rows whose stored value is JSON null", async () => { + const repo = productsRepo(); + await repo.putMany([ + { id: "hasNull", data: { sku: "X", stock: null as unknown as number, tier: 1, name: "N" } }, + { id: "hasValue", data: { sku: "Y", stock: 5, tier: 1, name: "V" } }, + ]); + + const result = await repo.query({ where: { stock: null } }); + expect(result.items.map((i) => i.id)).toEqual(["hasNull"]); + }); +}); diff --git a/packages/core/tests/unit/plugins/storage-query.test.ts b/packages/core/tests/unit/plugins/storage-query.test.ts index 61a949f59f..25ed89a579 100644 --- a/packages/core/tests/unit/plugins/storage-query.test.ts +++ b/packages/core/tests/unit/plugins/storage-query.test.ts @@ -18,6 +18,12 @@ import { createTestDatabase } from "../../utils/test-db.js"; describe("storage-query", () => { const db = createTestDatabase(); + + // SQLite type-guarded numeric extraction. Numeric comparisons wrap the + // extract in a CASE so a non-number stored value yields NULL (no match) + // instead of coercing — keeping parity with the Postgres numeric guard. + const numExpr = (field: string): string => + `CASE WHEN json_type(data, '$.${field}') IN ('integer', 'real') THEN json_extract(data, '$.${field}') END`; describe("type guards", () => { describe("isRangeFilter", () => { it("should return true for range filters with gt", () => { @@ -211,7 +217,7 @@ describe("storage-query", () => { it("should handle number values", () => { const result = buildCondition(db, "count", 42); - expect(result.sql).toBe("json_extract(data, '$.count') = ?"); + expect(result.sql).toBe(`${numExpr("count")} = ?`); expect(result.params).toEqual([42]); }); @@ -227,6 +233,19 @@ describe("storage-query", () => { expect(result.params).toEqual(["a", "b", "c"]); }); + it("should compare numerically for all-number IN filters", () => { + const result = buildCondition(db, "count", { in: [1, 2, 3] }); + expect(result.sql).toBe(`${numExpr("count")} IN (?, ?, ?)`); + expect(result.params).toEqual([1, 2, 3]); + }); + + it("should fall back to text comparison for mixed-type IN filters", () => { + // A single non-number element pins the whole list to text comparison. + const result = buildCondition(db, "tag", { in: [10, "x"] }); + expect(result.sql).toBe("json_extract(data, '$.tag') IN (?, ?)"); + expect(result.params).toEqual([10, "x"]); + }); + it("should handle startsWith filters", () => { const result = buildCondition(db, "name", { startsWith: "foo" }); expect(result.sql).toBe("json_extract(data, '$.name') LIKE ? ESCAPE '\\'"); @@ -241,35 +260,40 @@ describe("storage-query", () => { it("should handle range filters with gt", () => { const result = buildCondition(db, "age", { gt: 18 }); - expect(result.sql).toBe("json_extract(data, '$.age') > ?"); + expect(result.sql).toBe(`${numExpr("age")} > ?`); expect(result.params).toEqual([18]); }); it("should handle range filters with gte", () => { const result = buildCondition(db, "age", { gte: 18 }); - expect(result.sql).toBe("json_extract(data, '$.age') >= ?"); + expect(result.sql).toBe(`${numExpr("age")} >= ?`); expect(result.params).toEqual([18]); }); it("should handle range filters with lt", () => { const result = buildCondition(db, "age", { lt: 65 }); - expect(result.sql).toBe("json_extract(data, '$.age') < ?"); + expect(result.sql).toBe(`${numExpr("age")} < ?`); expect(result.params).toEqual([65]); }); it("should handle range filters with lte", () => { const result = buildCondition(db, "age", { lte: 65 }); - expect(result.sql).toBe("json_extract(data, '$.age') <= ?"); + expect(result.sql).toBe(`${numExpr("age")} <= ?`); expect(result.params).toEqual([65]); }); it("should handle combined range filters", () => { const result = buildCondition(db, "age", { gte: 18, lt: 65 }); - expect(result.sql).toBe( - "json_extract(data, '$.age') >= ? AND json_extract(data, '$.age') < ?", - ); + expect(result.sql).toBe(`${numExpr("age")} >= ? AND ${numExpr("age")} < ?`); expect(result.params).toEqual([18, 65]); }); + + it("should keep text comparison for string range bounds", () => { + // A string bound (e.g. ISO date) stays textual per side. + const result = buildCondition(db, "createdAt", { gte: "2024-01-01" }); + expect(result.sql).toBe("json_extract(data, '$.createdAt') >= ?"); + expect(result.params).toEqual(["2024-01-01"]); + }); }); describe("buildWhereClause", () => {