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

sql: cannot perform primary key changes while other schema changes are in progress #45510

Closed
1 of 4 tasks
rohany opened this issue Feb 27, 2020 · 5 comments · Fixed by #106175
Closed
1 of 4 tasks

sql: cannot perform primary key changes while other schema changes are in progress #45510

rohany opened this issue Feb 27, 2020 · 5 comments · Fixed by #106175
Assignees
@postamar
Labels
A-schema-changes skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-anchored-telemetry The issue number is anchored by telemetry references.

Comments

@rohany
Copy link
Contributor

rohany commented Feb 27, 2020

We cannot currently perform primary key changes when an in progress schema change on the same table is occuring, or when a schema change on the same table has been started earlier in the same transaction. Before we can allow this, we need to investigate the following cases and ensure that primary key changes can handle them.

  • preceding index creations
  • preceding column creations
  • preceding constraint creation/validation
  • preceding primary key changes
@rohany rohany added A-schema-changes X-anchored-telemetry The issue number is anchored by telemetry references. labels Feb 27, 2020
@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 12, 2021
@lrotim
Copy link

lrotim commented Nov 14, 2021

Any news or timeline on when this feature might be implemented?

@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Jan 20, 2022
@ajwerner
Copy link
Contributor

We're working on generalized transactional schema changes. That work will encompass this.

@vy-ton
Copy link
Contributor

vy-ton commented Jan 25, 2022

#42061 is the issue for generalized transactional schema changes

@healthy-pod healthy-pod removed the T-sql-schema-deprecated Use T-sql-foundations instead label May 17, 2023
@postamar postamar self-assigned this Jul 5, 2023
@postamar
Copy link
Contributor

postamar commented Jul 5, 2023

Reopening to track unskipping test case in TestPrimaryKeyChangeWithPrecedingIndexCreation

@rafiss rafiss reopened this Jul 5, 2023
postamar pushed a commit to postamar/cockroach that referenced this issue Jul 5, 2023
schemachanger,scbuild: fix assertion bug
438eb13 
The declarative schema changer builder contains assertions on the state
of elements and their targets and these could sometimes fail because the
check which verifies that the corresponding descriptors are not already
undergoing a schema change was performed later. This commit fixes this
by moving the check earlier so the assertion is never reached in those
cases.

This commit removes a skipped test which is superseded by a new test in
the schemachanger package.

Fixes: cockroachdb#45510

Release note (bug fix): fixed a bug which manifested itself in error
messages containing "failed to drop all of the relevant elements" when
executing DDL statements with the declarative schema changer. What this
really means is that there's a concurrent schema change that's ongoing.
Instead we now behave as expected and wait for it to finish.
@postamar postamar mentioned this issue Jul 5, 2023
Merged
@postamar postamar mentioned this issue Jul 6, 2023
Merged
@postamar postamar mentioned this issue Jul 6, 2023
Merged
postamar pushed a commit to postamar/cockroach that referenced this issue Jul 6, 2023
schemachanger,scbuild: fix assertion bug
2518b64 
The declarative schema changer builder contains assertions on the state
of elements and their targets and these could sometimes fail because the
check which verifies that the corresponding descriptors are not already
undergoing a schema change was performed later. This commit fixes this
by moving the check earlier so the assertion is never reached in those
cases.

This commit removes a skipped test which is superseded by a new test in
the schemachanger package.

Fixes: cockroachdb#45510
Fixes: cockroachdb#102927
Fixes: cockroachdb#105754

Release note (bug fix): fixed a bug which manifested itself in error
messages containing "failed to drop all of the relevant elements" when
executing DDL statements with the declarative schema changer. What this
really means is that there's a concurrent schema change that's ongoing.
Instead we now behave as expected and wait for it to finish.
postamar pushed a commit to postamar/cockroach that referenced this issue Jul 6, 2023
schemachanger,scbuild: fix assertion bug
db4e103 
The declarative schema changer builder contains assertions on the state
of elements and their targets and these could sometimes fail because the
check which verifies that the corresponding descriptors are not already
undergoing a schema change was performed later. This commit fixes this
by moving the check earlier so the assertion is never reached in those
cases.

