Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .changeset/introspect-primary-key-order-pg-mysql.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
---
"@objectstack/driver-sql": patch
---

fix(driver-sql): `introspectPrimaryKeys` returns the Postgres and MySQL composite key in DECLARED KEY ORDER (#11101)

`SqlDriver.introspectPrimaryKeys` ordered its result on exactly one of its three
dialect arms. #10997 repaired SQLite (completeness *and* key ordering, by sorting
on the `PRAGMA table_info` ordinal); the Postgres and MySQL arms returned the key
in unspecified row order.

- **Postgres**: `a.attnum = ANY(i.indkey)` is a *membership* test. `i.indkey` is
an `int2vector` holding the key's attnums **in key order**, but `ANY()` reads
the vector as a set and discards the position, and the query carried no
`ORDER BY`. It now joins the **ordinality** of `indkey`
(`unnest(i.indkey) WITH ORDINALITY`) and orders by that ordinal.
- **MySQL**: `KEY_COLUMN_USAGE.ORDINAL_POSITION` *is* the key ordinal and was
selected by neither the projection nor an order clause. It now carries
`ORDER BY ORDINAL_POSITION`.

Both arms were measured returning **column order** — the key reversed — against
live servers before the fix: PostgreSQL 16.13 and MySQL 8.0.46, on a table
declared `(carrier_code, shipment_id, leg_seq)` with
`PRIMARY KEY (shipment_id, carrier_code)`. The MySQL result is worth naming
explicitly, because the received wisdom is the opposite: InnoDB did **not**
return ordinal order for an out-of-sequence key.

Why the order is load-bearing rather than cosmetic: `primaryKeys` is consumed as
an **addressing / upsert-conflict-target** key — federated-object codegen, the
persisted `external_catalog` under ADR-0015, and schema-drift comparison against
a declared key. For those consumers a key in the wrong order is a *different*
key. Until now the same table introspected through different dialects could
disagree, since SQLite reported declared key order and the other two did not; all
three now agree.

Covered by `sql-driver-primary-key-order-dialects.test.ts`, which runs the same
DDL on all three dialects and asserts the exact ordered array. Its live Postgres
and MySQL cells execute in the `Temporal Conformance (live PG + MySQL)` CI job
(a required check) and are reported as named skips elsewhere.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,301 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#11101] `introspectPrimaryKeys` must report a composite key in DECLARED KEY
* ORDER on **every** dialect — SQLite, Postgres and MySQL — for the same table.
*
* #10997 repaired the SQLite arm (completeness *and* ordering, by sorting on the
* `PRAGMA table_info` ordinal). The other two arms did not order at all:
*
* - **Postgres**: `a.attnum = ANY(i.indkey)` is a MEMBERSHIP test. `i.indkey`
* holds the key's attnums in key order, but `ANY()` reads the vector as a set
* and discards the position; with no `ORDER BY` the row order was whatever
* the plan yielded.
* - **MySQL**: `KEY_COLUMN_USAGE.ORDINAL_POSITION` *is* the key ordinal and was
* selected by neither the projection nor an order clause.
*
* Both were measured returning **column order** on live servers before the fix
* (PostgreSQL 16.13 and MySQL 8.0.46 — see the PR body), i.e. the key REVERSED
* for the fixture below. `primaryKeys` is consumed as an addressing /
* upsert-conflict-target key (federated-object codegen, the persisted
* `external_catalog` under ADR-0015, schema-drift comparison), so a key in the
* wrong order is a DIFFERENT key — and the same table introspected through
* different dialects disagreed.
*
* ## ⛔ Why every assertion here is POSITIVE and ORDERED
*
* `introspectPrimaryKeys` wraps its whole body in `catch { }` and returns `[]`.
* A query that is invalid on a live server therefore does **not** fail loudly —
* it degrades to *no primary key at all*, with no diagnostic. So a test that
* asserts "does not throw", or that checks membership / set equality, is
* worthless here: it stays green over total key loss.
*
* Every leg asserts the **exact array**, and {@link expectDeclaredKeyOrder}
* checks the length first so a degradation reads as "the silent catch ate the
* query" rather than as a diff nobody can interpret. (The catch itself is out of
* scope for this card and is filed separately — this file does not pin it.)
*
* ## ⛔ Why the fixture declares its key OUT OF COLUMN SEQUENCE
*
* Column order and key order coincide for most tables, and column order is
* exactly what the unordered queries already returned — so a table whose key
* follows its columns proves nothing. {@link KEY_ORDER} is a genuine permutation
* of the key columns' positions, and `asserts the fixture is non-vacuous` fails
* if a later edit ever flattens it back into column sequence.
*
* ## How the three dialects are held to ONE answer
*
* Every cell runs the **same DDL** and asserts against the **same**
* {@link KEY_ORDER} constant, so agreement across dialects is by construction
* rather than by a cross-suite comparison that vitest's file parallelism could
* not make reliable. The live cells are declared through `declareDialectCell`:
* REPORTED as a named skip without `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL`,
* and a hard failure under `OS_EXPECT_LIVE_DIALECT_MATRIX=1` — which is what the
* `Temporal Conformance (live PG + MySQL)` job sets, so these legs really do
* execute against `postgres:16` and `mysql:8.0` on a required check.
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { SqlDriver } from '../src/index.js';
import { DIALECT_CELLS, declareDialectCell, type DialectCell } from './live-dialect-matrix.testkit.js';

