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: columns can only be used in one outgoing foreign key #23580

Closed
knz opened this issue Mar 8, 2018 · 13 comments
Closed

sql: columns can only be used in one outgoing foreign key #23580

knz opened this issue Mar 8, 2018 · 13 comments
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@knz
Copy link
Contributor

knz commented Mar 8, 2018

From @erichocean on gitter:

(02:15) < erichocean> I'm getting an "column cannot be used by multiple foreign key constraints" error on what I think should be legal uses.
(02:16) < erichocean> specifically, I have interleaved tables A, A->B, A->B->C
(02:17) < erichocean> then I try to create another table
(02:18) < erichocean> in order to be able to set up a foreign key constraint two two or more of those tables, I'll get the "column cannot be used by multiple foreign key constraints" error—even though you're referencing totally different tables!
(02:18) < erichocean> [edit] I'm getting a "column cannot be used by multiple foreign key constraints" error on what I think should be legal usage.
(02:19) < erichocean> [edit] then I try to create another table, D, which references rows from A, B, and C
(02:19) < erichocean> because the interleaving requires including A's primary key in A->B, and A and B's primary keys in A->B->C, there's no way to do a foreign key reference without hitting that error
(02:20) < erichocean> i.e. A's key will be referenced three times, B's twice, and C's only once

@erichocean could you comment here and add the SQL statements you have used to (attempt to) create your schema?

@knz
Copy link
Contributor Author

knz commented Mar 8, 2018

cc @BramGruneir

@erichocean
Copy link

Example SQL that triggers the error:

https://gist.github.com/erichocean/9bc11d8c3ccfdfee6815ad61bb2cd850

@knz
Copy link
Contributor Author

knz commented Mar 8, 2018

Copying here for posterity:

CREATE TABLE a (
    id UUID NOT NULL DEFAULT gen_random_uuid(),
    PRIMARY KEY (id)
);

CREATE TABLE aa (
    a UUID NOT NULL,
    at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
    PRIMARY KEY (a, at),
    CONSTRAINT fk_a FOREIGN KEY (a) REFERENCES a (id)
) INTERLEAVE IN PARENT a (a);

CREATE TABLE ab (
    a UUID NOT NULL,
    at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
    PRIMARY KEY (a, at),
    CONSTRAINT fk_a FOREIGN KEY (a) REFERENCES a (id)
) INTERLEAVE IN PARENT a (a);

CREATE TABLE ac (
    a UUID NOT NULL,
    aa TIMESTAMPTZ,
    ab TIMESTAMPTZ,
    PRIMARY KEY (a),
    CONSTRAINT fk_a FOREIGN KEY (a) REFERENCES a (id),
    CONSTRAINT fk_aa FOREIGN KEY (a, aa) REFERENCES aa (a, at),
    CONSTRAINT fk_ab FOREIGN KEY (a, ab) REFERENCES ab (a, at)
) INTERLEAVE IN PARENT a (a);

Fails with:

CREATE TABLE
CREATE TABLE
CREATE TABLE
pq: column "a" cannot be used by multiple foreign key constraints
Error: pq: column "a" cannot be used by multiple foreign key constraints
Failed running "sql"

Workaround:

CREATE TABLE a (
    id UUID NOT NULL DEFAULT gen_random_uuid(),
    PRIMARY KEY (id)
);

CREATE TABLE aa (
    a UUID NOT NULL,
    at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
    PRIMARY KEY (a, at),
    CONSTRAINT fk_a FOREIGN KEY (a) REFERENCES a (id)
) INTERLEAVE IN PARENT a (a);

CREATE TABLE ab (
    a UUID NOT NULL,
    at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
    PRIMARY KEY (a, at),
    CONSTRAINT fk_a FOREIGN KEY (a) REFERENCES a (id)
) INTERLEAVE IN PARENT a (a);

CREATE TABLE ac (
    a UUID NOT NULL,
    a1 UUID NOT NULL CHECK (a = a1),                            -- <====== duplicate 'a' here with a check() statement
    aa TIMESTAMPTZ,
    ab TIMESTAMPTZ,
    PRIMARY KEY (a),
    CONSTRAINT fk_a FOREIGN KEY (a) REFERENCES a (id),
    CONSTRAINT fk_aa FOREIGN KEY (a, aa) REFERENCES aa (a, at),
    CONSTRAINT fk_ab FOREIGN KEY (a1, ab) REFERENCES ab (a, at) -- <====== use the duplicate 'a1' instead in the FK constraint
) INTERLEAVE IN PARENT a (a);

