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

Incorrect Discovery for Composite Foreign Keys in PostgreSQL #100

Closed
julmaxi opened this issue Feb 8, 2023 · 4 comments · Fixed by #118
Closed

Incorrect Discovery for Composite Foreign Keys in PostgreSQL #100

julmaxi opened this issue Feb 8, 2023 · 4 comments · Fixed by #118

Comments

@julmaxi
Copy link
Contributor

julmaxi commented Feb 8, 2023

Description

Postgres schema discovery produces incorrect results for composite foreign keys.
Specifically, for a key with $n$ columns each key is repeated $n$ times in both columns and foreign_columns.

Steps to Reproduce

On a Postgres database:

  1. Create a schema with a composite primary key and a corresponding foreign key. The following is a minimal example:
CREATE TABLE Parent (
    id1 int,
    id2 int,

   PRIMARY KEY (id1, id2)
);

CREATE TABLE Child (
    id INT PRIMARY KEY,
    parent_id1 INT,
    parent_id2 INT,
    
    FOREIGN KEY (parent_id1, parent_id2) REFERENCES Parent(id1, id2)
);
  1. Run schema discovery

Expected Behavior

The references section for the example should like this:

References {
  name: "child_parent_id1_parent_id2_fkey",
  columns: [
    "parent_id1",
    "parent_id2",
  ],
  table: "parent",
  foreign_columns: [
    "id1",
    "id2",
  ],
  on_update: Some(
    NoAction,
  ),
  on_delete: Some(
    NoAction,
  ),
}

Actual Behavior

References {
  name: "child_parent_id1_parent_id2_fkey",
  columns: [
    "parent_id1",
    "parent_id1",
    "parent_id2",
    "parent_id2",
  ],
  table: "parent",
  foreign_columns: [
    "id1",
    "id2",
    "id1",
    "id2",
  ],
  on_update: Some(
    NoAction,
  ),
  on_delete: Some(
    NoAction,
  ),
}

Reproduces How Often

Always

Versions

sea-schema=0.11.0
Postgres 14.6

Additional Information

I am relatively confident that the root cause is this query:

.join_subquery(
JoinType::LeftJoin,
Query::select()
.distinct()
.columns(vec![
(Schema::ReferentialConstraints, RefC::ConstraintName),
(Schema::ReferentialConstraints, RefC::UniqueConstraintSchema),
(Schema::ReferentialConstraints, RefC::UniqueConstraintName),
(Schema::ReferentialConstraints, RefC::MatchOption),
(Schema::ReferentialConstraints, RefC::UpdateRule),
(Schema::ReferentialConstraints, RefC::DeleteRule),
])
.columns(vec![
(Schema::ConstraintColumnUsage, Kcuf::TableName),
(Schema::ConstraintColumnUsage, Kcuf::ColumnName),
])
.from((Schema::Schema, Schema::ReferentialConstraints))
.left_join(
(Schema::Schema, Schema::ConstraintColumnUsage),
Expr::col((Schema::ReferentialConstraints, RefC::ConstraintName))
.equals((Schema::ConstraintColumnUsage, Kcuf::ConstraintName)),
)
.take(),
rcsq.clone(),
Expr::col((Schema::TableConstraints, Tcf::ConstraintName))
.equals((rcsq.clone(), RefC::ConstraintName)),
)

Specifically, the inner select will produce all primary keys targeted by a foreign key. Each of these will be joined with all referencing foreign key columns in a matching child table. This produces all $n \times n$ possible combinations of primary and foreign key columns which is of course fine when $n=1$ but not in any other case.

One potential way to fix this is to additionally query the ordinal_position of each column in the primary key in the inner select.
This requires joining with the key_column_usage table in the inner select to retrieve the primary key.
ordinal_position can then be matched with position_in_unique_constraint in the key_column_usage row for the foreign key.

The overall query would look like this (changes commented)

