Skip to content

fix(structure): let a save reuse the name of an index or check constraint it deletes - #3155

Open
datlechin wants to merge 1 commit into
mainfrom
fix/structure-pending-deletion-duplicates
Open

datlechin wants to merge 1 commit into
mainfrom
fix/structure-pending-deletion-duplicates

Conversation

@datlechin

Copy link
Copy Markdown
Member

Follows #3147, which rewrote column validation in the same function.

Root cause

StructureChangeManager.validate() grouped index names and check constraint names over every working row, filtered only on isValid. A row struck through for deletion kept its name in the group. So each of these saves was refused with Duplicate index name: idx_a under "Some Changes Are Incomplete":

  • delete idx_a and add a new idx_a;
  • Duplicate an index, edit the copy, then delete the original;
  • delete idx_a and rename idx_b to idx_a.

Check constraints behaved the same way. The generator never needed that caution: sortByDependency runs every index drop and every check constraint drop before any add or rename. #3147 already stopped counting deleted columns.

The same grouping also blocked saves that left the group alone. SQLite accepts CONSTRAINT c CHECK (a > 0), CONSTRAINT c CHECK (a < 10) and the tab lists both, so every Structure save on such a table was refused.

"A deleted name is free" is not the whole answer, because three paths relied on the old check without knowing it:

  • SQLite. DROP CONSTRAINT c removes the first constraint named c in the table's text, and names compare without case: on a table holding c and C, DROP CONSTRAINT "C" dropped c. Once deletions stop counting, deleting one of two same-named constraints could drop the other.
  • DynamoDB. It takes one GSI create or delete per UpdateTable and refuses another request while the table is UPDATING. The driver sends one request per change. A delete and a same-name add were refused by the duplicate check by accident; without it, the delete would run and the add would fail.
  • MySQL and MariaDB. Neither lets any index take the name PRIMARY: they refuse it with ERROR 1280, in any letter case. Duplicating the PRIMARY row and deleting the original was also refused by accident. Without that, a save that also changes a column splits the replacement: DROP INDEX PRIMARY commits, the add fails, and the table has no key. Editing the PRIMARY row in place alongside a column change already did this on main.

Fix

  • Duplicate names. flagDuplicateNames replaces both hand-written blocks, at the same positions. It groups the rows the table keeps, leaving out pending deletions. A group blocks the save only when the save staged one of its rows, the rule fix(structure): hold only the columns a save changes to having a name and a type #3147 uses for columns.
  • Shared constraint names. flagChangesToASharedConstraintName groups loaded check constraints by name, ignoring case. When some but not all of a group are changed or deleted, it refuses the changed ones with "More than one check constraint is named %@. Change or delete all of them in the same save." Changing every one of them saves, because each is dropped by name and added back from its own definition. This check covers check constraints only: no engine lists two indexes under one name, and folding case there would only refuse PostgreSQL's case-distinct names.
  • Replacements. SchemaStatementGenerator.replacingIndexesInPlace pairs a .deleteIndex and an .addIndex of the same name into .modifyIndex(old:new:), which is what PluginSchemaOperation.modifyIndex describes. sortByDependency pairs through it and the refusal loop reads the same list, so what the driver is asked about matches what it writes. Results by engine:
    • MySQL and MariaDB, when the save changes no column: one ALTER TABLE … DROP INDEX idx_a, ADD INDEX idx_a (…), which keeps the old index if the server rejects the new one.
    • Every other engine, and any save with a column change: the same drop then add as before.
    • DynamoDB: its existing .modifyIndex refusal answers the same-name case before anything runs.
  • Refusal order. SchemaOperationRefusal asks about .dropIndex(old) before .modifyIndex and .addIndex(new), so DynamoDB's primary key and local indexes get their real reason instead of "delete it and save".
  • MySQL PRIMARY. The MySQL driver refuses an added index named PRIMARY in any letter case, before anything runs.
  • Names handed along in one save. Once a deleted or renamed row's name counts as free, the save has to run in an order that frees it first. SchemaStatementGenerator.inNameOrder orders renames and one-statement index replacements so none takes a name before the change holding it has let it go, keeping staging order otherwise. Two or more that hold each other's names in a cycle, such as a swap, are each split into a drop, run with the other drops, and an add. Names compare without case for this, as MySQL, MariaDB and SQLite resolve them.
  • Check constraint names by case. c and C are one name to SQLite and MariaDB, so the duplicate check groups constraint names without case and refuses the pair before anything runs.
  • SQL Server clustered copies. A CLUSTERED index added by Duplicate or paste is re-decided after every change: CLUSTERED when no index the table keeps holds the place, NONCLUSTERED beside one that does. So Duplicate, edit, then delete the original ends with a clustered index, and undoing the delete hands the place back. A copy whose type the user picks keeps it.
  • Docs. table-structure.mdx covers replacing an index, PRIMARY, reusing a constraint name and SQLite's same-named constraints. dynamodb.mdx covers one index change per save.

