From 4f0b27bcf58eff8842f7c637c2c6e67d094dc6c8 Mon Sep 17 00:00:00 2001 From: Nick Gomez Date: Fri, 20 Feb 2026 00:38:27 -0800 Subject: [PATCH] Fix DROP CONSTRAINT for UNIQUE constraints 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(). --- .../convert_drop_unique_constraint.go | 59 +++++++++ server/analyzer/init.go | 4 +- testing/go/alter_table_test.go | 114 ++++++++++++++++++ 3 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 server/analyzer/convert_drop_unique_constraint.go diff --git a/server/analyzer/convert_drop_unique_constraint.go b/server/analyzer/convert_drop_unique_constraint.go new file mode 100644 index 0000000000..b221a04054 --- /dev/null +++ b/server/analyzer/convert_drop_unique_constraint.go @@ -0,0 +1,59 @@ +// Copyright 2025 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package analyzer + +import ( + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/analyzer" + "github.com/dolthub/go-mysql-server/sql/plan" + "github.com/dolthub/go-mysql-server/sql/transform" +) + +// convertDropUniqueConstraint converts a DropConstraint node dropping a unique constraint into +// an AlterIndex node with IndexAction_Drop that GMS can process to remove the unique index. +func convertDropUniqueConstraint(ctx *sql.Context, _ *analyzer.Analyzer, n sql.Node, _ *plan.Scope, _ analyzer.RuleSelector, _ *sql.QueryFlags) (sql.Node, transform.TreeIdentity, error) { + return transform.Node(n, func(n sql.Node) (sql.Node, transform.TreeIdentity, error) { + dropConstraint, ok := n.(*plan.DropConstraint) + if !ok { + return n, transform.SameTree, nil + } + + rt, ok := dropConstraint.Child.(*plan.ResolvedTable) + if !ok { + return nil, transform.SameTree, analyzer.ErrInAnalysis.New( + "Expected a TableNode for ALTER TABLE DROP CONSTRAINT statement") + } + + table := rt.Table + if it, ok := table.(sql.IndexAddressableTable); ok { + indexes, err := it.GetIndexes(ctx) + if err != nil { + return nil, transform.SameTree, err + } + for _, index := range indexes { + if index.IsUnique() && index.ID() != "PRIMARY" && index.ID() == dropConstraint.Name { + alterDropIndex := plan.NewAlterDropIndex(rt.Database(), rt, dropConstraint.IfExists, dropConstraint.Name) + newNode, err := alterDropIndex.WithTargetSchema(rt.Schema()) + if err != nil { + return n, transform.SameTree, err + } + return newNode, transform.NewTree, nil + } + } + } + + return n, transform.SameTree, nil + }) +} diff --git a/server/analyzer/init.go b/server/analyzer/init.go index 1047bb2457..80323e0615 100644 --- a/server/analyzer/init.go +++ b/server/analyzer/init.go @@ -36,6 +36,7 @@ const ( ruleId_AssignTriggers // assignTriggers ruleId_AssignUpdateCasts // assignUpdateCasts ruleId_ConvertDropPrimaryKeyConstraint // convertDropPrimaryKeyConstraint + ruleId_ConvertDropUniqueConstraint // convertDropUniqueConstraint ruleId_GenerateForeignKeyName // generateForeignKeyName ruleId_ReplaceIndexedTables // replaceIndexedTables ruleId_ReplaceNode // replaceNode @@ -68,7 +69,8 @@ func Init() { analyzer.OnceBeforeDefault = append([]analyzer.Rule{ {Id: ruleId_ApplyTablesForAnalyzeAllTables, Apply: applyTablesForAnalyzeAllTables}, - {Id: ruleId_ConvertDropPrimaryKeyConstraint, Apply: convertDropPrimaryKeyConstraint}}, + {Id: ruleId_ConvertDropPrimaryKeyConstraint, Apply: convertDropPrimaryKeyConstraint}, + {Id: ruleId_ConvertDropUniqueConstraint, Apply: convertDropUniqueConstraint}}, analyzer.OnceBeforeDefault...) // We remove several validation rules and substitute our own diff --git a/testing/go/alter_table_test.go b/testing/go/alter_table_test.go index a04591baa8..d2fe671c12 100644 --- a/testing/go/alter_table_test.go +++ b/testing/go/alter_table_test.go @@ -189,6 +189,120 @@ func TestAlterTable(t *testing.T) { }, }, }, + { + Name: "Drop unique constraint added via ALTER TABLE", + SetUpScript: []string{ + "CREATE TABLE t1 (pk int PRIMARY KEY, c1 int, c2 int);", + "INSERT INTO t1 VALUES (1, 10, 20);", + "ALTER TABLE t1 ADD CONSTRAINT uniq_c1c2 UNIQUE (c1, c2);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "INSERT INTO t1 VALUES (2, 10, 20);", + ExpectedErr: "unique", + }, + { + Query: "ALTER TABLE t1 DROP CONSTRAINT uniq_c1c2;", + Expected: []sql.Row{}, + }, + { + Query: "INSERT INTO t1 VALUES (2, 10, 20);", + Expected: []sql.Row{}, + }, + { + Query: "SELECT * FROM t1 ORDER BY pk;", + Expected: []sql.Row{{1, 10, 20}, {2, 10, 20}}, + }, + }, + }, + { + Name: "Drop unique constraint defined inline in CREATE TABLE", + SetUpScript: []string{ + "CREATE TABLE t1 (pk int PRIMARY KEY, c1 int, c2 int, CONSTRAINT uniq_inline UNIQUE (c1, c2));", + "INSERT INTO t1 VALUES (1, 10, 20);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "INSERT INTO t1 VALUES (2, 10, 20);", + ExpectedErr: "unique", + }, + { + Query: "ALTER TABLE t1 DROP CONSTRAINT uniq_inline;", + Expected: []sql.Row{}, + }, + { + Query: "INSERT INTO t1 VALUES (2, 10, 20);", + Expected: []sql.Row{}, + }, + }, + }, + { + Name: "Drop unique constraint with IF EXISTS", + SetUpScript: []string{ + "CREATE TABLE t1 (pk int PRIMARY KEY, c1 int);", + "ALTER TABLE t1 ADD CONSTRAINT uniq_c1 UNIQUE (c1);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "ALTER TABLE t1 DROP CONSTRAINT uniq_c1;", + Expected: []sql.Row{}, + }, + { + Query: "ALTER TABLE t1 DROP CONSTRAINT uniq_c1;", + ExpectedErr: "does not exist", + }, + { + Query: "ALTER TABLE t1 DROP CONSTRAINT IF EXISTS uniq_c1;", + Expected: []sql.Row{}, + }, + }, + }, + { + Name: "Drop unique constraint on single column", + SetUpScript: []string{ + "CREATE TABLE t1 (pk int PRIMARY KEY, email varchar(256), CONSTRAINT uniq_email UNIQUE (email));", + "INSERT INTO t1 VALUES (1, 'a@b.com');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "INSERT INTO t1 VALUES (2, 'a@b.com');", + ExpectedErr: "unique", + }, + { + Query: "ALTER TABLE t1 DROP CONSTRAINT uniq_email;", + Expected: []sql.Row{}, + }, + { + Query: "INSERT INTO t1 VALUES (2, 'a@b.com');", + Expected: []sql.Row{}, + }, + }, + }, + { + Name: "Drop and re-add unique constraint with different columns", + SetUpScript: []string{ + "CREATE TABLE t1 (pk int PRIMARY KEY, c1 int, c2 int, c3 int);", + "ALTER TABLE t1 ADD CONSTRAINT uniq1 UNIQUE (c1, c2);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "ALTER TABLE t1 DROP CONSTRAINT uniq1;", + Expected: []sql.Row{}, + }, + { + Query: "ALTER TABLE t1 ADD CONSTRAINT uniq1 UNIQUE (c1, c2, c3);", + Expected: []sql.Row{}, + }, + { + Query: "INSERT INTO t1 VALUES (1, 10, 20, 30);", + Expected: []sql.Row{}, + }, + { + Query: "INSERT INTO t1 VALUES (2, 10, 20, 30);", + ExpectedErr: "unique", + }, + }, + }, { Name: "Add Primary Key", SetUpScript: []string{