.join_subquery(
        JoinType::LeftJoin,
        Query::select()
            .distinct()
            .columns(vec![
                (Schema::ReferentialConstraints, RefC::ConstraintName),
                (Schema::ReferentialConstraints, RefC::UniqueConstraintSchema),
                (Schema::ReferentialConstraints, RefC::UniqueConstraintName),
                (Schema::ReferentialConstraints, RefC::MatchOption),
                (Schema::ReferentialConstraints, RefC::UpdateRule),
                (Schema::ReferentialConstraints, RefC::DeleteRule),
            ])
            .columns(vec![
                (Schema::ConstraintColumnUsage, Kcuf::TableName),
                (Schema::ConstraintColumnUsage, Kcuf::ColumnName),
            ])
            .columns(vec![
                // Extract the ordinal position of the referenced primary keys
                (Schema::KeyColumnUsage, Kcuf::OrdinalPosition)
            ])
            .from((Schema::Schema, Schema::ReferentialConstraints))
            .left_join(
                (Schema::Schema, Schema::ConstraintColumnUsage),
                Expr::col((Schema::ReferentialConstraints, RefC::ConstraintName))
                    .equals((Schema::ConstraintColumnUsage, Kcuf::ConstraintName)),
            )
            .left_join(
                // Join the key_column_usage rows for the referenced primary keys
                (Schema::Schema, Schema::KeyColumnUsage),
                Condition::all()
                .add(
                    Expr::col((Schema::ConstraintColumnUsage, Kcuf::ColumnName))
                    .equals((Schema::KeyColumnUsage, Kcuf::ColumnName)),
                ).add(
                    Expr::col((Schema::ReferentialConstraints, RefC::UniqueConstraintName))
                    .equals((Schema::KeyColumnUsage, Kcuf::ConstraintName))
                ).add(
                    Expr::col((Schema::ReferentialConstraints, RefC::UniqueConstraintSchema))
                    .equals((Schema::KeyColumnUsage, Kcuf::ConstraintSchema))
                )
            )
            .take(),
        rcsq.clone(),
        Condition::all().add(
            Expr::col((Schema::TableConstraints, Tcf::ConstraintName))
            .equals((rcsq.clone(), RefC::ConstraintName))
        ).add(
            // Only join when the referenced primary key position matches position_in_unique_constraint for the foreign key column
            Expr::col((Schema::KeyColumnUsage, Kcuf::PositionInUniqueConstraint))
            .equals((rcsq.clone(), Kcuf::OrdinalPosition))
        ))

However, I am not sure if I overlook any side effects of this fix.

@billy1624 billy1624 moved this from Triage to Next Up in SeaQL Dev Tracker Mar 23, 2023
@WinLinux1028
Copy link

Ping for @julmaxi
I tested your code, and It fixed this and SeaQL/sea-orm#1690
Please send your code as pull request.
If you don't do it, I'll submit a pull request on behalf of you.

@julmaxi
Copy link
Contributor Author

julmaxi commented Jan 7, 2024

Thanks for testing. I will create a pull request.

@WinLinux1028
Copy link

@julmaxi May I send this as pull request?

julmaxi added a commit to julmaxi/sea-schema that referenced this issue Jan 14, 2024
tyt2y3 pushed a commit that referenced this issue Jan 18, 2024
* Fix incorrect discovery of composite foreign keys in PostgreSQL

* Fixes #100
* Should also fix SeaQL/sea-orm#1690

* fmt

* Add test cases

---------

Co-authored-by: Billy Chan <[email protected]>
@github-project-automation github-project-automation bot moved this from Next Up to Done in SeaQL Dev Tracker Jan 18, 2024
@bouk
Copy link

bouk commented Sep 5, 2024

I've run into a semi-related issue where I still couldn't get the code to generate correctly in a composite foreign key but the problem was that I had a foreign key that referenced something that has a unique index but not a unique constraint, also see this: https://stackoverflow.com/questions/61249732/null-values-for-referential-constraints-unique-constraint-columns-in-informati

The fix was to run code like this:

ALTER TABLE table_name
ADD CONSTRAINT constraint_name UNIQUE USING INDEX index_name;

And then the constraint is correctly referenced in the information_schema

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants