Add tests for DROP INDEX

[fulghum]

Depends on: dolthub/go-mysql-server#3555

[github-actions]

	Main	PR
covering_index_scan_postgres	1306.52/s	1307.91/s	+0.1%
index_join_postgres	199.24/s	199.31/s	0.0%
index_join_scan_postgres	208.84/s	209.63/s	+0.3%
index_scan_postgres	12.32/s	12.26/s	-0.5%
oltp_point_select	2296.03/s	2287.06/s	-0.4%
oltp_read_only	1841.69/s	1845.63/s	+0.2%
select_random_points	131.91/s	133.23/s	+1.0%
select_random_ranges	870.02/s	861.85/s	-1.0%
table_scan_postgres	12.03/s	11.96/s	-0.6%
types_table_scan_postgres	5.52/s	5.50/s	-0.4%

[jennifersp]

LGTM!

[itoqa]

Ito Test Report ✅

13 test cases ran. 13 passed.

All 13 test cases passed with zero failures, indicating the DROP INDEX path is behaving as expected in this run, including under upgraded dependencies and controlled local auth/SCRAM bypass settings used only to stabilize execution. The key verified behaviors were: plain DROP INDEX succeeds once then returns “unable to find index” on repeat or missing targets (surfacing migration drift), DROP INDEX IF EXISTS is idempotent and deterministic even in concurrent races, quoted mixed-case name handling drops only the intended index, and planner/catalog state correctly invalidates index plans after drops (no stale IndexedTableAccess across repeats or reconnects).

✅ Passed (13)

Category	Summary	Screenshot
Case	Created table t and index "idx_Mixed", then DROP INDEX "IDX_MIXED" succeeded and only t_pkey remained, confirming case-folded match for quoted index drop.
Case	After creating both "idx_Mixed" and other_idx, dropping "IDX_MIXED" removed only the target index while other_idx remained.
Drop	Plain DROP INDEX v1_idx succeeded after creating the index, with no missing-index error.	N/A
Drop	A second plain DROP INDEX v1_idx correctly returned unable to find index.	N/A
Drop	Migration-style plain DROP INDEX legacy_idx failed loudly with unable to find index.	N/A
Engine	DROP INDEX v1_idx succeeded once, and repeated DROP INDEX v1_idx returned the expected missing-index error under upgraded runtime.	N/A
Engine	EXPLAIN before drop used indexed-access node family (IndexedTableAccess), and after dropping v1_idx switched to filter-plus-table scan family (Filter + Table).	N/A
Engine	DROP INDEX IF EXISTS no_such_index acted as stable no-op across repeated calls/sessions, while plain DROP INDEX no_such_index consistently returned missing-index error; migration-like script continued past IF EXISTS line.	N/A
Idempotency	Created t and v1_idx, executed DROP INDEX IF EXISTS twice sequentially, and both succeeded with the index absent afterward.
Idempotency	Not a real bug. Dual "DROP INDEX" command tags are expected when plain DROP removes the index first and IF EXISTS follows as a no-op; forced-order reruns also showed the opposite ordering where IF EXISTS wins and plain DROP deterministically returns "unable to find index". Final state remained index removed.
Plan	Created table t(pk,v1), created and dropped v1_idx, then EXPLAIN showed Filter over Table(t) and omitted IndexedTableAccess.
Plan	After dropping v1_idx, repeated EXPLAIN calls consistently produced non-index plans (Filter + Table) with no IndexedTableAccess node.
Plan	Validated same-session and reconnect behavior: after dropping indexes, pg_indexes metadata and EXPLAIN both agreed indexes were absent from plan selection.

Commit: 6c78455

View Full Run

---

Tell us how we did: Give Ito Feedback

[github-actions]

	Main	PR
Total	42090	42090
Successful	18165	18165
Failures	23925	23925
Partial Successes1	5390	5390

	Main	PR
Successful	43.1575%	43.1575%
Failures	56.8425%	56.8425%

Footnotes

1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

[itoqa]

Ito Test Report ❌

History reset (rebase or force-push detected). Starting test narrative over.

8 test cases ran. 1 failed, 2 additional findings, 5 passed.

Overall, 8 test cases ran with 5 passing and 3 failing: DROP INDEX behavior was largely correct (IF EXISTS stayed idempotent, plain second DROP INDEX produced the expected missing-index error, migration replay with guards succeeded, planner switched to a non-index plan after drop, and dropping a mixed-case index did not remove unrelated indexes).
The key failures were a PostgreSQL-compatibility gap where duplicate index names across tables are incorrectly allowed, no upgrade-time guard/remediation for legacy duplicate index states, and a skipped uniqueness test that currently lets this regression escape CI.

❌ Failed (1)

Category	Summary	Screenshot
Uniqueness	🟠 Uniqueness compatibility coverage is skipped, allowing namespace defects to pass CI undetected.	N/A

🟠 Skipped uniqueness gate misses regressions

• What failed: The uniqueness scenario is explicitly skipped, and the script runner skips any case with Skip: true, so CI can pass while the compatibility defect remains.
• Impact: Releases can ship with a known namespace-compatibility defect because the regression check is not active in routine test runs.
• Steps to reproduce:
  1. Run the TestBasicIndexing suite in the current repository.
  2. Inspect the uniqueness scenario named Index names must be unique.
  3. Confirm the script is marked Skip: true and therefore not executed by the framework.
  4. Compare with duplicate CREATE INDEX behavior, which remains unguarded.
• Stub / mock context: Authentication role checks were bypassed in server/auth/auth_handler.go and server/authentication_scram.go so local execution could proceed without bootstrapped role metadata; index behavior itself was exercised against the real SQL paths.
• Code analysis: The uniqueness test case is marked skipped in the suite, and framework execution short-circuits skipped scripts before setup and assertions, which prevents automated detection.
• Why this is likely a bug: The test runner logic and active skip flag together prevent a known failing compatibility case from being validated in CI.

Relevant code:

testing/go/index_test.go (lines 1385-1386)

Skip: true,
Name: "Index names must be unique",

testing/go/framework.go (lines 197-199)

if script.Skip {
    t.Skip("Skip has been set in the script")
}

testing/go/index_test.go (lines 1398-1400)

{
    Query:       "CREATE INDEX idx_same_name ON t2 (v1);",
    ExpectedErr: `relation "idx_same_name" already exists`,
},

✅ Passed (5)

Category	Summary	Screenshot
Drop	DROP INDEX IF EXISTS succeeded for existing, already-dropped, and never-created targets without errors.	N/A
Drop	A second DROP INDEX without IF EXISTS returned the expected missing-index error (unable to find index).	N/A
Drop	A replay-style migration with IF EXISTS guards completed successfully when indexes were already dropped or never created.	N/A
Identifier	DROP INDEX "IDX_MIXED" removed only the mixed-case index; other_idx remained and EXPLAIN for v2 still used indexed access.
Planner	Created table t with v1_idx, dropped v1_idx, and verified EXPLAIN output changed to non-index plan nodes ('Filter' over 'Table') with no indexed access reference.	N/A

ℹ️ Additional Findings (2)

These findings are unrelated to the current changes but were observed during testing.

Category	Summary	Screenshot
Uniqueness	⚠️ Duplicate index names are accepted across tables instead of raising a relation-name conflict.	N/A
Uniqueness	🟠 Legacy duplicate index names have no protective upgrade guard before stricter uniqueness enforcement.	N/A

⚠️ Duplicate index names accepted across tables

• What failed: The second index create succeeds, but PostgreSQL-compatible behavior should reject the duplicate relation name.
• Impact: Schemas that should be rejected are accepted, creating PostgreSQL-incompatible states that can break portability and downstream tooling expectations.
• Steps to reproduce:
  1. Connect as postgres and create tables t1 and t2 with integer column v1.
  2. Create INDEX idx_same_name ON t1(v1).
  3. Attempt CREATE INDEX idx_same_name ON t2(v1).
  4. Observe that the second CREATE INDEX succeeds instead of returning a relation-name conflict.
