Support ALTER TABLE ADD PRIMARY KEY statements
#11895
Open
+911
−260
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm not sure this is the best time to add this feature, as there is a pending PR to rework binding of Indexes (#11867) |
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
Tishj
reviewed
May 1, 2024
|
Thanks for the quick review! Regarding #11867, I guess that would affect the part about indexes, which should affect only small part of the code from this PR (the |
|
I fixed all comments, let me know if there is anything else I can change! |
frapa
force-pushed
the
support_alter_table_add_primary_key
branch
from
f14b1f4
to
e05ed09
Compare
frapa
force-pushed
the
support_alter_table_add_primary_key
branch
from
e05ed09
to
e7d1570
Compare
frapa
force-pushed
the
support_alter_table_add_primary_key
branch
from
e7d1570
to
ce62349
Compare
frapa
force-pushed
the
support_alter_table_add_primary_key
branch
from
ce62349
to
70b552a
Compare
frapa
force-pushed
the
support_alter_table_add_primary_key
branch
from
c09ce34
to
ab5a5fc
Compare
…se it. Taking advantage of polymorphism to replace switch statements in a couple places. I also had to refactor UniqueBoundConstraint to use PhysicalIndexes instead of LogicalIndexes, in a similar way to all other BoundConstraints.
…er for ALTER TABLE statements
I renamed it AddConstraintIndex. By the name, this method belongs there, and I need it later in the DataTable class constructor to handle addition of unique indices.
Now, when a new unique index is created and a modified instance of the storage is made, the DataTable will automatically add an ART index and index existing rows, throwing an error if the uniqueness is not satisfied.
This makes code that needs to access the index after the addition to the table simpler.
This is not necessary because creating a unique index automatically checks the uniqueness.
frapa
force-pushed
the
support_alter_table_add_primary_key
branch
from
ab5a5fc
to
8b87ebc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements
ALTER TABLE table ADD PRIMARY KEY (col1, ...);statements, as per #57.I suggest reviewing by commit, as I tried to make them thematic and self-contained. However, the first commits basically do changes needed for the last 2-3 commits, so please have a look at the final ones if something's unclear.
Let me know if there is anything I can improve.