|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#11379] `introspectForeignKeys`' MySQL arm must ORDER BY the key ordinal. |
| 5 | + * |
| 6 | + * ## Why this pin is structural, and why that is the honest shape |
| 7 | + * |
| 8 | + * The card was filed as an OBSERVATION, and it says so up front: it **did not |
| 9 | + * reproduce**. Re-measured here on live MySQL 8.0.46 before this pin was |
| 10 | + * written, with the reporter's own fixture — a key declared out of column |
| 11 | + * sequence, `foreign key (second_col, first_col) references ooo_parent |
| 12 | + * (pa, pb)`, so that "key order" and "column order" are different answers — |
| 13 | + * the arm's query WITHOUT `ORDER BY` returned: |
| 14 | + * |
| 15 | + * second_col -> pa (ORDINAL_POSITION 1) |
| 16 | + * first_col -> pb (ORDINAL_POSITION 2) |
| 17 | + * |
| 18 | + * which is key order: the correct answer, unpinned. So a behavioural pin — |
| 19 | + * "the columns come back in ordinal order" — is **vacuous** on this predicate. |
| 20 | + * It passes today, it passes with the fix, and it passes with the fix reverted. |
| 21 | + * A green that cannot go red is not evidence, so this file does not write one, |
| 22 | + * and does not dress one up as a guard. |
| 23 | + * |
| 24 | + * ## What was measured that makes the fix more than cosmetic |
| 25 | + * |
| 26 | + * On the SAME server, in the SAME session, against the SAME view, the sibling |
| 27 | + * `introspectPrimaryKeys` predicate — `CONSTRAINT_NAME = 'PRIMARY'` instead of |
| 28 | + * `REFERENCED_TABLE_NAME IS NOT NULL` — read an out-of-sequence primary key |
| 29 | + * `PRIMARY KEY (shipment_id, carrier_code)` back as: |
| 30 | + * |
| 31 | + * carrier_code (ORDINAL_POSITION 2) |
| 32 | + * shipment_id (ORDINAL_POSITION 1) |
| 33 | + * |
| 34 | + * i.e. COLUMN order, the wrong answer — reproducing #11101's measurement |
| 35 | + * exactly. `KEY_COLUMN_USAGE` therefore does NOT preserve the ordinal for free |
| 36 | + * on this server: which of the two orders comes back is decided by the WHERE |
| 37 | + * clause, and nothing declares that. The foreign-key predicate is currently on |
| 38 | + * the lucky side of a choice nobody made. That is what the `ORDER BY` removes, |
| 39 | + * and it is why "it did not reproduce" is not a reason to leave it out. |
| 40 | + * |
| 41 | + * ⛔ Deliberately NOT attempted here: proving that some plan shape on some |
| 42 | + * supported MySQL version returns the foreign-key predicate out of ordinal |
| 43 | + * order. That needs a fixture large enough to change the plan, and the card |
| 44 | + * rules it out as beyond what an observation should spend. |
| 45 | + * |
| 46 | + * ## So the pin is on the emitted SQL, and it can go red |
| 47 | + * |
| 48 | + * Removing `ORDER BY ORDINAL_POSITION` from the arm turns the first test in |
| 49 | + * this file red — verified by doing it, not by assuming it. That is the whole |
| 50 | + * claim this file makes, and it is stated no more strongly than that. |
| 51 | + * |
| 52 | + * ⚠️ It is a pin on **this method's** emitted statement, captured at the knex |
| 53 | + * seam — never a grep of the source file for the literal. `sql-driver.ts` |
| 54 | + * contains `ORDER BY ORDINAL_POSITION` three times (`introspectColumnOrder`, |
| 55 | + * this method, and `introspectPrimaryKeys`), so a file-level match would report |
| 56 | + * this arm as fixed while it was still unordered — which is exactly how a live |
| 57 | + * defect gets closed as already-absorbed. |
| 58 | + * |
| 59 | + * The second test pins the other half of the same contract, which lives in TS |
| 60 | + * rather than in SQL: the arm must EMIT the rows in the order the server |
| 61 | + * returned them. A sort, a `Map` keyed by column name, or a regrouping pass |
| 62 | + * inserted into that loop would silently undo the `ORDER BY` above, and unlike |
| 63 | + * the row order itself, that one is fully determined here and really can fail. |
| 64 | + */ |
| 65 | + |
| 66 | +import { describe, it, expect, afterEach } from 'vitest'; |
| 67 | +import { SqlDriver } from '../src/index.js'; |
| 68 | + |
| 69 | +/** One row of `KEY_COLUMN_USAGE` as the MySQL arm's projection aliases it. */ |
| 70 | +interface FkRow { |
| 71 | + column_name: string; |
| 72 | + referenced_table: string; |
| 73 | + referenced_column: string; |
| 74 | + constraint_name: string; |
| 75 | +} |
| 76 | + |
| 77 | +/** |
| 78 | + * A driver that DECLARES MySQL and answers from a canned result set. |
| 79 | + * |
| 80 | + * `isMysql` is derived from `config.client` and from nothing else, and the |
| 81 | + * constructor already keeps `this.config` as the DECLARED target while the knex |
| 82 | + * instance points somewhere else (#6743 — that split is the documented |
| 83 | + * behaviour of this class, not a hole this test opens). So re-declaring the |
| 84 | + * client after construction drives the REAL dispatch through the REAL getter, |
| 85 | + * while the transport stays an in-memory SQLite handle that is never asked to |
| 86 | + * execute anything. No MySQL server, so this pin runs in every CI job rather |
| 87 | + * than only in the provisioned live-matrix one. |
| 88 | + */ |
| 89 | +class MysqlFkEmissionProbe extends SqlDriver { |
| 90 | + /** Every statement the arm handed to knex, in order. */ |
| 91 | + readonly emitted: { sql: string; bindings: unknown }[] = []; |
| 92 | + |
| 93 | + constructor(private readonly rows: FkRow[]) { |
| 94 | + super({ |
| 95 | + client: 'better-sqlite3', |
| 96 | + connection: { filename: ':memory:' }, |
| 97 | + useNullAsDefault: true, |
| 98 | + }); |
| 99 | + |
| 100 | + (this.config as { client?: string }).client = 'mysql2'; |
| 101 | + |
| 102 | + const knex = this.knex as unknown as Record<string, unknown>; |
| 103 | + // knex defines `raw` as non-writable (but configurable), so a plain |
| 104 | + // assignment throws — the swap has to go through `defineProperty`. |
| 105 | + Object.defineProperty(knex, 'raw', { |
| 106 | + configurable: true, |
| 107 | + value: (sql: unknown, bindings: unknown) => { |
| 108 | + this.emitted.push({ sql: String(sql), bindings }); |
| 109 | + // mysql2 hands knex back `[rows, fields]`; the arm reads `result[0]`. |
| 110 | + return [this.rows, []]; |
| 111 | + }, |
| 112 | + }); |
| 113 | + } |
| 114 | + |
| 115 | + foreignKeys(table: string) { |
| 116 | + return this.introspectForeignKeys(table); |
| 117 | + } |
| 118 | + |
| 119 | + /** The one statement this method emitted. Fails loudly if it was not one. */ |
| 120 | + soleStatement(): string { |
| 121 | + expect( |
| 122 | + this.emitted.length, |
| 123 | + 'introspectForeignKeys emitted no statement, or more than one — the ' + |
| 124 | + 'capture below would be measuring nothing. Did the dialect dispatch ' + |
| 125 | + 'stop reaching the MySQL arm?', |
| 126 | + ).toBe(1); |
| 127 | + return this.emitted[0]!.sql; |
| 128 | + } |
| 129 | +} |
| 130 | + |
| 131 | +/** |
| 132 | + * The reporter's fixture, as rows: `(second_col, first_col)` referencing |
| 133 | + * `(pa, pb)` — a key declared out of column sequence, so key order and column |
| 134 | + * order are different answers and an accidental sort is visible. |
| 135 | + */ |
| 136 | +const OUT_OF_SEQUENCE_ROWS: FkRow[] = [ |
| 137 | + { |
| 138 | + column_name: 'second_col', |
| 139 | + referenced_table: 'ooo_parent', |
| 140 | + referenced_column: 'pa', |
| 141 | + constraint_name: 'fk_ooo', |
| 142 | + }, |
| 143 | + { |
| 144 | + column_name: 'first_col', |
| 145 | + referenced_table: 'ooo_parent', |
| 146 | + referenced_column: 'pb', |
| 147 | + constraint_name: 'fk_ooo', |
| 148 | + }, |
| 149 | +]; |
| 150 | + |
| 151 | +describe('introspectForeignKeys (MySQL) orders a composite key by the ordinal (#11379)', () => { |
| 152 | + let probe: MysqlFkEmissionProbe | undefined; |
| 153 | + |
| 154 | + afterEach(async () => { |
| 155 | + await (probe as unknown as { knex?: { destroy(): Promise<void> } } | undefined)?.knex?.destroy(); |
| 156 | + probe = undefined; |
| 157 | + }); |
| 158 | + |
| 159 | + it('emits ORDER BY ORDINAL_POSITION on the KEY_COLUMN_USAGE read', async () => { |
| 160 | + probe = new MysqlFkEmissionProbe(OUT_OF_SEQUENCE_ROWS); |
| 161 | + await probe.foreignKeys('ooo_child'); |
| 162 | + |
| 163 | + const sql = probe.soleStatement(); |
| 164 | + |
| 165 | + // Control first: the captured statement really is the foreign-key read of |
| 166 | + // this method, not some other statement that happened past the seam. Without |
| 167 | + // this, the assertion below could go green on the wrong query — the |
| 168 | + // file-level-grep failure mode, one layer in. |
| 169 | + expect(sql).toMatch(/information_schema\.KEY_COLUMN_USAGE/i); |
| 170 | + expect(sql).toMatch(/REFERENCED_TABLE_NAME IS NOT NULL/i); |
| 171 | + expect(sql).not.toMatch(/CONSTRAINT_NAME\s*=\s*'PRIMARY'/i); |
| 172 | + |
| 173 | + // The pin: the ordinal clause is in THIS statement, and it comes after the |
| 174 | + // predicate that identifies it, so it cannot be satisfied by a clause that |
| 175 | + // belongs to a different read. |
| 176 | + expect(sql).toMatch(/REFERENCED_TABLE_NAME IS NOT NULL[\s\S]*ORDER BY\s+ORDINAL_POSITION/i); |
| 177 | + }); |
| 178 | + |
| 179 | + it('emits the rows in the order the server returned them', async () => { |
| 180 | + probe = new MysqlFkEmissionProbe(OUT_OF_SEQUENCE_ROWS); |
| 181 | + const keys = await probe.foreignKeys('ooo_child'); |
| 182 | + |
| 183 | + // `IntrospectedForeignKey` is a flat per-column record with no ordinal |
| 184 | + // field, so ORDERED SIBLING ROWS is the only way a composite key is |
| 185 | + // expressed (#11324). Re-sorting or regrouping in the arm would undo the |
| 186 | + // `ORDER BY` above without touching the SQL. |
| 187 | + expect(keys.map((k) => `${k.columnName} -> ${k.referencedTable}.${k.referencedColumn}`)).toEqual([ |
| 188 | + 'second_col -> ooo_parent.pa', |
| 189 | + 'first_col -> ooo_parent.pb', |
| 190 | + ]); |
| 191 | + expect(keys.every((k) => k.constraintName === 'fk_ooo')).toBe(true); |
| 192 | + }); |
| 193 | +}); |
0 commit comments