Fix drop constraint unique pk

[anshikavashistha]

Summary

This PR extends the ALTER TABLE ... DROP CONSTRAINT <name> support in Vitess to correctly handle the dropping of named PRIMARY KEY and UNIQUE constraints, aligning with MySQL 8.0.19+ behavior.

Previously, PR #19183 added support for dropping FOREIGN KEY and CHECK constraints using named constraints, but did not handle named UNIQUE and PRIMARY KEY constraints.

The root cause was that DROP CONSTRAINT logic only searched TableSpec.Constraints. However, named UNIQUE and PRIMARY KEY constraints are stored in TableSpec.Indexes under IndexDefinition.ConstraintName.

With this change, users can now drop explicitly named PRIMARY KEY or UNIQUE constraints using standard ALTER TABLE ... DROP CONSTRAINT <name> syntax, matching MySQL behavior.

What changed?

• DROP CONSTRAINT handling extended – Now searches both TableSpec.Constraints and TableSpec.Indexes
• Named PRIMARY KEY constraints – Properly removed when dropped
• Named UNIQUE constraints – Properly removed when dropped
• Unit tests added – Comprehensive test coverage for both constraint types
• Backward compatibility – All existing behavior preserved
• MySQL 8.0.19+ alignment – Behavior now matches MySQL standard

Related Issue(s)

Fixes #19232

Checklist

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

Deployment Notes

None.

[codecov]

Codecov Report

❌ Patch coverage is 88.23529% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.73%. Comparing base (06f2aa7) to head (0d14873).
⚠️ Report is 19 commits behind head on main.

Files with missing lines	Patch %	Lines
go/vt/sqlparser/parser.go	72.22%	5 Missing ⚠️
go/vt/schemadiff/onlineddl.go	84.61%	2 Missing ⚠️
go/vt/schemadiff/table.go	95.23%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #19420      +/-   ##
==========================================
+ Coverage   69.62%   69.73%   +0.11%     
==========================================
  Files        1614     1614              
  Lines      216398   216870     +472     
==========================================
+ Hits       150658   151235     +577     
+ Misses      65740    65635     -105     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[anshikavashistha]

@mattlord ,please take a look.

[mhamza15]

You accidentally left this Go toolchain artifact.

[anshikavashistha]

I have removed this file

[mhamza15]

I might be missing something, but why do we need this?

[anshikavashistha]

Good catch on the inconsistency. Named UNIQUE/PRIMARY constraints stored in IndexInfo.ConstraintName are part of MySQL's schema-wide constraint namespace, so they do need renaming during table duplication — but you're right that "_" suffix is wrong here. I've updated this to use generateConstraintNameWithHash consistent with how FK/check constraints are handled.

[mhamza15]

This logic is really brittle. It matches the substring anywhere, including inside string literals, comments, or DEFAULT values. For example, drop constraint primary_key would become invalid drop primary key_key.

Furthermore, DROP CONSTRAINT PRIMARY (unquoted) is not actually valid MySQL syntax. PRIMARY is a reserved keyword, not a constraint symbol. MySQL docs state that the type-specific form must be used for primary keys:

The SQL standard specifies that all types of constraints (primary key, unique index, foreign key, check) belong to the same namespace. In MySQL, each constraint type has its own namespace per schema. Consequently, names for each type of constraint must be unique per schema, but constraints of different types can have the same name. When multiple constraints have the same name, DROP CONSTRAINT and ADD CONSTRAINT are ambiguous and an error occurs. In such cases, constraint-specific syntax must be used to modify the constraint. For example, use DROP PRIMARY KEY or DROP FOREIGN KEY to drop a primary key or foreign key.

https://dev.mysql.com/doc/refman/8.4/en/alter-table.html

I suggest we drop this logic and any related logic. Was there a reason this was added in the first place? Am I missing some Vitess-specific behavior that requires this? In the case we would need this, it would better fit in the grammar and not as string replacements.

[anshikavashistha]

You're absolutely right — the substring rewrite in parser.go was brittle and DROP CONSTRAINT PRIMARY is not valid MySQL syntax at the parser level.
I've removed the entire pre-parse string replacement block from parser.go and reverted ParseStrictDDL to its original state.

[mhamza15]

This may not needed right now, but MySQL returns an error when multiple constraints have the same name (when using DROP CONSTRAINT). We should likely match that behavior.