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

cherrypick: sql: two bugfixes to fks that use a partial index #17653

Merged
merged 2 commits into from
Aug 15, 2017

Conversation

jordanlewis
Copy link
Member

Cherrypick of #17638.

Previously, tables with foreign key references that were a prefix of a pre-existing index could have their foreign key constraints violated by deletion from the referenced table.

This means that any tables created on 1.0 (and earlier) versions with foreign key constraints on an index prefix might have constraint violations if rows were deleted from the referenced table.

The tests missed this case because of an unhappy coincidence - the proximal cause of the failure mode was that the key into the referencing index was encoded with the full length of the referencing index, even though the foreign key itself had less columns that that index. This
caused the key to contain NULL values at the end. The test case that was supposed to guard against this problem had NULL values in the referencing value at the end positions of the key, so a match was found and the deletion was prevented.

Also, SHOW CREATE TABLE for tables with such foreign keys produced incorrect output.

Fixes #17626.

cc @cockroachdb/release

@jordanlewis jordanlewis requested review from dt and knz August 15, 2017 14:15
@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.

Wow, nice catch (sorry I'm late I didn't catch the previous PR).

Previously, tables with foreign key references that were a prefix of a
pre-existing index could have their foreign key constraints violated by
deletion from the referenced table.

This means that any tables created on 1.0 (and earlier) versions with
foreign key constraints on an index prefix might have constraint
violations if rows were deleted from the referenced table.

The tests missed this case because of an unhappy coincidence - the
proximal cause of the failure mode was that the key into the referencing
index was encoded with the full length of the referencing index, even
though the foreign key itself had less columns that that index. This
caused the key to contain `NULL` values at the end. The test case that
was supposed to guard against this problem had `NULL` values in the
referencing value at the end positions of the key, so a match was found
and the deletion was prevented.
Previously, a table with a foreign key constraint on a set of columns
that's a prefix to a pre-existing index on that table had incorrect
output for `SHOW CREATE TABLE`. The output claimed that the foreign key
constraint included all of the columns of the index, instead of just the
correct prefix of it.
@jordanlewis jordanlewis merged commit 828a46a into cockroachdb:release-1.0 Aug 15, 2017
@jordanlewis jordanlewis deleted the cherrypick-fk-fix branch August 15, 2017 17:07
knz added a commit to knz/cockroach that referenced this pull request Oct 10, 2017
This groups commits by author, in the style of the Linux CHANGES file.

For example:

```
% ./scripts/changelog.py v1.0.5 v1.0.4
Ben Darnell:
        cockroachdb#17654 182613 (+  63 -  20 ~  83/ 1) cherrypick: security: Disable 3DES cipher suites

Jordan Lewis:
        cockroachdb#17653 828a46 (+  23 -   4 ~  27/ 4) cherrypick: sql: two bugfixes to fks that use a partial index (2 commits)

kena:
        cockroachdb#17689 6255bb (+   7 -   2 ~   9/ 1) cherrypick: log: allow panic to propagate on crash reporting timeouts
        cockroachdb#17675 a725c2 (+  17 -   5 ~  22/ 2) cherrypick: log: ensure Fatal calls are reported even if logs don't go to stderr
        cockroachdb#17681 e73fc3 (+  50 -  39 ~  89/ 2) cherrypick: sql: disallow AS OF SYSTEM TIME 0, add more AS OF tests
        cockroachdb#17281 8d0dea (+  15 -   0 ~  15/ 3) cherrypick: sql: ensure that planNodes are Close()d on error after expansion

Tobias Schottdorf:
        cockroachdb#17385 63abc8 (+  18 -   3 ~  21/ 1) cherry-pick: base: bump DefaultSendNextTimeout from 500ms to 10m
```
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.

3 participants