Postgres parity from 250 real project migrations: transactional DDL, type checking, RLS fixes

package.json

@@ -49,12 +49,13 @@
     "immutable": "^4.3.4",
     "lru-cache": "^6.0.0",
     "object-hash": "^2.0.3",
-    "pgsql-ast-parser": "npm:@tinbase/pgsql-ast-parser@^12.1.0"
+    "pgsql-ast-parser": "npm:@tinbase/pgsql-ast-parser@^12.2.0"
   },
   "devDependencies": {
     "@babel/cli": "^7.10.5",
     "@babel/core": "^7.10.5",
     "@babel/preset-env": "^7.10.4",
+    "@electric-sql/pglite": "0.5.4",
     "@hot-loader/react-dom": "^16.13.0",
     "@mikro-orm/core": "^5.8.1",
     "@mikro-orm/postgresql": "^5.8.1",

src/column.ts

@@ -1,7 +1,8 @@
 import { _Column, IValue, _IIndex, NotSupported, _Transaction, QueryError, _IType, SchemaField, ChangeHandler, nil, ISubscription, DropHandler } from './interfaces-private';
 import type { MemoryTable } from './table';
 import { Evaluator } from './evaluator';
-import { ColumnConstraint, AlterColumn, Expr, toSql } from 'pgsql-ast-parser';
+import { ColumnConstraint, AlterColumn, Expr, toSql, DataTypeDef } from 'pgsql-ast-parser';
+import { policiesDependingOn } from './execution/policy-deps';
 import { ignore, nullIsh } from './utils';
 import { columnEvaluator } from './transforms/selection';
 import { BIndex } from './schema/btree-index';
@@ -29,10 +30,18 @@ export class ColRef implements _Column {
     usedInIndexes = new Set<BIndex>();
     private drophandlers = new Set<DropHandler>();
 
+    /**
+     * The type as written in the DDL. pg-mem folds smallint into integer, real into double and
+     * char(n) into text, but information_schema reports the declared type, and a generator
+     * reading data_type should see `smallint`, not `integer`.
+     */
+    declaredType: DataTypeDef | nil;
+
     constructor(readonly table: MemoryTable
         , public expression: Evaluator
         , _schema: SchemaField
         , public name: string) {
+        this.declaredType = (_schema as any)?.declaredType ?? (_schema as any)?.dataType ?? null;
     }
 
     addConstraints(clist: ColumnConstraint[], t: _Transaction): this {
@@ -178,6 +187,10 @@ export class ColRef implements _Column {
                     break;
                 case 'set type':
                     const newType = this.table.ownerSchema.getType(alter.dataType);
+                    if (policiesDependingOn(this.table, this.expression.id!).length) {
+                        // postgres stores policies as bound expressions over the column's type
+                        throw new QueryError('cannot alter type of a column used in a policy definition', '0A000');
+                    }
                     // USING <expr> gives an explicit per-row conversion (referencing the
                     // column and any others); without it, an implicit cast is used.
                     const conv = alter.using
@@ -235,15 +248,24 @@ export class ColRef implements _Column {
     }
 
     checkConstraints(toInsert: any, t: _Transaction) {
+        const col = this.expression.get(toInsert, t);
+        if (this.isSmallint && typeof col === 'number' && (col < -32768 || col > 32767)) {
+            // smallint is stored as integer here; enforce int2's range like postgres does
+            throw new QueryError('smallint out of range', '22003');
+        }
         if (!this.notNull) {
             return;
         }
-        const col = this.expression.get(toInsert, t);
         if (nullIsh(col)) {
             throw new QueryError(`null value in column "${this.expression.id}" violates not-null constraint`);
         }
     }
 
+    private get isSmallint() {
+        const n = (this.declaredType as any)?.name?.toLowerCase();
+        return n === 'smallint' || n === 'int2' || n === 'smallserial';
+    }
+
     setDefaults(toInsert: any, t: _Transaction) {
         const col = this.expression.get(toInsert, t);
         if (col !== undefined) {

src/constraints/foreign-key.ts

@@ -3,6 +3,7 @@ import { Expr, ExprBinary, TableConstraintForeignKey } from 'pgsql-ast-parser';
 import { asTable, CreateIndexColDef, _IConstraint, _ITable, _Transaction } from '../interfaces-private';
 import { nullIsh } from '../utils';
 import { deferCheck } from '../execution/deferred-checks';
+import { enqueueRi } from '../execution/ri-queue';
 
 export class ForeignKey implements _IConstraint {
 
@@ -46,6 +47,11 @@ export class ForeignKey implements _IConstraint {
         return this.foreignTable.name;
     }
 
+    /** The referenced table, for dependency checks (DROP TABLE / DROP COLUMN refuse to orphan this FK). */
+    get referencedTable(): _ITable {
+        return this.foreignTable;
+    }
+
 
     constructor(readonly name: string) {
     }
@@ -124,13 +130,14 @@ export class ForeignKey implements _IConstraint {
                 right: b,
             }), equals[0]);
 
-            // check nothing matches
-            for (const local of table.selection.filter(expr).enumerate(dt)) {
+            // check nothing matches - as a queued RI action, see ri-queue.ts
+            enqueueRi(() => {
+            for (const local of [...table.selection.filter(expr).enumerate(dt)]) {
                 // ====== ON DELETE
                 switch (neu ? onUpdate : onDelete) {
                     case 'no action':
                     case 'restrict':
-                        throw new QueryError(`update or delete on table "${ftable.name}" violates foreign key constraint on table "${this.name}"`);
+                        throw new QueryError(`update or delete on table "${ftable.name}" violates foreign key constraint "${this.name}" on table "${table.name}"`, '23503');
                     case 'cascade':
                         if (neu) {
                             for (let i = 0; i < fcols.length; i++) {
@@ -150,6 +157,7 @@ export class ForeignKey implements _IConstraint {
                         break;
                 }
             }
+            });
         }));
 
         // =====================
@@ -191,7 +199,7 @@ export class ForeignKey implements _IConstraint {
                     yielded = true;
                 }
                 if (!yielded) {
-                    throw new QueryError(`insert or update on table "${ftable.name}" violates foreign key constraint on table "${this.name}"`);
+                    throw new QueryError(`insert or update on table "${table.name}" violates foreign key constraint "${this.name}"`, '23503');
                 }
             };
             if (deferred) {
@@ -243,8 +251,14 @@ export class ForeignKey implements _IConstraint {
         return this;
     }
 
+    /** set by the owning table: drops this FK from its constraint list (and so the catalogues) */
+    onUninstalled?: () => void;
+
     uninstall(t: _Transaction): void {
         this.unsubs.forEach(x => x.unsubscribe());
         this.unsubs = [];
+        // also when uninstalled from the other side (DROP TABLE <referenced> CASCADE), not only
+        // via the table's own ConstraintWrapper - or the catalogues keep listing a dead FK
+        this.onUninstalled?.();
     }
 }

src/datatypes/datatypes.ts

@@ -101,6 +101,26 @@ export function isInteger(t: DataType | IType) {
     return integers.has(type);
 }
 
+const INT4_MIN = -2147483648;
+const INT4_MAX = 2147483647;
+const INT8_MIN = BigInt('-9223372036854775808');
+const INT8_MAX = BigInt('9223372036854775807');
+
+/** Postgres stores integer as int4: a value outside it is an error, not a silently wider number. */
+export function checkInt4(n: number): number {
+    if (n < INT4_MIN || n > INT4_MAX) {
+        throw new QueryError('integer out of range', '22003');
+    }
+    return n;
+}
+
+export function checkInt8(n: bigint): bigint {
+    if (n < INT8_MIN || n > INT8_MAX) {
+        throw new QueryError('bigint out of range', '22003');
+    }
+    return n;
+}
+
 class NumberType extends TypeBase<number> {
 
     constructor(readonly primary: DataType, typeId: number) {
@@ -122,12 +142,16 @@ class NumberType extends TypeBase<number> {
     }
 
     doPrefer(type: _IType): _IType | null {
+        // the wider of the two, as postgres' implicit numeric promotion does:
+        // int4 < int8 < numeric < float8 (so int = bigint compares as bigint, whichever side it is on)
         switch (type.primary) {
             case DataType.integer:
-            case DataType.bigint:
                 return this;
-            case DataType.float:
+            case DataType.bigint:
+                return this.primary === DataType.integer ? type : this;
             case DataType.decimal:
+                return this.primary === DataType.float ? this : type;
+            case DataType.float:
                 return type;
         }
         return null;
@@ -157,9 +181,11 @@ class NumberType extends TypeBase<number> {
                 , value
                 , (raw, t) => {
                     const got = value.get(raw, t);
-                    return typeof got === 'number'
-                        ? Math.round(got)
-                        : got;
+                    if (typeof got !== 'number') {
+                        return got;
+                    }
+                    const r = Math.round(got);
+                    return to.primary === DataType.integer ? checkInt4(r) : r;
                 }
             );
         }
@@ -244,7 +270,7 @@ class BigIntType extends TypeBase<string> {
             case DataType.text:
                 return value.setType(to);
             case DataType.integer:
-                return value.setType(to).setConversion((s: string) => Number(BigInt(s)), toInt => ({ toInt }));
+                return value.setType(to).setConversion((s: string) => checkInt4(Number(BigInt(s))), toInt => ({ toInt }));
             case DataType.float:
                 return value.setType(to).setConversion((s: string) => Number(s), toFloat => ({ toFloat }));
             case DataType.decimal:
@@ -296,7 +322,7 @@ class DecimalType extends TypeBase<string> {
             case DataType.float:
                 return value.setType(to).setConversion((s: string) => Number(s), toFloat => ({ toFloat }));
             case DataType.integer:
-                return value.setType(to).setConversion((s: string) => Number(Decimal.fromText(s).round(0).toString()), toInt => ({ toInt }));
+                return value.setType(to).setConversion((s: string) => checkInt4(Number(Decimal.fromText(s).round(0).toString())), toInt => ({ toInt }));
             case DataType.bigint:
                 return value.setType(to).setConversion((s: string) => Decimal.fromText(s).round(0).toString(), toBig => ({ toBig }));
         }
@@ -516,17 +542,46 @@ class TextType extends TypeBase<string> {
 
         }
         if (numbers.has(to.primary)) {
-            const isInt = integers.has(to.primary);
+            // parse like postgres' input functions: the whole string (surrounding blanks aside)
+            // must be a number - parseFloat('12abc') = 12 is not - and produce the representation
+            // the target type stores (numeric and bigint are digit strings, not JS numbers)
+            const pgName = to.primary === DataType.decimal ? 'numeric' : to.primary === DataType.float ? 'double precision' : to.primary;
+            const scale = (to as any).scale as number | null | undefined;
             return value
                 .setConversion(str => {
-                    const val = Number.parseFloat(str);
-                    if (!Number.isFinite(val)) {
-                        throw new QueryError(`invalid input syntax for ${to.primary}: ${str}`);
-                    }
-                    if (isInt && Math.floor(val) !== val) {
-                        throw new QueryError(`invalid input syntax for ${to.primary}: ${str}`)
+                    const s = String(str).trim();
+                    const bad = () => new QueryError(`invalid input syntax for type ${pgName}: "${str}"`, '22P02');
+                    switch (to.primary) {
+                        case DataType.integer:
+                            if (!/^[+-]?\d+$/.test(s)) {
+                                throw bad();
+                            }
+                            return checkInt4(Number(s));
+                        case DataType.bigint:
+                            if (!/^[+-]?\d+$/.test(s)) {
+                                throw bad();
+                            }
+                            return checkInt8(BigInt(s)).toString();
+                        case DataType.decimal: {
+                            if (!/^[+-]?(\d+\.?\d*|\.\d+)(e[+-]?\d+)?$/i.test(s) && !/^nan$/i.test(s)) {
+                                throw bad();
+                            }
+                            const d = Decimal.fromText(s);
+                            return (scale === null || scale === undefined ? d : d.round(scale)).toString();
+                        }
+                        default: {
+                            if (/^[+-]?inf(inity)?$/i.test(s)) {
+                                return s.startsWith('-') ? -Infinity : Infinity;
+                            }
+                            if (/^nan$/i.test(s)) {
+                                return NaN;
+                            }
+                            if (!/^[+-]?(\d+\.?\d*|\.\d+)(e[+-]?\d+)?$/i.test(s)) {
+                                throw bad();
+                            }
+                            return Number(s);
+                        }
                     }
-                    return val;
                 }
                     , castNum => ({ castNum, to: to.primary }));
         }
@@ -906,6 +961,46 @@ export function reconciliateTypes(values: IValue[], nullIfNoMatch?: boolean, str
 
 
 
+/**
+ * Postgres type category (pg_type.typcategory) of the built-in types pg-mem models.
+ * Postgres has no implicit cast between these categories: a typed value only crosses one
+ * through an explicit cast, so `uuid_col = text_col` is "operator does not exist: uuid = text".
+ * Untyped literals and bind parameters are exempt - they are 'unknown' and coerce to anything.
+ */
+function typeCategory(t: _IType): string | null {
+    switch (t.primary) {
+        case DataType.text:
+        case DataType.citext:
+            return 'string';
+        case DataType.uuid:
+            return 'uuid';
+        case DataType.bool:
+            return 'bool';
+        case DataType.json:
+        case DataType.jsonb:
+            return 'json';
+        case DataType.integer:
+        case DataType.bigint:
+        case DataType.float:
+        case DataType.decimal:
+            return 'numeric';
+        case DataType.date:
+        case DataType.timestamp:
+        case DataType.timestamptz:
+            return 'datetime';
+        case DataType.bytea:
+            return 'bytea';
+    }
+    return null;
+}
+
+/** True when Postgres refuses to implicitly convert a typed value of `from` into `to`. */
+export function crossesTypeCategory(from: _IType, to: _IType): boolean {
+    const a = typeCategory(from);
+    const b = typeCategory(to);
+    return !!a && !!b && a !== b;
+}
+
 /** Finds a common type by implicit conversion */
 function reconciliateTypesRaw(values: IValue[], nullIfNoMatch?: false, stricterType?: boolean): _IType;
 function reconciliateTypesRaw(values: IValue[], nullIfNoMatch: true, stricterType?: boolean): _IType | nil;
@@ -926,7 +1021,8 @@ function reconciliateTypesRaw(values: IValue[], nullIfNoMatch?: boolean, stricte
 
     // check that all constant literals are matching this.
     for (const x of values) {
-        if (!x.isConstantLiteral && !x.type.canConvertImplicit(foundType)) {
+        const unknownLiteral = x.isConstantLiteral && x.type.primary === DataType.text;
+        if (!x.isConstantLiteral && !x.type.canConvertImplicit(foundType) || !unknownLiteral && crossesTypeCategory(x.type, foundType)) {
             if (nullIfNoMatch) {
                 return null;
             }

src/datatypes/t-interval.ts

@@ -1,5 +1,5 @@
 import { DataType, nil, _IType } from '../interfaces-private';
-import { Interval, normalizeInterval, parseIntervalLiteral } from 'pgsql-ast-parser';
+import { Interval, normalizeInterval, parseIntervalLiteral, intervalToString } from 'pgsql-ast-parser';
 import { TypeBase } from './datatype-base';
 import { Evaluator } from '../evaluator';
 import { intervalToSec } from '../utils';
@@ -31,6 +31,19 @@ export class IntervalType extends TypeBase<Interval> {
         return null;
     }
 
+    doCanCast(to: _IType) {
+        return to.primary === DataType.text;
+    }
+
+    doCast(value: Evaluator<Interval>, to: _IType): Evaluator<any> | nil {
+        if (to.primary !== DataType.text) {
+            return null;
+        }
+        return value
+            .setType(to)
+            .setConversion((i: Interval) => pgIntervalText(i), intervalToText => ({ intervalToText }));
+    }
+
     doEquals(a: Interval, b: Interval): boolean {
         return intervalToSec(a) === intervalToSec(b);
     }
@@ -41,3 +54,11 @@ export class IntervalType extends TypeBase<Interval> {
         return intervalToSec(a) < intervalToSec(b);
     }
 }
+
+/** postgres' default (IntervalStyle = postgres) text form: "1 year 2 mons 3 days 04:05:06", "00:00:00" for zero */
+function pgIntervalText(i: Interval): string {
+    const txt = String(intervalToString(i))
+        .replace(/(-?\d+) months?\b/, (_, n) => `${n} ${Math.abs(Number(n)) === 1 ? 'mon' : 'mons'}`)
+        .trim();
+    return txt || '00:00:00';
+}