Skip to content

/{server, testing}: fix drop unique constraint - #3569

Open
coffeegoddd wants to merge 3 commits into
mainfrom
db/fix-3524
Open

coffeegoddd wants to merge 3 commits into
mainfrom
db/fix-3524

Conversation

@coffeegoddd

Copy link
Copy Markdown
Contributor

Fix DROP CONSTRAINT on tables with expression indexes by resolving constraints against the underlying table. Ensure IF EXISTS removes existing unique constraints and schema-qualified drops affect the correct table. Fixes #3524

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 20955 20955
Failures 21135 21135
Partial Successes1 5404 5405
Main PR
Successful 49.7862% 49.7862%
Failures 50.2138% 50.2138%

${\color{red}Regressions (1)}$

subselect

QUERY:          select count(*) from tenk1 t
where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0);
RECEIVED ERROR: timeout during Receive

${\color{lightgreen}Progressions (1)}$

insert_conflict

QUERY: insert into insertconflicttest values (26, 'Peach') on conflict (key) do update set fruit = excluded.fruit;

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

itoqa Bot commented Oct 8, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: d9d9c95: 13 test cases ran, 1 failed ❌, 12 passed ✅.

Summary

Coverage spans database constraint removal across unique, check, primary-key, and foreign-key rules, including preserving unrelated indexes, schema isolation, missing-object behavior, and concurrent operations. It exercises both normal data flows and edge cases involving duplicate data, invalid references, case differences, and multi-schema changes.

Merge with caution — a medium-severity, PR-attributable issue prevents schema-qualified foreign-key removal in multi-schema deployments, blocking an intended database operation while leaving existing data intact. The broader constraint and concurrency behavior appears healthy.

Tests run by Ito

View full run

Result Severity Type Description
❌ Medium severity Rev The foreign-key drop command returns ERROR: table not found: child instead of removing the constraint from billing.child. Both billing.child and archive.child still have child_parent_id_fkey.
✅ — General Two sessions dropped the same unique constraint at the same time. The server stayed usable, the unrelated index remained, and duplicate email values were accepted afterward.
✅ — General Two concurrent requests removed the unique constraint from their requested tables without changing the sibling schema. Both tables then accepted duplicate email values as expected.
✅ — Check The case-insensitive drop removed the CHECK constraint, kept the lower-case email index, and allowed the previously blocked value to be inserted.
✅ — Exists The existing email rule was removed, the separate email index stayed in place, and both email values were accepted.
✅ — Exists The missing constraint was ignored, the existing indexes stayed in place, and the unique rule still rejected an exact duplicate email.
✅ — Exists Trying to drop a constraint that does not exist returns SQLSTATE 42704, and the existing unique constraint stays in place.
✅ — Foreign The named foreign-key constraint was removed despite different letter casing. A child row with parent_id 999 was then inserted and read back successfully.
✅ — Primary The primary key was removed without removing the email expression index, and the table accepted and returned both rows with the same ID.
✅ — Rev Dropping the foreign-key constraint succeeded. The expression index stayed in place, and a child row with an invalid parent was accepted afterward.
✅ — Rev The named unique rule was removed, the separate lowercase index stayed in place, and the table accepted and returned both duplicate rows.
✅ — Schema Dropping the constraint from auth.users allowed duplicate email values there, while public.users kept its constraint and rejected the duplicate.
✅ — Unique The unique email constraint was removed, the lower-case email index stayed available, and both case-variant email rows were inserted and returned by the query.

Tip

Reply with @itoqa to send us feedback on this test run.

"Expected a TableNode for ALTER TABLE DROP CONSTRAINT statement")
}

// Expression indexes introduce virtual columns, whose table wrapper doesn't expose the underlying

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View All Evidence

Medium severity Qualified foreign-key drops fail in another schema

