Fix ALTER TABLE ... DROP CONSTRAINT for UNIQUE constraints#2359
Fix ALTER TABLE ... DROP CONSTRAINT for UNIQUE constraints#2359nick-inkeep wants to merge 1 commit intodolthub:mainfrom
Conversation
2517ea9 to
75bd0c1
Compare
Add convertDropUniqueConstraint analyzer rule that intercepts plan.DropConstraint nodes for UNIQUE constraints and converts them to plan.NewAlterDropIndex nodes so GMS can process the index drop. Previously, DROP CONSTRAINT only handled CHECK, FOREIGN KEY, and PRIMARY KEY constraints. UNIQUE constraints were missed because GMS's default DropConstraint handler checks GetChecks() and GetDeclaredForeignKeys() but never calls GetIndexes().
75bd0c1 to
4f0b27b
Compare
|
Thank you for this excellent write up! I've repro'ed the issue and am working through the code in the debugger. This looks to also be an issue with Dolt, where Dolt has a similar behavior that does not match MySQL's behavior. Based on that, I'm going to keep digging in deeper and I suspect there is a fix in GMS that will make this work for both Doltgres and Dolt. Thank you for finding and reporting this one! |
|
Thank you for reporting this one! 🙏 Your description and repro case were a big help for me to make progress on this one quickly. I noticed Dolt wasn't able to execute this statement correctly either and made a fix in GMS here: dolthub/go-mysql-server#3441 I've got that fix merged into Dolt and I'm waiting on one last CI job to complete before Doltgres is updated. After that I'll kick off a release for Doltgres version 0.55.4. Since we made another fix in GMS for this issue, I'll go ahead and close out this PR. Let us know if you come across any other problems and we'll be happy to fix them. |
Summary
ALTER TABLE ... DROP CONSTRAINT "name"fails withConstraint "name" does not existwhen the constraint is a UNIQUE constraint. This affects both inline (CREATE TABLE ... UNIQUE(...)) and ALTER-added (ALTER TABLE ADD CONSTRAINT ... UNIQUE(...)) UNIQUE constraints.DROP CONSTRAINT works correctly for CHECK, FOREIGN KEY, and PRIMARY KEY. Only UNIQUE is broken.
Impact: Blocks any ORM/migration tool (Drizzle, Prisma, Knex) that generates standard PostgreSQL
DROP CONSTRAINTfor UNIQUE constraint changes.Root cause
The existing
convertDropPrimaryKeyConstraintanalyzer rule interceptsplan.DropConstraintand converts PK drops toplan.AlterDropPk. No equivalent rule exists for UNIQUE constraints. GMS's defaultDropConstrainthandler checksGetChecks()andGetDeclaredForeignKeys()but never callsGetIndexes(), so UNIQUE constraints (stored assql.IndexwithIsUnique()=true) are never found.Fix
Added
convertDropUniqueConstraintanalyzer rule mirroring the PK handler:plan.DropConstraintnodessql.IndexAddressableTable.GetIndexes()plan.NewAlterDropIndexIF EXISTScorrectlyFiles changed
server/analyzer/convert_drop_unique_constraint.goserver/analyzer/init.go(rule ID + registration)testing/go/alter_table_test.go(5 new test cases)Test coverage
Reproduction