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

backport-2.1: sqlbase: don't permit PK columns in STORING list #31032

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

jordanlewis
Copy link
Member

Backport 1/1 commits from #31019.

/cc @cockroachdb/release


Previously, users could create indexes that included PRIMARY KEY columns
in the STORING list. This was effectively a no-op and misleading to
users, since PRIMARY KEY columns are always stored in indexes regardless
of the STORING list. Separately, permitting this tickled a bug in the
descriptor accounting system.

Now, this behavior is prevented by returning an error to the user if
they try to do such a thing, with a helpful message explaining why it's
not allowed.

Fixes #30984.

Release note (sql change): don't allow specifying STORING columns in
indexes that are already part of the index's corresponding PRIMARY
INDEX.

Previously, users could create indexes that included PRIMARY KEY columns
in the STORING list. This was effectively a no-op and misleading to
users, since PRIMARY KEY columns are always stored in indexes regardless
of the STORING list. Separately, permitting this tickled a bug in the
descriptor accounting system.

Now, this behavior is prevented by returning an error to the user if
they try to do such a thing, with a helpful message explaining why it's
not allowed.

Release note (sql change): don't allow specifying STORING columns in
indexes that are already part of the index's corresponding PRIMARY
INDEX.
@jordanlewis jordanlewis requested review from knz and a team October 5, 2018 21:31
@jordanlewis jordanlewis requested a review from a team as a code owner October 5, 2018 21:32
@jordanlewis jordanlewis requested review from a team October 5, 2018 21:32
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

thanks

@knz
Copy link
Contributor

knz commented Oct 7, 2018

Can you add #31010 to this backport please?

@jordanlewis
Copy link
Member Author

I'll grab that separately.

@jordanlewis jordanlewis merged commit e73a623 into cockroachdb:release-2.1 Oct 8, 2018
@jordanlewis jordanlewis deleted the backport2.1-31019 branch October 8, 2018 14:43
@jordanlewis
Copy link
Member Author

cc @rmloveland this will need special attention in the release notes. Schemas made with this invalid STORING list will need to be adjusted, but note that the schema itself won't change - just the way it's written down.

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.

None yet

3 participants