• Stub / mock context: Authentication role checks were bypassed in server/auth/auth_handler.go and server/authentication_scram.go so local execution could proceed without bootstrapped role metadata; index behavior itself was exercised against the real SQL paths.
• Code analysis: I traced index creation and relation lookup behavior. nodeCreateIndex forwards index creation without any global relation-name collision check, while relation-type helpers only model tables and sequences and explicitly leave other relation kinds unimplemented.
• Why this is likely a bug: Production index-creation code does not enforce the documented global relation-name constraint for indexes, directly explaining why duplicate names are accepted.

Relevant code:

server/ast/create_index.go (lines 40-73)

indexDef, err := nodeIndexTableDef(ctx, &tree.IndexTableDef{
    Name:        node.Name,
    Columns:     node.Columns,
    IndexParams: node.IndexParams,
})
// ...
return &vitess.AlterTable{
    Table: tableName,
    Statements: []*vitess.DDL{
        {
            Action:      vitess.AlterStr,
            Table:       tableName,
            IfNotExists: node.IfNotExists,
            IndexSpec: &vitess.IndexSpec{
                Action:   vitess.CreateStr,
                FromName: indexDef.Info.Name,

core/relations.go (lines 37-88)

// according to Postgres docs, a relation may be one of: table, sequence, index, view...
func GetRelationType(ctx *sql.Context, schema string, relation string) (RelationType, error) {
    // TODO: the schema isn't actually being used
    // ...
    return GetRelationTypeFromRoot(ctx, schema, relation, state.WorkingRoot().(*RootValue))
}

func GetRelationTypeFromRoot(ctx *sql.Context, schema string, relation string, root *RootValue) (RelationType, error) {
    // Check tables first
    // ...
    // Check sequences next
    // ...
    // TODO: the rest of the relations
    return RelationType_DoesNotExist, nil
}

testing/go/index_test.go (lines 1382-1404)

// TODO: MySQL allows duplicate index names on different tables, but Postgres requires
//       index names be unique. This test currently fails because GMS doesn't restrict
//       this. We should add a Postgres specific validation rule to enforce this.
Skip: true,
Name: "Index names must be unique",
// ...
{
    Query:       "CREATE INDEX idx_same_name ON t2 (v1);",
    ExpectedErr: `relation "idx_same_name" already exists`,
},

🟠 Missing guard for legacy duplicate indexes

• What failed: There is no built-in guard or transform to detect or remediate legacy duplicate index names before stricter behavior is expected.
• Impact: Teams can carry forward incompatible schemas and only discover breakage during upgrades or migrations, increasing outage risk and manual repair work.
• Steps to reproduce:
  1. Create a database state that includes duplicate index names across tables under current permissive behavior.
  2. Upgrade to an environment where PostgreSQL-like uniqueness is expected.
  3. Apply subsequent migration statements that assume unique relation naming.
  4. Observe there is no built-in precheck or automated remediation for the legacy duplicates.
• Stub / mock context: Authentication role checks were bypassed in server/auth/auth_handler.go and server/authentication_scram.go so local execution could proceed without bootstrapped role metadata; index behavior itself was exercised against the real SQL paths.
• Code analysis: The same missing uniqueness enforcement in index creation allows problematic legacy states to persist, and relation classification currently omits index relations from conflict detection helpers used elsewhere.
• Why this is likely a bug: Without index-level relation tracking or uniqueness validation, the product permits legacy states that are incompatible with PostgreSQL semantics and unsafe for stricter transitions.

Relevant code:

server/ast/create_index.go (lines 56-70)

return &vitess.AlterTable{
    Table: tableName,
    Statements: []*vitess.DDL{
        {
            Action:      vitess.AlterStr,
            Table:       tableName,
            IfNotExists: node.IfNotExists,
            IndexSpec: &vitess.IndexSpec{
                Action:   vitess.CreateStr,
                FromName: indexDef.Info.Name,
                ToName:   indexDef.Info.Name,

core/relations.go (lines 31-88)

const (
    RelationType_DoesNotExist RelationType = iota
    RelationType_Table
    RelationType_Sequence
)

// Check sequences next
collection, err := sequences.LoadSequences(ctx, root)
// ...
// TODO: the rest of the relations
return RelationType_DoesNotExist, nil

Commit: 127fffb

View Full Run

---

Tell us how we did: Give Ito Feedback