What failed: The foreign-key drop command returns ERROR: table not found: child instead of removing the constraint from billing.child. Both billing.child and archive.child still have child_parent_id_fkey.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Database operators cannot remove the foreign key from the requested schema-qualified table. This can block a schema change, but there is no evidence of data loss or corruption.
  • Steps to Reproduce:
    1. Set the search path to public and create billing and archive schemas.
    2. Create billing.parent and archive.parent, then create billing.child and archive.child with a foreign key named child_parent_id_fkey in each schema.
    3. Create the billing expression index and insert valid parent and child rows.
    4. Run ALTER TABLE ONLY billing.child DROP CONSTRAINT child_parent_id_fkey.
    5. Run ALTER TABLE billing.child DROP CONSTRAINT child_parent_id_fkey as a second qualified attempt and inspect the foreign-key metadata.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The new resolver unwraps the resolved table at lines 42-47, so the target schema remains available as rt.Schema(ctx). For a matching foreign key, however, lines 49-58 call plan.NewAlterDropForeignKey(rt.Database().Name(), rt.Name(), fk.Name) and only attach the database provider; no target schema is supplied. The neighboring unique-index branch at lines 93-98 explicitly calls WithTargetSchema(rt.Schema(ctx)), demonstrating the missing schema propagation. The runtime test created billing.child and archive.child with identical constraint names, set search_path to public, and observed table not found: child for both ALTER TABLE ONLY billing.child and ALTER TABLE billing.child; metadata then showed both schema-qualified foreign keys still present. The smallest fix is to preserve the resolved schema when constructing the foreign-key drop operation, using the corresponding target-schema mechanism supported by the AlterDropForeignKey plan node, or otherwise execute it against the already resolved schema-qualified table rather than reacquiring an unqualified child table.
  • Why this is likely a bug: The requested behavior is independently stated in the PR description: schema-qualified drops must affect only the requested table. The failure is deterministic with two schema-qualified syntaxes and is not caused by missing setup, because both schemas, both tables, their parent rows, the child rows, and the duplicate foreign-key names were created successfully before the drop. The resolver has already identified the qualified table, but the new foreign-key operation discards that schema before execution. A targeted schema-preserving change in this PR's new branch should allow billing.child to be changed without touching archive.child.
Relevant code

server/analyzer/resolve_drop_constraint.go:42-58

resolvedTable := *rt
resolvedTable.Table = sql.GetUnderlyingTable(rt.Table)
rt = &resolvedTable
...
if fkt, ok := table.(sql.ForeignKeyTable); ok {
	...
	if strings.EqualFold(fk.Name, dropConstraint.Name) {
		newNode, err := plan.NewAlterDropForeignKey(rt.Database().Name(), rt.Name(), fk.Name).
			WithDatabaseProvider(a.Catalog.DbProvider)
		return newNode, transform.NewTree, err
	}
}

server/analyzer/resolve_drop_constraint.go:93-98

if index.ID() != "PRIMARY" && index.IsUnique() && index.ID() == dropConstraint.Name {
	newNode, err := plan.NewAlterDropIndex(rt.Database(), rt, dropConstraint.IfExists, index.ID()).
		WithTargetSchema(rt.Schema(ctx))
	return newNode, transform.NewTree, err
}
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Qualified foreign-key drops fail in another schema**

**What failed:** The foreign-key drop command returns `ERROR: table not found: child` instead of removing the constraint from billing.child. Both billing.child and archive.child still have child_parent_id_fkey.