Verified

  • Unit tests. 158 of 158 passed across 11 suites. New cases: 19 in StructureChangeNameReuseTests, 6 in SchemaStatementGeneratorPluginTests, 2 in CheckConstraintStatementTests, 5 in SchemaOperationRefusalTests, 2 through the real DynamoDB driver and 1 for MySQL PRIMARY.
  • Unit red check. StructureChangeNameReuseTests against the base validator: 13 of 19 failed.
  • UI test. StructureIndexNameReuseUITests passed, and it failed against the base validator with "Some Changes Are Incomplete Duplicate index name: idx_tag". It seeds idx_body on body and idx_tag on tag, deletes one, renames the other onto its name, saves, and asserts sqlite_master holds only the deleted name, now defined on the other column.
  • SQLite 3.54.0. DROP CONSTRAINT c takes the first of two constraints named c, and DROP CONSTRAINT "C" takes c. Two drops remove both. Drop, drop, then add leaves exactly the rewritten constraint. CREATE INDEX idx_a works straight after DROP INDEX idx_a.
  • PostgreSQL 17.11. In one transaction, DROP INDEX idx_a then CREATE INDEX idx_a, DROP CONSTRAINT c then ADD CONSTRAINT c, and DROP CONSTRAINT c then RENAME CONSTRAINT d TO c all work. "c" and "C" coexist on one table.
  • MariaDB 13.0.2.
    • ADD INDEX PRIMARY, primary and Primary each fail with 1280.
    • Split into DROP INDEX PRIMARY then ADD UNIQUE INDEX PRIMARY, the table is left with no key.
    • DROP INDEX idx_a, ADD UNIQUE INDEX idx_a (b) over duplicate values fails with 1062 and keeps idx_a. The split form loses it.
  • Codex review, round 1. A P1 that a rename chain (c to b, delete a, b to a) ran in staging order and failed partway on MySQL and MariaDB, a P2 that c and C passed as two constraint names, and a P2 that a duplicated clustered index stayed NONCLUSTERED after its original was deleted, leaving the table with none. All three are fixed above. 12 of the new cases fail against the round-0 sources, and all pass with the fix.
  • Rebased onto main at c21dc512e. 7 unit suites plus StringCatalogIntegrityTests: 134 of 134. StructureIndexNameReuseUITests: 2 of 2.
  • Other checks. AllPlugins builds. Lint reports 0 violations on the 12 changed Swift files. verify.sh docs passes.

Deliberately not fixed here

  • DynamoDB, two index changes under different names. A save with two GSI changes still sends two UpdateTable requests, and the second fails after the first has run, as on main. The rule belongs in a save-level review, which feat/mongodb-structure-field-edits adds as reviewSchemaChange. A second batch refusal in PluginKit here would compete with it. The docs now say to save one index change at a time.
  • SQLite constraints the parser does not list. The parser reads only table-level CONSTRAINT name CHECK clauses. A column-level check, UNIQUE or foreign key sharing the name is invisible to the Structure tab. A complete guard belongs in the SQLite driver.
  • Index name case. Duplicate index names still compare exactly, as on main and in fix(structure): hold only the columns a save changes to having a name and a type #3147, so idx_a beside IDX_A is refused by the server at save time. PostgreSQL keeps the two apart, so folding case for indexes would refuse a valid save there.
  • Found while designing, each its own change:
    • A SQLite rebuild that drops a column and its index fails.
    • On MySQL, changing a foreign key's columns while keeping its name loses the key.
    • On PostgreSQL, renaming a trigger leaves both triggers in place.

@mintlify

mintlify Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 26, 2026, 5:54 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

This branch was successfully deployed

1 active deployment
staging - docs — faff41df Deployed Sep 26, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant