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: fix validation of MATCH FULL foreign keys with mixed column types #42528

Merged
merged 1 commit into from
Nov 21, 2019

Conversation

thoszhang
Copy link
Contributor

Previously, the internal validation query for MATCH FULL foreign keys checked
for the presence of invalid keys for a set of columns c_1, ..., c_n by using
COALESCE(c_1, ..., c_n) IS NULL, which fails if any of the columns differ in
type because COALESCE requires its arguments to be all of the same type. This
PR fixes the validation query by using c_1 IS NULL AND ... AND c_n IS NULL
instead.

Fixes #42498.

Release note (bug fix): Fixed a bug that would produce a spurious failure with
the error message "incompatible COALESCE expressions" when adding or validating
MATCH FULL foreign key constraints involving composite keys with columns of
differing types.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@thoszhang
Copy link
Contributor Author

This was introduced in 19.1 (along with support for MATCH FULL, I think), but it becomes worse in 19.2 since all FKs get validated by default. I think we should backport to at least 19.2.

Copy link
Contributor

@solongordon solongordon left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @lucy-zhang and @solongordon)


pkg/sql/check.go, line 73 at r1 (raw file):

// SELECT s.a_id, s.b_id, s.pk1, s.pk2 FROM child@c_idx
// WHERE
//   NOT ((a_id IS NULL AND b_id IS NULL) OR (a_id IS NOT NULL AND b_id IS NOT NULL))

This is unrelated to your fix but seems like this could be simplified to WHERE (a_id IS NULL OR b_id IS NULL) AND (a_id IS NOT NULL OR b_id IS NOT NULL).

Copy link
Contributor Author

@thoszhang thoszhang left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @solongordon)


pkg/sql/check.go, line 73 at r1 (raw file):

Previously, solongordon (Solon) wrote…

This is unrelated to your fix but seems like this could be simplified to WHERE (a_id IS NULL OR b_id IS NULL) AND (a_id IS NOT NULL OR b_id IS NOT NULL).

Done.

Previously, the internal validation query for MATCH FULL foreign keys checked
for the presence of invalid keys for a set of columns `c_1, ..., c_n` by using
`COALESCE(c_1, ..., c_n) IS NULL`, which fails if any of the columns differ in
type because `COALESCE` requires its arguments to be all of the same type. This
PR fixes the validation query by using `c_1 IS NULL AND ... AND c_n IS NULL`
instead.

Release note (bug fix): Fixed a bug that would produce a spurious failure with
the error message "incompatible COALESCE expressions" when adding or validating
`MATCH FULL` foreign key constraints involving composite keys with columns of
differing types.
@thoszhang
Copy link
Contributor Author

TFTR!

bors r+

craig bot pushed a commit that referenced this pull request Nov 21, 2019
42528: sql: fix validation of MATCH FULL foreign keys with mixed column types r=lucy-zhang a=lucy-zhang

Previously, the internal validation query for MATCH FULL foreign keys checked
for the presence of invalid keys for a set of columns `c_1, ..., c_n` by using
`COALESCE(c_1, ..., c_n) IS NULL`, which fails if any of the columns differ in
type because `COALESCE` requires its arguments to be all of the same type. This
PR fixes the validation query by using `c_1 IS NULL AND ... AND c_n IS NULL`
instead.

Fixes #42498.

Release note (bug fix): Fixed a bug that would produce a spurious failure with
the error message "incompatible COALESCE expressions" when adding or validating
`MATCH FULL` foreign key constraints involving composite keys with columns of
differing types.

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

craig bot commented Nov 21, 2019

Build succeeded

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: unexpected FK validation problem with MATCH FULL
4 participants