const MATRIX = 'composite primary-key ORDER';

/** Tables this file owns. The SCHEMA/database they land in is per-file (#9350). */
const TWO_PART = 'os11101_shipment_legs';
const THREE_PART = 'os11101_route_hops';

/**
* Column order — deliberately NOT the key order.
*
* `varchar(64)` rather than `text` so one DDL string is legal on all three
* dialects: MySQL cannot take a bare `TEXT` column into a primary key without a
* prefix length, and the point of this file is that the three dialects answer
* identically **for the same table**.
*/
const TWO_PART_DDL = `create table ${TWO_PART} (
carrier_code varchar(64) not null,
shipment_id varchar(64) not null,
leg_seq integer,
primary key (shipment_id, carrier_code)
)`;

/**
* Column order exactly as {@link TWO_PART_DDL} declares it.
*
* Held as a constant rather than read back from `introspectSchema().columns`,
* because that list is NOT in declared column order on every dialect: measured
* on MySQL 8.0.46 it comes back ALPHABETICAL (`carrier_code, leg_seq,
* shipment_id`), since `introspectColumns` builds it from knex's
* `columnInfo()` — an object keyed by column name. That is a separate finding
* filed from this card; it is not this file's subject, and depending on it here
* would make the fixture's own premise dialect-specific.
*
* The `asserts the fixture is non-vacuous` leg pins this constant against the
* DDL text so the two cannot drift apart.
*/
const COLUMN_ORDER = ['carrier_code', 'shipment_id', 'leg_seq'];

/** Declared KEY order: a reversal of the two key columns' positions. */
const KEY_ORDER = ['shipment_id', 'carrier_code'];

/** The same key as the buggy arms reported it — i.e. in COLUMN order. */
const KEY_IN_COLUMN_ORDER = ['carrier_code', 'shipment_id'];

/**
* A three-part key that is a genuine PERMUTATION, not merely a reversal.
*
* A two-column fixture cannot tell "sorted by key ordinal" apart from "sorted
* backwards", and a fix that reversed the row order would satisfy the two-part
* leg while still being wrong. `(b, c, a)` over columns `(c, a, b, d)` is fixed
* by neither reversal nor sorting.
*/
const THREE_PART_DDL = `create table ${THREE_PART} (
c varchar(64) not null,
a varchar(64) not null,
b varchar(64) not null,
d integer,
primary key (b, c, a)
)`;

const THREE_PART_KEY_ORDER = ['b', 'c', 'a'];

/**
* Assert the exact ordered key, with the `[]` degradation named.
*
* The length check is first on purpose: under the method's silent `catch` a
* query that a server rejects yields `[]`, and "expected [] to equal
* ['shipment_id', 'carrier_code']" does not tell the next reader that the SQL
* never ran. This message does.
*/
function expectDeclaredKeyOrder(actual: string[], expected: string[], cell: DialectCell): void {
expect(
actual.length,
`${cell.label}: introspectPrimaryKeys returned ${actual.length} column(s), expected ` +
`${expected.length}. An EMPTY result usually means the dialect arm's query was rejected by ` +
`the server and swallowed by the method's \`catch { }\` — read the query, not this fixture.`,
).toBe(expected.length);

expect(actual, `${cell.label}: key must be in DECLARED order, not column order`).toEqual(expected);
}

function declareKeyOrderSuite(cell: DialectCell): void {
describe(`introspectPrimaryKeys key order — ${cell.label} (#11101)`, () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver(cell.config());
for (const t of [TWO_PART, THREE_PART]) {
await driver.execute(`drop table if exists ${t}`).catch(() => {});
}
await driver.execute(TWO_PART_DDL);
await driver.execute(THREE_PART_DDL);
});

