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.

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

sql: make FK schema changes backward-compatible with 19.1 #39614

Merged

Conversation

thoszhang
Copy link
Contributor

@thoszhang thoszhang commented Aug 13, 2019

This PR makes adding a foreign key using ADD CONSTRAINT backward-compatible
with 19.1, by doing it in the user transaction and adding the constraint in the
Unvalidated state if the cluster is not upgraded to 19.2, instead of queuing
a mutation that can't be handled by 19.1 nodes.

The cluster version used for this is cluster.VersionTopLevelForeignKeys,
which is also used for the new foreign key representation. It made sense to
reuse this cluster version since both sets of changes affect the same code, and
it's assumed that either both or neither sets of changes will be active.

This PR also updates the special mixed 19.1/19.2 logic tests for foreign keys,
introduced in the FK representation refactoring, to revert the files to their
19.1 state, aside from some small changes to conform to unrelated 19.2
behavior, since 19.1 supports fewer FK schema change-related features.

Closes #39037.

Release note: None

@thoszhang thoszhang requested a review from a team as a code owner August 13, 2019 00:18
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@thoszhang
Copy link
Contributor Author

thoszhang commented Aug 13, 2019

The first 2 commits are #39383.

@thoszhang thoszhang force-pushed the constraint-backward-compatibility branch from 63b54fa to 8b493e4 Compare August 13, 2019 00:23
@thoszhang thoszhang requested a review from dt August 13, 2019 00:25
@thoszhang thoszhang force-pushed the constraint-backward-compatibility branch from f0f7b27 to 8b493e4 Compare August 14, 2019 00:38
@thoszhang thoszhang force-pushed the constraint-backward-compatibility branch 2 times, most recently from b0202e4 to 07e2ff0 Compare August 20, 2019 18:20
COMMIT

# TODO(lucy): this fails (hangs) but we don't know why, in the mixed version state.
# TODO(lucy): This doesn't work and will become basically irrelevant when
Copy link
Member

Choose a reason for hiding this comment

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

it took me a little while to figure out what "this doesn't work" meant here, so might be good to explicitly call out "this hangs" or something instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

This PR makes adding a foreign key using `ADD CONSTRAINT` backward-compatible
with 19.1, by doing it in the user transaction and adding the constraint in the
`Unvalidated` state if the cluster is not upgraded to 19.2, instead of queuing
a mutation that can't be handled by 19.1 nodes.

The cluster version used for this is `cluster.VersionTopLevelForeignKeys`,
which is also used for the new foreign key representation. It made sense to
reuse this cluster version since both sets of changes affect the same code, and
it's assumed that either both or neither sets of changes will be active.

This PR also updates the special mixed 19.1/19.2 logic tests for foreign keys,
introduced in the FK representation refactoring, to revert the files to their
19.1 state, aside from some small changes to conform to unrelated 19.2
behavior, since 19.1 supports fewer FK schema change-related features.

Release note: None
@thoszhang thoszhang force-pushed the constraint-backward-compatibility branch from 07e2ff0 to 47a4c4f Compare August 20, 2019 21:30
@thoszhang
Copy link
Contributor Author

bors r+

craig bot pushed a commit that referenced this pull request Aug 20, 2019
39614:  sql: make FK schema changes backward-compatible with 19.1 r=lucy-zhang a=lucy-zhang

This PR makes adding a foreign key using `ADD CONSTRAINT` backward-compatible
with 19.1, by doing it in the user transaction and adding the constraint in the
`Unvalidated` state if the cluster is not upgraded to 19.2, instead of queuing
a mutation that can't be handled by 19.1 nodes.

The cluster version used for this is `cluster.VersionTopLevelForeignKeys`,
which is also used for the new foreign key representation. It made sense to
reuse this cluster version since both sets of changes affect the same code, and
it's assumed that either both or neither sets of changes will be active.

This PR also updates the special mixed 19.1/19.2 logic tests for foreign keys,
introduced in the FK representation refactoring, to revert the files to their
19.1 state, aside from some small changes to conform to unrelated 19.2
behavior, since 19.1 supports fewer FK schema change-related features.

Closes #39037.

Release note: None

Co-authored-by: Lucy Zhang <lucy-zhang@users.noreply.github.com>
@craig
Copy link
Contributor

craig bot commented Aug 20, 2019

Build succeeded

@craig craig bot merged commit 47a4c4f into cockroachdb:master Aug 20, 2019
@thoszhang thoszhang deleted the constraint-backward-compatibility branch August 20, 2019 21:58
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.

sql: make schema changes backward compatible with 19.1
4 participants