- **Impact:** Database operators cannot remove the foreign key from the requested schema-qualified table. This can block a schema change, but there is no evidence of data loss or corruption.
- **Steps to reproduce:**
  1. Set the search path to public and create billing and archive schemas.
  2. Create billing.parent and archive.parent, then create billing.child and archive.child with a foreign key named child_parent_id_fkey in each schema.
  3. Create the billing expression index and insert valid parent and child rows.
  4. Run ALTER TABLE ONLY billing.child DROP CONSTRAINT child_parent_id_fkey.
  5. Run ALTER TABLE billing.child DROP CONSTRAINT child_parent_id_fkey as a second qualified attempt and inspect the foreign-key metadata.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The new resolver unwraps the resolved table at lines 42-47, so the target schema remains available as rt.Schema(ctx). For a matching foreign key, however, lines 49-58 call plan.NewAlterDropForeignKey(rt.Database().Name(), rt.Name(), fk.Name) and only attach the database provider; no target schema is supplied. The neighboring unique-index branch at lines 93-98 explicitly calls WithTargetSchema(rt.Schema(ctx)), demonstrating the missing schema propagation. The runtime test created billing.child and archive.child with identical constraint names, set search_path to public, and observed table not found: child for both ALTER TABLE ONLY billing.child and ALTER TABLE billing.child; metadata then showed both schema-qualified foreign keys still present. The smallest fix is to preserve the resolved schema when constructing the foreign-key drop operation, using the corresponding target-schema mechanism supported by the AlterDropForeignKey plan node, or otherwise execute it against the already resolved schema-qualified table rather than reacquiring an unqualified child table.
- **Why this is likely a bug:** The requested behavior is independently stated in the PR description: schema-qualified drops must affect only the requested table. The failure is deterministic with two schema-qualified syntaxes and is not caused by missing setup, because both schemas, both tables, their parent rows, the child rows, and the duplicate foreign-key names were created successfully before the drop. The resolver has already identified the qualified table, but the new foreign-key operation discards that schema before execution. A targeted schema-preserving change in this PR's new branch should allow billing.child to be changed without touching archive.child.

**Relevant code:**

`server/analyzer/resolve_drop_constraint.go:42-58`

~~~go
resolvedTable := *rt
resolvedTable.Table = sql.GetUnderlyingTable(rt.Table)
rt = &resolvedTable
...
if fkt, ok := table.(sql.ForeignKeyTable); ok {
	...
	if strings.EqualFold(fk.Name, dropConstraint.Name) {
		newNode, err := plan.NewAlterDropForeignKey(rt.Database().Name(), rt.Name(), fk.Name).
			WithDatabaseProvider(a.Catalog.DbProvider)
		return newNode, transform.NewTree, err
	}
}
~~~

`server/analyzer/resolve_drop_constraint.go:93-98`

~~~go
if index.ID() != "PRIMARY" && index.IsUnique() && index.ID() == dropConstraint.Name {
	newNode, err := plan.NewAlterDropIndex(rt.Database(), rt, dropConstraint.IfExists, index.ID()).
		WithTargetSchema(rt.Schema(ctx))
	return newNode, transform.NewTree, err
}
~~~

@coffeegoddd

coffeegoddd commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

@coffeegoddd DOLT

read_tests from_latency to_latency percent_change
covering_index_scan_postgres 2.48 2.48 0.0
groupby_scan_postgres 81.48 81.48 0.0
index_join_postgres 2.3 2.3 0.0
index_join_scan_postgres 1.67 1.67 0.0
index_scan_postgres 467.3 467.3 0.0
oltp_point_select 0.39 0.39 0.0
oltp_read_only 6.67 6.55 -1.8
select_random_points 0.74 0.74 0.0
select_random_ranges 1.1 1.1 0.0
table_scan_postgres 458.96 467.3 1.82
types_table_scan_postgres 1170.65 1191.92 1.82
write_tests from_latency to_latency percent_change
oltp_delete_insert_postgres 4.57 4.57 0.0
oltp_insert 2.26 2.26 0.0
oltp_read_write 12.52 12.52 0.0
oltp_update_index 2.48 2.48 0.0
oltp_update_non_index 2.22 2.18 -1.8
oltp_write_only 5.88 5.88 0.0
types_delete_insert_postgres 5.0 5.0 0.0

@itoqa

itoqa Bot commented Oct 9, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Report — d9d9c95 → 31a34b7: 14 test cases ran, 1 new failure ❌, 10 passing ✅, 3 additional findings ⚠️.