afterEach(async () => {
for (const t of [TWO_PART, THREE_PART]) {
await driver.execute(`drop table if exists ${t}`).catch(() => {});
}
await driver.disconnect();
});

it('asserts the fixture is non-vacuous: declared key order differs from column order', async () => {
// 1. COLUMN_ORDER really is the order the DDL declares — an anti-drift
// check on the constant, read off the fixture's own text so it cannot
// quietly stop describing the table it names.
const declaredAt = COLUMN_ORDER.map((c) => TWO_PART_DDL.indexOf(`\n ${c} `));
expect(declaredAt.every((at) => at > 0)).toBe(true);
expect([...declaredAt].sort((x, y) => x - y)).toEqual(declaredAt);

// 2. The key columns, taken in COLUMN order, are not the declared KEY
// order. This is the whole premise of the fixture: column order is
// precisely what the unordered queries already returned, so a key that
// followed its columns would make every assertion below a tautology
// the buggy arms also passed.
expect(KEY_IN_COLUMN_ORDER).toEqual(COLUMN_ORDER.filter((c) => KEY_ORDER.includes(c)));
expect(KEY_ORDER).not.toEqual(KEY_IN_COLUMN_ORDER);

// 3. The table really has those columns (as a SET — see COLUMN_ORDER's
// note on why the introspected order is not comparable across
// dialects).
const schema = await driver.introspectSchema();
const found = schema.tables[TWO_PART].columns.map((c) => c.name).sort();
expect(found).toEqual([...COLUMN_ORDER].sort());
});

it('reports a two-part composite key in declared key order, not column order', async () => {
const schema = await driver.introspectSchema();
const introspected = schema.tables[TWO_PART].primaryKeys;

expectDeclaredKeyOrder(introspected, KEY_ORDER, cell);

// The pre-fix answer, named: this is what both live servers returned
// before the rewrite, and it is a DIFFERENT addressing key.
expect(introspected).not.toEqual(KEY_IN_COLUMN_ORDER);
});

it('reports a three-part key that is a permutation of column order', async () => {
const schema = await driver.introspectSchema();
const introspected = schema.tables[THREE_PART].primaryKeys;

expectDeclaredKeyOrder(introspected, THREE_PART_KEY_ORDER, cell);

// Neither the column order nor its reverse — so an arm that merely
// reversed rows, or sorted them, cannot pass this.
expect(introspected).not.toEqual(['c', 'a', 'b']);
expect(introspected).not.toEqual(['b', 'a', 'c']);
expect(introspected).not.toEqual([...THREE_PART_KEY_ORDER].sort());
});

it('derives the per-column primaryKey flag for every key member', async () => {
const schema = await driver.introspectSchema();
const flags = Object.fromEntries(
schema.tables[TWO_PART].columns.map((c) => [c.name, c.primaryKey === true]),
);

// `introspectSchema` derives this FROM `primaryKeys`, so it is the second
// signal an empty result would corrupt.
expect(flags).toEqual({ carrier_code: true, shipment_id: true, leg_seq: false });
});
});
}

for (const cell of DIALECT_CELLS) {
declareDialectCell(cell, MATRIX, declareKeyOrderSuite);
}