This commit removes a skipped test which is superseded by a new test in
the schemachanger package.

Fixes: cockroachdb#45510
Fixes: cockroachdb#102927
Fixes: cockroachdb#105754

Release note (bug fix): fixed a bug which manifested itself in error
messages containing "failed to drop all of the relevant elements" when
executing DDL statements with the declarative schema changer. What this
really means is that there's a concurrent schema change that's ongoing.
Instead we now behave as expected and wait for it to finish.
craig bot pushed a commit that referenced this issue Jul 6, 2023
@wenyihu6
Merge #105795 #106175
65409a7 
105795: asim: add exact_bound, upper_bound, lower_bound to CmdArgs r=kvoli a=wenyihu6

**asim: move sim helper functions to helpers_test.go**

This patch moves helper functions for scanning and parsing arguments from
`datadriven_simulation_test.go` to `helpers_test.go`.

Note that this commit does not change any existing functionality.

Epic: None

Release Note: None

---

**asim: add exact_bound, upper_bound, lower_bound to CmdArgs**

Previously, the allocator simulator's assertions were limited to accepting only
a single threshold value for type=balance, type=steady, and type=stat
assertions. This single threshold functioned as an upper limit for balance and
steady and as an exact limit for stat. This is less ideal as there are instances
where different types of bounds need to be asserted. To improve this, this patch
replaces the argument `threshold=` with 
`exact_bound=, upper_bound=, and lower_bound=`. Note that only one 
argument should be specified at a time. If multiple are specified, the expected 
precedence order is exact_bound > upper_bound > lower_bound.

Note that this commit does not change any existing functionality.

Epic: None

Release Note: None

106175: schemachanger,scbuild: fix assertion bug r=postamar a=postamar

The declarative schema changer builder contains assertions on the state of elements and their targets and these could sometimes fail because the check which verifies that the corresponding descriptors are not already undergoing a schema change was performed later. This commit fixes this by moving the check earlier so the assertion is never reached in those cases.

This commit removes a skipped test which is superseded by a new test in the schemachanger package.

Fixes: #45510
Fixes: #102927
Fixes: #105754

Release note (bug fix): fixed a bug which manifested itself in error messages containing "failed to drop all of the relevant elements" when executing DDL statements with the declarative schema changer. What this really means is that there's a concurrent schema change that's ongoing. Instead we now behave as expected and wait for it to finish.

Co-authored-by: Wenyi <wenyi.hu@cockroachlabs.com>
Co-authored-by: Marius Posta <marius@cockroachlabs.com>
@craig craig bot closed this as completed in fbd7d65 Jul 6, 2023
@SunRunAway
Copy link

SunRunAway commented Sep 25, 2024 •
edited
Loading

Should this issue be closed?

defaultdb=> BEGIN;

ALTER TABLE public.status_test
ADD COLUMN id UUID DEFAULT gen_random_uuid();

ALTER TABLE public.status_test
DROP CONSTRAINT status_pk,
ADD PRIMARY KEY (id);

COMMIT;
BEGIN
ALTER TABLE
ERROR:  unimplemented: cannot perform a primary key change on status_test with other schema changes on status_test in the same transaction
提示:  You have attempted to use a feature that is not yet implemented.
See: https://go.crdb.dev/issue-v/45510/v24.1
ROLLBACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@postamar postamar

Labels
A-schema-changes skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-anchored-telemetry The issue number is anchored by telemetry references.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

schemachanger,scbuild: fix assertion bug postamar/cockroach
8 participants
@SunRunAway @rafiss @ajwerner @postamar @lrotim @vy-ton @rohany @healthy-pod