@knz knz added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. labels Mar 8, 2018
@knz
Copy link
Contributor Author

knz commented Mar 8, 2018

cc @awoods187 for triage and prioritization

@BramGruneir
Copy link
Member

Just to be clear, this is a known limitation.
One could also interleave this in either aa, or ab. But it will always be messy.

@knz knz added A-bulkio-schema-changes O-community Originated from the community and removed O-community-questions labels Apr 24, 2018
@knz knz added the A-sql-pgcompat Semantic compatibility with PostgreSQL label Apr 27, 2018
@nstewart
Copy link
Contributor

This problem is exacerbated with geo-partitioning where many tables will use the same partition key. for example, if I have users, rides, and vehicles I can't easily use city as a partition key because creating rides will attempt to create an FK with a user and a vehicle using the shared city column and fail with this error cc @awoods187

@bithavoc
Copy link

This is very annoying.
We should have able to use the same columns in multiple foreign keys.

@knz
Copy link
Contributor Author

knz commented Nov 21, 2018

@dt @BramGruneir can we schedule some time to sit together and hash out an analysis of this situation. We need to learn:

  • how to best explain the current situation (how it came to be, what the error means), so that expectations about the current implementation becomes clear

  • what are the various directions we can take from there

  • what amount of work is required for each of them

Once we have hashed this out, we will be able to more confidently and clearly react to user expectations.

@knz
Copy link
Contributor Author

knz commented Nov 27, 2018

Meeting notes from today

  • for the case where is no interleaving / no geo-partitioning [1]

    • mental model is that FK relations express "ownership". A column cannot be owned (conceptually) by more than one constraint. Hence the check.
    • in postgres the relationship is more "mechanical" - defines set predicates per-index (per group of columns, really) but overlapping sets are perfectly fine.
    • in the wild our experience shows that cases where client attempt to issue multiple FK constraints on the same column are outright mistakes, and we were working under the assumption that CockroachDB has a responsibility to block the user from shooting themselves in the foot.
    • for 3rd party compatibility perhaps our working assumption needs to be revisited. However perhaps sql_safe_updates or similar can be used to gate the mistake detection.
  • for the case with interleaving:

    • the same check put in place above (a single column can only be "owned" by 1 FK constraint) now kicks in for "valid" scenarios where the column is an interleaved prefix.
    • this is annoying because the scenario now is not a mistake.

[1] really we would also have ran into this issue with multiple indexes on the same group of columns but different orderings. This is a reasonable / realistic scenario even without geo-partitioning.

Course of action:

  • the overall goal would be to lift the limitation for 3rd party compatibility and also easing the interleaving scenario

  • the way to achieve this goal:

    1. investigate if our execution machinery is healthy if we were to support multiple references
    2. extend the protobuf to support the relationship
    3. extend/lift the check, perhaps with a condition on a session var for compatibility (we need a discussion about opt-in/opt-out)
  • [dt] a low hanging fruit that will help with interleaving but not 3rd party compat is to remove the check that the same prefix is used by multiple constraints.

    • investigate that conflicting rules for cascading behavior are properly handled during execution.

@Prakhash
Copy link

Prakhash commented Jan 9, 2020

Hi,

As per the discussion above, it seems like it's been discussed to lift the limitations with the FK. Any idea when you are planning to do this change. I'm having a similar table as described in [1] and I'm facing the issue which is addressed in [1]. I'm using the latest version of cockroachdb

I really appreciate your comments on this.

Also please let me know if there are any other ways to avoid this as changing the DB schema is not a valid solution for me

[1] #23580 (comment)

Thanks

@awoods187 awoods187 changed the title sql: concern with FKs and interleaved tables sql: columns can only be used in one outgoing foreign key Mar 27, 2020
@awoods187
Copy link
Contributor

awoods187 commented Mar 27, 2020

This came up again when thinking about upgrading from a single-region to multi-region with Movr.
We'd like to adjust the schema to the following:
rides.city, rides.vehicle_id --- fk ---> vehicles.city, vehicles.id
rides.city, rides.rider_id --- fk ---> users.city, users.id
This is currently impossible because you can’t use a column (rides.city) in more than one outgoing foreign key.

cc @RaduBerinde we will want to address this after we plan foreign key cascades

@jordanlewis
Copy link
Member

This is actually going to be fixed in 20.1! 🎉

This is a duplicate of #38850.

@BramGruneir
Copy link
Member

BramGruneir commented Mar 27, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-community Originated from the community S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Development

No branches or pull requests

9 participants