/**
* The catalog facts each rewritten arm rests on, pinned per dialect.
*
* Same role as `#10997`'s "`pk` is a 1-based ordinal, not a boolean" pin: if a
* server ever stopped reporting these, the arm above would be wrong for a
* reason no assertion on its OUTPUT could localise.
*
* ⛔ Note what is deliberately NOT pinned: the row order the *unordered* query
* returns. That order is unspecified by both engines — asserting the reversal
* these servers happen to produce would be pinning a behaviour neither vendor
* promises. The measured pre-fix output is recorded in the PR body instead.
*/
function declareCatalogPins(cell: DialectCell): void {
if (cell.id === 'sqlite') return; // covered by sql-driver-composite-primary-key-introspection.test.ts

describe(`introspectPrimaryKeys catalog facts — ${cell.label} (#11101)`, () => {
let driver: SqlDriver;

beforeEach(async () => {
driver = new SqlDriver(cell.config());
await driver.execute(`drop table if exists ${TWO_PART}`).catch(() => {});
await driver.execute(TWO_PART_DDL);
});

afterEach(async () => {
await driver.execute(`drop table if exists ${TWO_PART}`).catch(() => {});
await driver.disconnect();
});

if (cell.id === 'pg') {
it('pg_index.indkey holds attnums in KEY order, while pg_attribute is in COLUMN order', async () => {
const indkey: any = await driver.execute(
`select i.indkey::text as indkey from pg_index i
where i.indrelid = '${TWO_PART}'::regclass and i.indisprimary`,
);
// carrier_code is attnum 1, shipment_id is attnum 2 — so "2 1" is the
// key order, and it is the REVERSE of the attnum sequence. This is the
// position `a.attnum = ANY(i.indkey)` discarded.
expect(indkey.rows[0].indkey).toBe('2 1');

const atts: any = await driver.execute(
`select attnum, attname from pg_attribute
where attrelid = '${TWO_PART}'::regclass and attnum > 0 and not attisdropped
order by attnum`,
);
expect(atts.rows.map((r: any) => r.attname)).toEqual([
'carrier_code',
'shipment_id',
'leg_seq',
]);
});
}

if (cell.id === 'mysql') {
it('KEY_COLUMN_USAGE.ORDINAL_POSITION is the key ordinal', async () => {
const res: any = await driver.execute(
`select COLUMN_NAME as column_name, ORDINAL_POSITION as ordinal_position
from information_schema.KEY_COLUMN_USAGE
where TABLE_SCHEMA = DATABASE() and TABLE_NAME = '${TWO_PART}'
and CONSTRAINT_NAME = 'PRIMARY'`,
);
const ordinalByName = Object.fromEntries(
res[0].map((r: any) => [r.column_name, Number(r.ordinal_position)]),
);
// The ordinal is the KEY position, not the column position: shipment_id
// is the table's SECOND column but the key's FIRST member.
expect(ordinalByName).toEqual({ shipment_id: 1, carrier_code: 2 });
});
}
});
}

for (const cell of DIALECT_CELLS) {
declareDialectCell(cell, `${MATRIX} catalog facts`, declareCatalogPins);
}
30 changes: 28 additions & 2 deletions packages/drivers/driver-sql/src/sql-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -12715,14 +12715,32 @@ export class SqlDriver implements IDataDriver {

try {
if (this.isPostgres) {
// `i.indkey` is an `int2vector` holding the key's attnums IN KEY ORDER,
// but `a.attnum = ANY(i.indkey)` is a MEMBERSHIP test: it reads the
// vector as a set and discards the position. With no `ORDER BY`, the row
// order was whatever the plan yielded — measured on PostgreSQL 16.13,
// `pg_attribute` scan order, i.e. COLUMN order. For a table declared
// `(carrier_code, shipment_id, leg_seq)` with `PRIMARY KEY (shipment_id,
// carrier_code)` that is the key REVERSED.
//
// Joining the ORDINALITY of `indkey` keeps the position that the
// membership test threw away, and `ORDER BY k.ord` makes the result the
// declared key order — the same guarantee the SQLite arm below gets from
// sorting on the `PRAGMA table_info` ordinal. The two orders differ
// whenever a key is declared out of column sequence, and this list is
// used as an addressing / upsert-conflict-target key, where the order is
// load-bearing: a key in the wrong order is a DIFFERENT key.
const result = await this.knex.raw(
`
SELECT a.attname as column_name
FROM pg_index i
JOIN pg_attribute a ON a.attrelid = i.indrelid
AND a.attnum = ANY(i.indkey)
CROSS JOIN LATERAL unnest(i.indkey) WITH ORDINALITY AS k(attnum, ord)
JOIN pg_attribute a
ON a.attrelid = i.indrelid
AND a.attnum = k.attnum
WHERE i.indrelid = ?::regclass
AND i.indisprimary
ORDER BY k.ord
`,
[tableName],
);
Expand All@@ -12731,13 +12749,21 @@ export class SqlDriver implements IDataDriver {
primaryKeys.push(row.column_name);
}
} else if (this.isMysql) {
// `KEY_COLUMN_USAGE.ORDINAL_POSITION` IS the key ordinal, and it was
// selected by neither the projection nor an order clause. Without
// `ORDER BY` the row order is unspecified — and measured on MySQL
// 8.0.46 it is COLUMN order, not ordinal order, so an out-of-sequence
// key came back reversed. (The InnoDB folklore that it "tends to"
// return ordinal order does not hold on this shape.) Same reason as the
// Postgres arm above: the order is load-bearing.
const result = await this.knex.raw(
`
SELECT COLUMN_NAME as column_name
FROM information_schema.KEY_COLUMN_USAGE
WHERE TABLE_SCHEMA = DATABASE()
AND TABLE_NAME = ?
AND CONSTRAINT_NAME = 'PRIMARY'
ORDER BY ORDINAL_POSITION
`,
[tableName],
);
Expand Down
Loading