Skip to content

Numeric - is registered as commutative, so column - $1 evaluates as $1 - column #482

Description

@crumbtrail-dev

Summary

column - value evaluates as value - column whenever the right operand is an untyped literal and the two operands are different numeric widths. The numeric - operator is registered with commutative: true, which is not true of subtraction.

registerOperator reacts to that flag by also registering a mirrored overload whose implementation swaps its operands:

https://github.com/oguimbal/pg-mem/blob/master/src/schema/schema.ts#L515-L526

registerOperator(op: OperatorDefinition, replace?: boolean): this {
    this._registerOperator(op, replace ?? true);
    if (op.commutative && op.left !== op.right) {
        this._registerOperator({
            ...op,
            left: op.right,
            right: op.left,
            implementation: (a, b) => op.implementation(b, a),
        }, replace ?? true);
    }
    return this;
}

registerNumericOperators loops over every ordered pair from numberPairs(), so for a mixed-width pair such as (bigint, integer) both the direct overload and the mirror of (integer, bigint) are registered under the same signature. replace defaults to true, so whichever is registered last wins, and for - the mirror wins for some pairs. The mirror computes b - a.

https://github.com/oguimbal/pg-mem/blob/master/src/schema/pg-catalog/binary-operators.ts#L34-L42

for (const [left, right, returns] of numberPairs()) {
    schema.registerOperator({
        operator: '-',
        commutative: true,   // <-- subtraction is not commutative
        left,
        right,
        returns,
        implementation: (a, b) => a - b,
    });
}

+ and * set the same flag and are genuinely commutative, so they are fine. / already sets commutative: false. The date/time and jsonb - overloads are also fine.

An untyped literal on the right is what selects a mixed-width overload, which is why this shows up through the pg adapter: it inlines query parameters as untyped literals, so any SET col = col - $1 runs backwards.

Repro

import { newDb } from 'pg-mem';

const db = newDb();
db.public.none(`CREATE TABLE w (big bigint NOT NULL, i int NOT NULL)`);
db.public.none(`INSERT INTO w (big, i) VALUES (328, 328)`);

db.public.many(`SELECT big - '300' AS a FROM w`)[0].a;          // -28, expected 28
db.public.many(`SELECT i   - '300' AS a FROM w`)[0].a;          // -28, expected 28
db.public.many(`SELECT big - 300 AS a FROM w`)[0].a;            //  28  (typed literal, correct)
db.public.many(`SELECT big - '300'::bigint AS a FROM w`)[0].a;  //  28  (explicit cast, correct)
db.public.many(`SELECT '400' - big AS a FROM w`)[0].a;          //  72  (correct)

const { Pool } = db.adapters.createPg();
const pool = new Pool();
(await pool.query('SELECT big - $1 AS a FROM w', [300])).rows[0].a;   // -28, expected 28
(await pool.query('SELECT big / $1 AS a FROM w', [4])).rows[0].a;     //  82  (correct)
(await pool.query('SELECT big >= $1 AS a FROM w', [300])).rows[0].a;  // true (correct)

Actual output on 3.0.14:

A1 big - '300'   = -28 (expected 28)
A2 i   - '300'   = -28 (expected 28)
A3 big - 300     = 28 (expected 28)
A4 big - '300'::bigint = 28 (expected 28)
A5 '400' - big   = 72 (expected 72)
B1 big - $1      = -28 (expected 28)
B2 big / $1      = 82 (expected 82)
B3 big >= $1     = true (expected true)

Why it is easy to miss

A decrement guarded by a CHECK constraint fails with a message that points at the data rather than the operator:

db.public.none(`CREATE TABLE ledger (id text PRIMARY KEY, reserved bigint NOT NULL CHECK (reserved >= 0))`);
await pool.query("INSERT INTO ledger VALUES ('a', 100)");
await pool.query(
  "UPDATE ledger SET reserved = reserved - $1 WHERE id = 'a' AND reserved >= $1 RETURNING reserved",
  [60],
);
// throws: check constraint "ledger_constraint_1" is violated by some row
// expected: reserved = 40

The guard reserved >= $1 passes because comparisons are unaffected, so the statement gets as far as writing -60. And a decrement that happens to zero the column (reserved - $1 where the two are equal) gives the same answer either way, so the sign error stays hidden until a case leaves a positive remainder.

mem.public.registerOperator({ operator: '-', ... }) is not a usable workaround from application code. Re-registering all 16 numeric pairs with commutative: false did not displace the mirrored overload.

Suggested fix

commutative: false on the numeric - registration, matching what / already does:

     for (const [left, right, returns] of numberPairs()) {
         schema.registerOperator({
             operator: '-',
-            commutative: true,
+            commutative: false,
             left,
             right,
             returns,
             implementation: (a, b) => a - b,
         });
     }

Because numberPairs() already yields every ordered pair, dropping the flag loses no coverage: each signature is still registered directly, just without a swapped mirror overwriting it.

Verified on a fresh master clone (bun install && bun test):

  • before the change: 846 pass, 25 skip, 3 fail
  • after the change: 846 pass, 25 skip, 3 fail

The 3 failures are identical in both runs and unrelated (typeorm decorator errors in src/tests/irl-tests/, undefined is not an object (evaluating 'target.constructor'), which look like a typeorm-under-bun environment issue rather than anything to do with operators). Every repro line above returns the expected value with the change applied.

Happy to send this as a PR with a regression test if that is useful.

Environment

  • pg-mem 3.0.14 (also present on master)
  • Node 24.14.1, macOS

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions