Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove support for primary indexes #6870

Open
wants to merge 10 commits into
base: 5.0.x
Choose a base branch
from
Open

Remove support for primary indexes #6870

wants to merge 10 commits into from

Conversation

morozov
Copy link
Member

@morozov morozov commented Mar 23, 2025 •
edited
Loading

The affected features were deprecated in #6867. See the upgrade notes for details on the end-user impact.

Improvements

As a side effect, this change provides the following:

  1. Dropping a primary key constraint is now supported on all platforms.
  2. Naming a primary key constraint is now supported on all platforms except for MySQL and SQLite. This is a limitation of the platforms in question.

Technical details

  1. This PR only forbids passing true as the $isPrimary argument of the Index constructor but does not remove the parameter itself. Removing the parameter would require modifying code that instantiates non-PK indexes as well. Once we introduce IndexEditor, we will mark the constructor as @internal and then remove the parameter.
  2. The code modified by the "Compare unquoted names to unquoted names" commit is incorrect in all DBAL versions but the corresponding issue is only reproducible on 5.0.x and only in-between the commits of this PR. The issue is that the comparison is performed between the quoted index column names (derived from the introspected PK constraint). This way, they will never match. In 4.3.x, all introspected SQLite objects are always unquoted, so the comparison works aas expected.

Closes #807
Fixes #2294
Closes #3890

@morozov morozov added this to the 5.0.0 milestone Mar 23, 2025
@morozov morozov force-pushed the remove-primary-index branch from e45f2b4 to bf5bb05 Compare March 23, 2025 21:11
@morozov morozov force-pushed the remove-primary-index branch 2 times, most recently from ad9e28e to 50fef26 Compare March 23, 2025 23:41
@morozov morozov linked an issue Mar 24, 2025 that may be closed by this pull request
Support named PKs #2294
Open
morozov added 2 commits March 23, 2025 18:57
@morozov
Use PrimaryKeyConstraint when creating tables
d398a70 
The SQLServerPlatformTest::testCreateNonClusteredPrimaryKeyInTable()
failure is expected here. It happens because the test mutates the
primary index after adding it to the table, but that is not reflected
in the primary key constraint derived from that index at the time when
the index was added to the table.

Tracking mutations in the index might significantly complicate the
forward compatibility code and so far is considered not worth it.
@morozov
Use PrimaryKeyConstraint for schema comparison and management
746ca81
@morozov morozov force-pushed the remove-primary-index branch from 50fef26 to 9f310c0 Compare March 24, 2025 01:58
@morozov morozov force-pushed the remove-primary-index branch from 9f310c0 to c6ea5b1 Compare March 24, 2025 02:09
@morozov morozov marked this pull request as ready for review March 24, 2025 02:18
@morozov morozov requested a review from greg0ire March 24, 2025 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire Awaiting requested review from greg0ire

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
BC Break Improvement Indexes Primary Keys Schema Definition Schema Management
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

Support named PKs
1 participant
@morozov