diff --git a/CHANGELOG.md b/CHANGELOG.md index e6a8794..ea3aa6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ Notable changes to `@tinbase/pg-mem`, the tinbase fork of pg-mem. Released from `main`, which carries the scoped package name. Upstream is tracked through the `upstream` remote (`oguimbal/pg-mem`) rather than a branch; the leftover `master` is vestigial. +## 4.0.4 + +From mutation probes and seeded reads over 120 production projects (validator on PGlite vs pg-mem). + +- **Wrong results, fixed:** `sum()` over numeric/bigint concatenated the digit strings (`sum` of 10 and 32.5 was `'1032.5'`), and `avg` was computed from that; `max`/`min` compared numeric/bigint as text (`max(9, 10)` was `9`). Now exact. Enums ordered alphabetically instead of by declaration (ORDER BY, `>`/`<`, and `max`/`min`, which now accept enums). +- plpgsql: double-quoted identifiers are one token (`new."updated_at" := now()` in a trigger, quoted names in function bodies). +- JSON: dates and times in postgres' format (`"2026-05-26"`, `"2026-05-26T10:30:00"`, `"...+00:00"`), and numerics as numbers, in `row_to_json`, `json_agg`, `to_json[b]`, `json[b]_build_object`, `json[b]_build_array`. +- Postgres' wording for errors the migration validator shows: duplicate column (`column "x" of relation "t" already exists`), NOT NULL (names the relation), policy predicate type, and syntax errors (`syntax error at or near "x"` / `at end of input`, code 42601). + ## 4.0.3 - A user column named after a system column (`tableoid`, `xmin`, `cmin`, `xmax`, `cmax`, `ctid`) is refused in CREATE TABLE, ADD COLUMN and RENAME COLUMN, as postgres does: `column name "xmin" conflicts with a system column name`. The migration validator accepted DDL postgres rejects. diff --git a/package.json b/package.json index dececcf..0881963 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tinbase/pg-mem", - "version": "4.0.3", + "version": "4.0.4", "description": "Fork of pg-mem with extended Postgres conformance (PL/pgSQL, triggers, RLS, correlated subqueries, MERGE, ranges, full-text, partitioning, ...). Tracks oguimbal/pg-mem; pending upstream PR #476.", "main": "index.js", "publishConfig": { diff --git a/src/column.ts b/src/column.ts index 57871d8..7a11027 100644 --- a/src/column.ts +++ b/src/column.ts @@ -132,7 +132,7 @@ export class ColRef implements _Column { rename(to: string, t: _Transaction): this { if (this.table.getColumnRef(to, true)) { - throw new QueryError(`Column "${to}" already exists`); + throw new QueryError(`column "${to}" of relation "${this.table.name}" already exists`, '42701'); } assertNotSystemColumn(to); @@ -258,7 +258,7 @@ export class ColRef implements _Column { return; } if (nullIsh(col)) { - throw new QueryError(`null value in column "${this.expression.id}" violates not-null constraint`); + throw new QueryError(`null value in column "${this.expression.id}" of relation "${this.table.name}" violates not-null constraint`, '23502'); } } diff --git a/src/datatypes/json-numbers.ts b/src/datatypes/json-numbers.ts index ea9c422..3ea940a 100644 --- a/src/datatypes/json-numbers.ts +++ b/src/datatypes/json-numbers.ts @@ -29,6 +29,19 @@ export function toJsonValue(value: any, type: _IType | nil): any { return value; } + // dates and times are json strings in postgres' own text format, not JS's toISOString() + // (`row_to_json` of a date column is "2026-05-26", not "2026-05-26T00:00:00.000Z") + if (value instanceof Date && !isNaN(value.getTime())) { + switch (type.primary) { + case DataType.date: + return value.toISOString().slice(0, 10); + case DataType.timestamp: + return pgIsoTime(value); + case DataType.timestamptz: + return pgIsoTime(value) + '+00:00'; + } + } + if (JSON_NUMBER_TYPES.has(type.primary)) { if (typeof value === 'string') { const n = Number(value); @@ -65,3 +78,10 @@ export function toJsonValue(value: any, type: _IType | nil): any { return value; } + +/** YYYY-MM-DDTHH:MM:SS[.fff] in UTC, fractional seconds only when non-zero (postgres' json format) */ +function pgIsoTime(d: Date): string { + const iso = d.toISOString(); // YYYY-MM-DDTHH:MM:SS.mmmZ + const ms = iso.slice(20, 23).replace(/0+$/, ''); + return iso.slice(0, 19) + (ms ? '.' + ms : ''); +} diff --git a/src/datatypes/t-custom-enum.ts b/src/datatypes/t-custom-enum.ts index 2a647ee..6559c18 100644 --- a/src/datatypes/t-custom-enum.ts +++ b/src/datatypes/t-custom-enum.ts @@ -57,6 +57,16 @@ export class CustomEnumType extends TypeBase { } + // an enum orders by its values' declaration order, not alphabetically + // ('low' < 'medium' < 'high' for `create type prio as enum ('low', 'medium', 'high')`) + doGt(a: string, b: string): boolean { + return this.values.indexOf(a) > this.values.indexOf(b); + } + + doLt(a: string, b: string): boolean { + return this.values.indexOf(a) < this.values.indexOf(b); + } + drop(t: _Transaction): void { this.schema._unregisterType(this); } diff --git a/src/execution/plpgsql.ts b/src/execution/plpgsql.ts index 4f30e9c..d08d2a7 100644 --- a/src/execution/plpgsql.ts +++ b/src/execution/plpgsql.ts @@ -48,7 +48,7 @@ function tokenize(code: string): string[] { // first so its content — which may contain ; ' etc. — is not split). JS regex // backreferences let us balance the tag, which the moo lexer can't do. const raw = stripComments(code) - .match(/\$([a-zA-Z_]\w*)?\$[\s\S]*?\$\1\$|'(?:[^']|'')*'|\d+\.\d+|\d+|\.\.|:=|::|[+\-*/<>=~!@#%^&|`?]+|[a-zA-Z_][\w$]*|[(),.;]|[^\s]/g) ?? []; + .match(/\$([a-zA-Z_]\w*)?\$[\s\S]*?\$\1\$|'(?:[^']|'')*'|"(?:[^"]|"")*"|\d+\.\d+|\d+|\.\.|:=|::|[+\-*/<>=~!@#%^&|`?]+|[a-zA-Z_][\w$]*|[(),.;]|[^\s]/g) ?? []; return raw.flatMap(splitOperator); } @@ -80,6 +80,16 @@ function unquote(s: string): string { return s; } +/** a quoted identifier token without its quotes (doubled quotes inside become one) */ +function unquoteIdent(tok: string): string { + return tok.slice(1, -1).replace(/""/g, '"'); +} + +/** a variable name as a token: bare when it is a plain lower-case identifier, quoted otherwise */ +function identToken(name: string): string { + return /^[a-z_][a-z0-9_$]*$/.test(name) ? name : `"${name.replace(/"/g, '""')}"`; +} + function joinTokens(toks: string[]): string { let out = ''; for (let i = 0; i < toks.length; i++) { @@ -344,10 +354,11 @@ class GParser { return this.parseBlockBody(); default: { // assignment: name := expr | name = expr - const name = this.next(); + const tok = this.next(); + const name = tok.startsWith('"') ? unquoteIdent(tok) : tok; const op = this.peek(); if (op !== ':=' && op !== '=') { - throw new NotSupported(`plpgsql statement starting with "${name}"`); + throw new NotSupported(`plpgsql statement starting with "${tok}"`); } this.i++; const expr = this.readUntil([';'], true); @@ -709,8 +720,10 @@ function mangleTrigger(toks: string[]): string[] { i += 3; continue; } - if ((lt === 'new' || lt === 'old') && toks[i + 1] === '.' && /^[a-zA-Z_]/.test(toks[i + 2] ?? '')) { - out.push((lt === 'new' ? '__n_' : '__o_') + toks[i + 2].toLowerCase()); + if ((lt === 'new' || lt === 'old') && toks[i + 1] === '.' && /^[a-zA-Z_"]/.test(toks[i + 2] ?? '')) { + // NEW.col folds to lower case; NEW."Col" keeps its case, as a column name does + const col = toks[i + 2].startsWith('"') ? unquoteIdent(toks[i + 2]) : toks[i + 2].toLowerCase(); + out.push(identToken((lt === 'new' ? '__n_' : '__o_') + col)); i += 2; } else if (lt === 'new') { out.push(`'__trg_new__'`); diff --git a/src/execution/schema-amends/alter.ts b/src/execution/schema-amends/alter.ts index 0a07840..414901b 100644 --- a/src/execution/schema-amends/alter.ts +++ b/src/execution/schema-amends/alter.ts @@ -35,7 +35,7 @@ export class Alter extends ExecHelper implements _IStatementExecutor { ignoreChange(); break; } else { - throw new QueryError('Column already exists: ' + col.id); + throw new QueryError(`column "${col.id}" of relation "${this.table.name}" already exists`, '42701'); } } else { ignore(change.ifNotExists); diff --git a/src/execution/schema-amends/create-policy.ts b/src/execution/schema-amends/create-policy.ts index 41faaa3..fcb9fbf 100644 --- a/src/execution/schema-amends/create-policy.ts +++ b/src/execution/schema-amends/create-policy.ts @@ -29,7 +29,7 @@ export class CreatePolicy extends ExecHelper implements _IStatementExecutor { } const v = withSelection(this.table.selection, () => buildValue(expr)); if (v.type.primary !== DataType.bool && v.type.primary !== DataType.null) { - throw new QueryError(`argument of POLICY ${clause} must be type boolean, not type ${v.type.name}`, '42804'); + throw new QueryError(`argument of POLICY must be type boolean, not type ${v.type.name}`, '42804'); } }; check(p.using, 'USING'); diff --git a/src/parser/function-call.ts b/src/parser/function-call.ts index cf31e7c..e4e47a9 100644 --- a/src/parser/function-call.ts +++ b/src/parser/function-call.ts @@ -72,7 +72,7 @@ export function buildCall(name: string | QName, args: IValue[]): IValue { const toJsonArg = args[0].type; type = Types.jsonb; acceptNulls = true; - get = (v: any) => v instanceof Date ? v.toISOString() : toJsonValue(v ?? null, toJsonArg); + get = (v: any) => toJsonValue(v ?? null, toJsonArg); break; } case 'json_build_object': @@ -88,7 +88,8 @@ export function buildCall(name: string | QName, args: IValue[]): IValue { if (nullIsh(kv[i])) { throw new QueryError(`argument ${i + 1}: key must not be null`, '22004'); } - ret[String(kv[i])] = kv[i + 1] ?? null; + // values convert as to_jsonb would (numeric -> number, date -> 'YYYY-MM-DD', ...) + ret[String(kv[i])] = toJsonValue(kv[i + 1] ?? null, args[i + 1].type); } return ret; }; @@ -98,7 +99,7 @@ export function buildCall(name: string | QName, args: IValue[]): IValue { case 'jsonb_build_array': { type = Types.jsonb; acceptNulls = true; - get = (...vals: any[]) => vals.map(v => v ?? null); + get = (...vals: any[]) => vals.map((v, i) => toJsonValue(v ?? null, args[i].type)); break; } // polymorphic array functions: need the actual element type of their argument, diff --git a/src/parser/parse-cache.ts b/src/parser/parse-cache.ts index 064a28f..9a957ba 100644 --- a/src/parser/parse-cache.ts +++ b/src/parser/parse-cache.ts @@ -51,8 +51,12 @@ export function parseSql(sql: string, entry?: string): any { } - // throw a nice parsing error. - throw new QueryError(`💔 Your query failed to parse. + // postgres' first line (what a caller showing one line sees), pg-mem's detail after it + // (pg-mem terminates every query with ';', so failing on it means the input ran out) + const near = /Unexpected [^:]*token: "([^"]*)"/.exec(msg)?.[1]; + const where = near !== undefined && near !== ';' ? `at or near "${near}"` : 'at end of input'; + throw new QueryError(`syntax error ${where} +💔 Your query failed to parse. This is most likely due to a SQL syntax error. However, you might also have hit a bug, or an unimplemented feature of pg-mem. If this is the case, please file an issue at https://github.com/oguimbal/pg-mem along with a query that reproduces this syntax error. @@ -60,7 +64,7 @@ If this is the case, please file an issue at https://github.com/oguimbal/pg-mem ${sql} -💀 ${msg}`); +💀 ${msg}`, '42601'); } } diff --git a/src/table.ts b/src/table.ts index 38874b4..de5cb0b 100644 --- a/src/table.ts +++ b/src/table.ts @@ -271,7 +271,7 @@ export class MemoryTable extends DataSourceBase implements IMemoryTable, _I } if (this.columnMgr.has(column.name)) { - throw new QueryError(`Column "${column.name}" already exists`); + throw new QueryError(`column "${column.name}" of relation "${this.name}" already exists`, '42701'); } assertNotSystemColumn(column.name); const type = typeof column.type === 'string' diff --git a/src/tests/corpus-parity.spec.ts b/src/tests/corpus-parity.spec.ts index 8bb74f1..e396c32 100644 --- a/src/tests/corpus-parity.spec.ts +++ b/src/tests/corpus-parity.spec.ts @@ -678,6 +678,64 @@ describe('corpus parity', () => { }); }); + describe('aggregates over numeric and bigint', () => { + // numeric and bigint are held as digit strings: sum() concatenated them ('10' + '32.5' = '1032.5') + // and max/min compared them as text ('9' > '10') + beforeEach(() => none(`create table o (g int, a numeric, d bigint); insert into o values (1, 0.1, 9007199254740993), (1, 0.2, 1), (1, 10, -3), (2, null, null)`)); + it('sum adds exactly', () => { + expect(many(`select g, sum(a) as a, sum(d) as d from o group by g order by g`)).toEqual([{ g: 1, a: '10.3', d: '9007199254740991' }, { g: 2, a: null, d: null }]); + }); + it('avg is exact, max/min compare as numbers', () => { + expect(many(`select avg(a) as a, max(a) as mx, min(a) as mn, max(d) as dx from o`)).toEqual([{ a: 3.433333333333333, mx: '10', mn: '0.1', dx: '9007199254740993' }]); + }); + }); + + describe('plpgsql quoted identifiers', () => { + it('NEW."col" / OLD."col" in triggers, and quoted names in function bodies', () => { + none(`create table t (id int, "updated_at" timestamptz, "My Col" text); + create function f() returns trigger language plpgsql as $$ begin new."updated_at" := now(); if new."My Col" is null then new."My Col" := 'y'; end if; return new; end $$; + create trigger tr before insert on t for each row execute function f(); + insert into t (id) values (1)`); + expect(many(`select "My Col" as c, "updated_at" is not null as u from t`)).toEqual([{ c: 'y', u: true }]); + none(`do $$ begin update t set "My Col" = 'z' where "id" = 1; end $$`); + expect(many(`select "My Col" as c from t`)).toEqual([{ c: 'z' }]); + }); + }); + + describe('postgres wording for errors the validator shows', () => { + it('duplicate column, NOT NULL, policy predicate type, syntax errors', () => { + none(`create table t (id int not null, s text)`); + expectQueryError(() => none(`alter table t add column id text`), /column "id" of relation "t" already exists/); + expectQueryError(() => none(`insert into t (s) values ('x')`), /null value in column "id" of relation "t" violates not-null constraint/); + expectQueryError(() => none(`create policy p on t using (s)`), /argument of POLICY must be type boolean, not type text/); + expectQueryError(() => none(`select from where`), /^syntax error at or near "where"/); + expectQueryError(() => none(`insert into t values (1,`), /^syntax error at end of input/); + }); + }); + + describe('dates, times and numbers inside json', () => { + // postgres' text format, not JS's toISOString(); tinbase's REST answers are built with row_to_json + it('row_to_json / json_agg / to_jsonb / jsonb_build_object / jsonb_build_array', () => { + none(`create table k (d date, ts timestamp, tz timestamptz, price numeric(10,2)); insert into k values ('2026-05-26', '2026-05-26 10:30:00', '2026-05-26 10:30:00.5+00', 98.4)`); + const row = { d: '2026-05-26', ts: '2026-05-26T10:30:00', tz: '2026-05-26T10:30:00.5+00:00', price: 98.4 }; + expect(many(`select row_to_json(k) as j from k`)).toEqual([{ j: row }]); + expect(many(`select json_agg(k) as j from k`)).toEqual([{ j: [row] }]); + expect(many(`select to_jsonb(d) as a, jsonb_build_object('d', d, 'p', price) as b, jsonb_build_array(ts, price) as c from k`)) + .toEqual([{ a: '2026-05-26', b: { d: '2026-05-26', p: 98.4 }, c: ['2026-05-26T10:30:00', 98.4] }]); + }); + }); + + describe('enums order by declaration', () => { + // from a production project: order by an enum column came back alphabetical + it('in ORDER BY, comparisons and max/min', () => { + none(`create type prio as enum ('low', 'medium', 'high', 'critical'); create table t (p prio); create index on t (p); + insert into t values ('medium'), ('critical'), ('low'), ('high')`); + expect(many(`select p from t order by p`).map(r => r.p)).toEqual(['low', 'medium', 'high', 'critical']); + expect(many(`select p from t where p > 'medium' order by p`).map(r => r.p)).toEqual(['high', 'critical']); + expect(many(`select max(p) as mx, min(p) as mn from t`)).toEqual([{ mx: 'critical', mn: 'low' }]); + }); + }); + describe('CREATE OR REPLACE TRIGGER', () => { it('replaces an existing trigger', () => { none(`create table o (id int, n int); diff --git a/src/tests/publicapi.spec.ts b/src/tests/publicapi.spec.ts index df039ac..96043fa 100644 --- a/src/tests/publicapi.spec.ts +++ b/src/tests/publicapi.spec.ts @@ -31,7 +31,7 @@ describe('Public api', () => { it('matches constraints', () => { const table = simple(); - expect(() => table.insert({})).toThrow(/null value in column "id" violates not-null constraint/); + expect(() => table.insert({})).toThrow(/null value in column "id" of relation "test" violates not-null constraint/); }) it('cannot insert twice', () => { diff --git a/src/transforms/aggregations/avg.ts b/src/transforms/aggregations/avg.ts index 01609da..cae36bc 100644 --- a/src/transforms/aggregations/avg.ts +++ b/src/transforms/aggregations/avg.ts @@ -2,7 +2,8 @@ import { AggregationComputer, AggregationGroupComputer, IValue, nil, QueryError, import { ExprCall } from 'pgsql-ast-parser'; import { buildValue } from '../../parser/expression-builder'; import { Types } from '../../datatypes'; -import { nullIsh, sum } from '../../utils'; +import { Decimal } from '../../datatypes/numeric'; +import { nullIsh } from '../../utils'; import { withSelection } from '../../parser/context'; @@ -24,7 +25,12 @@ class AvgExpr implements AggregationComputer { full.push(value); } }, - finish: () => full.length === 0 ? null : sum(full) / full.length, + // summed exactly (numeric / bigint arrive as digit strings, and 0.1 + 0.2 must not drift) + finish: () => full.length === 0 + ? null + : full.reduce((acc, v) => acc.add(Decimal.fromText(String(v))), Decimal.fromNumber(0)) + .div(Decimal.fromNumber(full.length)) + .toNumber(), } } } diff --git a/src/transforms/aggregations/max-min.ts b/src/transforms/aggregations/max-min.ts index aacaeff..a9336bc 100644 --- a/src/transforms/aggregations/max-min.ts +++ b/src/transforms/aggregations/max-min.ts @@ -20,10 +20,11 @@ class MinMax implements AggregationComputer { return { feedItem: (item) => { const value = this.exp.get(item, t); + // compare as the type does (numeric/bigint are digit strings: '9' > '10' as text) if (!nullIsh(value) && (nullIsh(val) || ( this.isMax - ? val! < value - : val! > value + ? this.exp.type.gt(value, val) + : this.exp.type.lt(value, val) ))) { val = value; } @@ -55,7 +56,11 @@ export function buildMinMax(this: void, base: _ISelection, args: Expr[], op: 'ma case DataType.timestamptz: break; default: - throw new QueryError(`function min(${what.type.primary}) does not exist`, '42883'); + // enums (max(priority)) compare in declaration order + if (Array.isArray((what.type as any).values)) { + break; + } + throw new QueryError(`function ${op}(${what.type.primary}) does not exist`, '42883'); } return new MinMax(what, op === 'max'); }); diff --git a/src/transforms/aggregations/sum.ts b/src/transforms/aggregations/sum.ts index f28bade..1e6e1f1 100644 --- a/src/transforms/aggregations/sum.ts +++ b/src/transforms/aggregations/sum.ts @@ -2,28 +2,47 @@ import { AggregationComputer, AggregationGroupComputer, IValue, nil, QueryError, import { ExprCall } from 'pgsql-ast-parser'; import { buildValue } from '../../parser/expression-builder'; import { Types } from '../../datatypes'; +import { Decimal } from '../../datatypes/numeric'; +import { DataType } from '../../interfaces'; import { nullIsh } from '../../utils'; import { withSelection } from '../../parser/context'; -class SumExpr implements AggregationComputer { +class SumExpr implements AggregationComputer { constructor(private exp: IValue) { } + /** sum(numeric) and sum(bigint) are numeric in postgres; the others keep their kind */ get type(): _IType { - return Types.integer; + switch (this.exp.type.primary) { + case DataType.decimal: + case DataType.bigint: + return Types.decimal(); + case DataType.float: + return Types.float; + default: + return Types.integer; + } } - createGroup(t: _Transaction): AggregationGroupComputer { - let val: number | nil = null; + createGroup(t: _Transaction): AggregationGroupComputer { + // numeric and bigint are held as digit strings: add them exactly (`+` concatenated them) + const exact = this.exp.type.primary === DataType.decimal || this.exp.type.primary === DataType.bigint; + let val: any = null; return { feedItem: (item) => { const value = this.exp.get(item, t); - if (!nullIsh(value)) { + if (nullIsh(value)) { + return; + } + if (exact) { + const d = Decimal.fromText(String(value)); + val = nullIsh(val) ? d : (val as Decimal).add(d); + } else { val = nullIsh(val) ? value : val + value; } }, - finish: () => val, + finish: () => exact && !nullIsh(val) ? (val as Decimal).toString() : val, } } }