Diff Summary

Coverage spans constraint removal across unique, primary-key, foreign-key, and check rules, including schema isolation, index preservation, retries, same-session recovery, and concurrent operations. The main gap is in an edge-case migration path for safely ignoring an already-missing constraint; broader dependency-action compatibility issues were also observed but are unrelated to this change.

Merge with caution — this PR introduces a medium-severity failure in the missing-constraint IF EXISTS behavior, which can interrupt repeatable schema migrations instead of treating the operation as a no-op. Other medium-severity dependency-handling findings are pre-existing and should be tracked separately, but the attributable failure warrants a fix before considering the change fully safe.

Tests run by Ito

View full run

Result State Severity Type Description
❌ ❌ New Failure Medium severity Exists Dropping an existing unique constraint succeeds, but the same command with a missing constraint and IF EXISTS shows a syntax error instead of succeeding as a no-op.
✅ Passing — General Dropping the constraint from auth.users allowed duplicate emails there, while public.users and another_schema.users kept rejecting duplicates.
✅ Passing — General A failed removal kept the foreign key active, and a later valid retry removed it so the previously blocked child row was accepted.
✅ Passing — General Two clients dropped the same unique constraint at the same time. Both commands succeeded, the unrelated index stayed in place, and duplicate rows were accepted after the constraint was gone.
✅ Passing — Check The CHECK rule was removed, the previously blocked value was accepted, and the lower-case email query still used the expression index.
✅ Passing — Constraint The unique email rule was removed, duplicate email rows were accepted, and the remaining lowercase email index still answered the query.
✅ Passing — Foreign Dropping the named foreign key removed its metadata and allowed a user row with an unmatched parent value.
✅ Passing — Primary Dropping the primary key worked. The table stored two rows with the same id, and the lower-case email index remained available.
✅ Passing — Rev The database returned the expected missing-constraint error, then accepted the next drop command in the same session. The unique rule was removed, the expression index stayed available, and duplicate rows were inserted successfully.
✅ Passing — Rev The named composite constraint was removed from auth.users, the lower(email) index stayed available, and a second row with the same tenant and email was accepted.
✅ Passing — Schema Dropping the constraint from auth.users removed it only from that table. Duplicate emails were accepted in auth.users, while public.users kept its constraint and rejected the second duplicate.
⏸️ Skipped — General Two sessions dropped the same unique constraint at the same time. The server stayed usable, the unrelated index remained, and duplicate email values were accepted afterward.
⏸️ Skipped — General Two concurrent requests removed the unique constraint from their requested tables without changing the sibling schema. Both tables then accepted duplicate email values as expected.
⏸️ Skipped — Check The case-insensitive drop removed the CHECK constraint, kept the lower-case email index, and allowed the previously blocked value to be inserted.
⏸️ Skipped — Exists The existing email rule was removed, the separate email index stayed in place, and both email values were accepted.
⏸️ Skipped — Exists The missing constraint was ignored, the existing indexes stayed in place, and the unique rule still rejected an exact duplicate email.
⏸️ Skipped — Exists Trying to drop a constraint that does not exist returns SQLSTATE 42704, and the existing unique constraint stays in place.
⏸️ Skipped — Foreign The named foreign-key constraint was removed despite different letter casing. A child row with parent_id 999 was then inserted and read back successfully.
⏸️ Skipped — Primary The primary key was removed without removing the email expression index, and the table accepted and returned both rows with the same ID.
⏸️ Skipped — Rev Dropping the foreign-key constraint succeeded. The expression index stayed in place, and a child row with an invalid parent was accepted afterward.
⏸️ Skipped — Rev The named unique rule was removed, the separate lowercase index stayed in place, and the table accepted and returned both duplicate rows.
⏸️ Skipped — Schema Dropping the constraint from auth.users allowed duplicate email values there, while public.users kept its constraint and rejected the duplicate.
⏸️ Skipped — Unique The unique email constraint was removed, the lower-case email index stayed available, and both case-variant email rows were inserted and returned by the query.
⚠️ Additional Finding Medium severity General The IF EXISTS statement returned SQLSTATE 42601, so the optional drop did not complete. The existing unique index and unrelated index stayed in place, the non-IF-EXISTS statement correctly returned SQLSTATE 42704, and duplicate email insertion was correctly rejected with SQLSTATE 23505.
⚠️ Additional Finding Medium severity Rev The drop was rejected with XX000 ('cannot drop index ... used by foreign key') instead of SQLSTATE 2BP01. The unique constraint, its supporting index, the foreign key, and the expression index remained present, and the duplicate insert was correctly rejected with 23505.
⚠️ Additional Finding Medium severity Rev RESTRICT returned SQLSTATE XX000 with a storage-engine error instead of SQLSTATE 2BP01 for a dependent object. CASCADE returned an unsupported-feature error, so neither requested dependency action completed.
Additional Findings Details

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

🟡 Missing IF EXISTS drops return a syntax error
  • Severity: Medium Medium severity
  • Description: The IF EXISTS statement returned SQLSTATE 42601, so the optional drop did not complete. The existing unique index and unrelated index stayed in place, the non-IF-EXISTS statement correctly returned SQLSTATE 42704, and duplicate email insertion was correctly rejected with SQLSTATE 23505.
  • Impact: Database users cannot safely rerun a statement that removes a constraint only when it exists. The operation fails with a syntax error when the constraint is missing, so affected schema changes need a manual workaround.
  • Steps to Reproduce:
    1. Create a table with a unique email constraint and an unrelated non-unique index.
    2. Run ALTER TABLE users DROP CONSTRAINT IF EXISTS missing_boundary;.
    3. Check the response and confirm that both indexes are still present.
    4. Run the same DROP CONSTRAINT without IF EXISTS and try a duplicate email insert to confirm the existing constraint still behaves normally.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The PostgreSQL AST preserves the constraint-level flag in server/ast/constraint_table_def.go:79-88 by assigning node.IfExists to ConstraintIfExists. The production resolver server/analyzer/resolve_drop_constraint.go:75-100 correctly searches all supported constraint types and passes dropConstraint.IfExists to AlterDropIndex when a matching unique index exists. For a missing constraint, however, server/analyzer/resolve_drop_constraint.go:103-108 creates an AlterDropCheck node and sets its IfExists flag to true. That fallback uses the CHECK-drop execution path for a constraint name that was not found in the foreign-key, CHECK, primary-key, or unique-index lookups. In the observed boundary case this fallback is rejected during SQL analysis with syntax error 42601 rather than producing the required no-op. The smallest practical fix is to use a generic missing-constraint no-op operation for the true IfExists fallback, or otherwise teach the existing fallback operation to accept a missing constraint without routing it through CHECK-specific resolution. The non-IF-EXISTS branch at line 110 already returns sql.ErrUnknownConstraint and matches the observed 42704 response.
Evidence Package
🟡 Dependent constraint drops return the wrong error code
  • Severity: Medium Medium severity
  • Description: The drop was rejected with XX000 ('cannot drop index ... used by foreign key') instead of SQLSTATE 2BP01. The unique constraint, its supporting index, the foreign key, and the expression index remained present, and the duplicate insert was correctly rejected with 23505.
  • Impact: Migration tools that expect PostgreSQL's dependent-object error may fail to handle this rejected drop correctly. The database keeps the constraint and indexes intact, so users do not lose data.
  • Steps to Reproduce:
    1. Create parent with a primary key and a unique constraint named parent_email_key.
    2. Create child with a foreign key from child.parent_email to parent.email, then create an expression index on lower(parent.email).
    3. Run ALTER TABLE parent DROP CONSTRAINT parent_email_key.
    4. Check the error code and then query the catalogs and insert a duplicate parent email.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The observed error is consistent with the production analyzer path in server/analyzer/resolve_drop_constraint.go. Lines 75-99 find a non-primary unique constraint and replace the PostgreSQL DROP CONSTRAINT node with plan.NewAlterDropIndex(...), preserving the underlying table and target schema for expression-index handling. That conversion routes a constraint drop through generic index-drop dependency handling. The repository has an explicit PostgreSQL error code for dependent objects in postgres/parser/pgcode/codes.go:187 (DependentObjectsStillExist = 2BP01), and server/extensions/objects.go:367-369 demonstrates the intended pattern of constructing a pgerror with that code for a dependency failure. No corresponding conversion exists in the unique-constraint/index path, so the engine's generic XX000 dependency error can escape. The local evidence shows no partial catalog mutation, so this is an error-contract defect rather than confirmed catalog corruption. The smallest practical fix is to make the AlterIndex dependency check return pgcode.DependentObjectsStillExist for a non-CASCADE drop blocked by a foreign key, or translate that dependency error at the production drop-constraint path; retain the existing atomic rejection behavior.
Evidence Package
🟡 Constraint dependency actions return wrong results
  • Severity: Medium Medium severity
  • Description: RESTRICT returned SQLSTATE XX000 with a storage-engine error instead of SQLSTATE 2BP01 for a dependent object. CASCADE returned an unsupported-feature error, so neither requested dependency action completed.
  • Impact: Database administrators and developers cannot use qualified RESTRICT or CASCADE constraint drops as expected. This can block schema migrations until they use a manual workaround, but the failed operation leaves the existing constraints and indexes intact.
  • Steps to Reproduce:
    1. Create an auth.parent table with a unique parent_email_key constraint and an auth.child table whose child_parent_email_fkey references it.
    2. Create the unrelated auth.parent_lower_email_idx expression index.
    3. Run ALTER TABLE ONLY auth.parent DROP CONSTRAINT parent_email_key RESTRICT and record the SQLSTATE.
    4. Run the same qualified drop with CASCADE and inspect the constraint and index catalogs afterward.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The parser-side conversion in server/ast/constraint_table_def.go:65-90 checks node.DropBehavior and immediately returns errors.Errorf("CASCADE is not yet supported for drop constraint") at lines 75-77, which directly explains the CASCADE failure. For RESTRICT, the same function emits a generic alter/drop DDL without encoding PostgreSQL dependency-error semantics. The analyzer rule server/analyzer/resolve_drop_constraint.go:75-99 resolves a matching non-primary unique constraint into plan.NewAlterDropIndex and preserves the target schema, but it does not handle dependent foreign keys or translate the execution error to postgres/parser/pgcode.DependentObjectsStillExist (2BP01). The recorded XX000 message, "cannot drop index ... used by foreign key ...", is therefore a plausible production-path result of delegating the constraint drop to the index operation. The smallest practical remediation is to carry the drop behavior into the constraint execution path, translate a RESTRICT dependency rejection to 2BP01, and implement CASCADE by removing only the dependent objects required by the named constraint while retaining unrelated indexes such as parent_lower_email_idx. The PR diff shown in pr-context.json contains no changes to either production file, so this finding predates the PR.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

@itoqa

itoqa Bot commented Oct 9, 2026

Copy link
Copy Markdown

Ito QA test results

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

Commit: 081c5e5: 11 test cases ran, 10 passed ✅, 1 additional finding ⚠️.

Summary

Coverage spans safe schema changes involving primary keys, foreign keys, unique and check rules, including qualified names, mixed-case names, missing objects, concurrent requests, multi-step updates, session recovery, and preservation of indexes and defaults. The exercised behavior is largely healthy across normal flows and edge cases, with one separate data-integrity issue where an invalid value bypassed an existing check rule.

Safe to merge — the only failure is explicitly unrelated to this PR and is not a regression or a previously flagged issue; the PR’s constraint-removal behavior remains covered by passing scenarios. The unrelated check-enforcement defect is a serious follow-up data-integrity concern, but it is not a merge blocker for this change.

Tests run by Ito

View full run

Result Severity Type Description
✅ — General Trying to remove a missing constraint returns an error unless IF EXISTS is used. The guarded command is a no-op, existing database objects remain, and invalid data is still rejected.
✅ — General The table change completed without an error or an unrelated change. Billing now accepts the invalid reference, while the archive table still rejects it and the billing expression index remains available.
✅ — General Two concurrent requests removed the billing foreign key without changing archive. Billing accepted the invalid reference afterward, while archive still rejected it.
✅ — Foreign Dropping the billing foreign key succeeded. Billing accepted the invalid reference afterward, while archive and public still rejected it, and the billing expression index remained available.
✅ — Foreign The database matched the mixed-case constraint name to the existing foreign key. The drop succeeded, and the child table then accepted the previously rejected reference.
✅ — Preserve The primary key was removed successfully. Duplicate former key values were accepted, the omitted column kept its default, and the expression index remained usable.
✅ — Rev The table change removed fk_old, kept fk_other and the expression index, accepted the row that only fk_old would reject, and still rejected a row that violated fk_new.
✅ — Rev Dropping a missing constraint returned the expected error, and the same database session continued working. The unique constraint still rejected a duplicate value afterward.
✅ — Schema Dropping the constraint from auth.users removed auth's unique rule and allowed the duplicate email. The same constraint and index remained on public.users, so its duplicate insert was rejected.
✅ — Unique Dropping the email constraint removed uniqueness without removing the lower-case email index. Both case-variant email rows were accepted, and the lower-case query returned both rows.
⚠️ High severity Check In a table with users_email_check requiring an @ in email, inserting invalid_again succeeded before DROP CONSTRAINT. After the drop, the constraint disappeared, the users_lower_email_idx index remained, and another invalid insert also succeeded.
Additional Findings Details

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

🟠 Check constraint accepts an invalid row
  • Severity: High High severity
  • Description: In a table with users_email_check requiring an @ in email, inserting invalid_again succeeded before DROP CONSTRAINT. After the drop, the constraint disappeared, the users_lower_email_idx index remained, and another invalid insert also succeeded.
  • Impact: The database can store invalid email values even while a check constraint says they are forbidden. This can leave user data incorrect and is hard to repair after the invalid rows are saved.
  • Steps to Reproduce:
    1. Create a table with email text and a named check constraint requiring strpos(email, '@') > 1.
    2. Create an expression index on lower(email).
    3. Insert a row whose email is invalid_again, before dropping the check constraint.
    4. Inspect the constraint and index, then run ALTER TABLE users DROP CONSTRAINT users_email_check.
    5. Insert another invalid value and query the table and index metadata.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The recorded SQL result shows the check constraint present in pg_constraint at lines 1-4 of evidence/test-cases/CHECK-1/drop-and-readback.txt, yet the pre-drop invalid insert completed with INSERT 0 1 in evidence/test-cases/CHECK-1/before-rejection.txt. The post-drop assertions are otherwise correct: users_email_check is absent at lines 11-14, users_lower_email_idx remains at lines 16-19, and the invalid value is stored at lines 21-25. In production code, server/analyzer/resolve_drop_constraint.go:44-48 unwraps expression-index table wrappers only while resolving a DROP CONSTRAINT node, and :63-71 converts a matching check to plan.NewAlterDropCheck; that path explains the successful removal but is not involved in the earlier INSERT. The PR diff adds this drop resolver and the related integration coverage, but does not change the INSERT/check-validation path. The likely defect is therefore in the existing check-enforcement handling for the expression-index-backed table wrapper, not in the new drop conversion. The smallest practical fix is to make the write-validation path evaluate the underlying table's declared checks when an expression-index wrapper is present, then add a regression assertion that the invalid insert fails before the drop and succeeds after it.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

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.

Dropping UNIQUE constraint fails when table has an expression index (blocks Supabase Auth)

1 participant