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

[BUG]: Multi column foreign key columns are sorted individually on introspection (critical bug) #3993

Open
1 task done
Mardoxx opened this issue Jan 24, 2025 · 8 comments · May be fixed by #3994
Open
1 task done
Labels
bug Something isn't working

Comments

@Mardoxx
Copy link

Mardoxx commented Jan 24, 2025

Prior title: Composite/multi column foreign key constraints keep getting dropped and created

Report hasn't been filed before.

  • I have verified that the bug I'm about to report hasn't been filed before.

What version of drizzle-orm are you using?

0.30.2

What version of drizzle-kit are you using?

0.38.4

Other packages

No response

Describe the Bug

export const userProfileImages = pgTable('user_profile_images', {
  id: uuid('id').primaryKey(),
  ownerId: uuid('owner_id').notNull(),
  image_blob: text('image_blob').notNull(), // whatever
}, (table) => [
  // Ensures (id, ownerId) is also unique for the composite fk to work
  unique().on(table.id, table.ownerId)
]);

export const userProfileCoverImages = pgTable('user_profile_cover_images', {
  id: uuid('id').primaryKey(),
  ownerId: uuid('owner_id').notNull(),
  profileImageId: uuid('profile_image_id')
    .notNull(),
}, (table) => [
  // Guarantees each cover image matches the correct profile + owner
  foreignKey({
    columns: [table.ownerId, table.profileImageId],
    foreignColumns: [userProfileImages.ownerId, userProfileImages.id],
    name: 'user_profile_cover_images_ownerId_profileImageId_fk'
  }).onDelete('cascade')
]);

Run drizzle-kit push twice on this and it will keep doing:

ALTER TABLE "user_profile_cover_images" DROP CONSTRAINT "user_profile_cover_images_ownerId_profileImageId_fk";

ALTER TABLE "user_profile_cover_images" ADD CONSTRAINT "user_profile_cover_images_ownerId_profileImageId_fk" FOREIGN KEY ("owner_id","profile_image_id") REFERENCES "public"."user_profile_images"("owner_id","id") ON DELETE cascade ON UPDATE no action;

If you try using

export const userProfileImages = pgTable('user_profile_images', {
  id: uuid('id'),
  ownerId: uuid('owner_id').notNull(),
  image_blob: text('image_blob').notNull(), // whatever
}, (table) => [
  primaryKey({ columns: [table.id, table.ownerId] })
]);

It tries to drop non null values in the id field

ALTER TABLE "user_profile_cover_images" DROP CONSTRAINT "user_profile_cover_images_ownerId_profileImageId_fk";
ALTER TABLE "user_profile_images" ALTER COLUMN "id" DROP NOT NULL;

ALTER TABLE "user_profile_cover_images" ADD CONSTRAINT "user_profile_cover_images_ownerId_profileImageId_fk" FOREIGN KEY ("owner_id","profile_image_id") REFERENCES "public"."user_profile_images"("owner_id","id") ON DELETE cascade ON UPDATE no action;

See below comment for why this happens.

@Mardoxx Mardoxx added the bug Something isn't working label Jan 24, 2025
@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

Possibly related #3238

Introspect after push gives

foreignKey({
	columns: [table.ownerId, table.profileImageId],
	foreignColumns: [userProfileImages.id, userProfileImages.ownerId],
	name: "user_profile_cover_images_ownerId_profileImageId_fk"
}).onDelete("cascade"),

N.B.

foreignColumns: [userProfileImages.id, userProfileImages.ownerId],

vs the original

foreignColumns: [userProfileImages.ownerId, userProfileImages.id],

The introspected version is entirely wrong. ownerId should match up with userProfileImages.ownerId.

I suspect the introspection code is responsible for building the tree used for the diff, and that’s where the critical failure occurs. The two arrays should be sorted based on the indices of the first array while maintaining their original pairings.

For example:

Input:

[a, d, c]
[3, 2, 4]

Expected Sorting:

[a, c, d]
[3, 4, 2]

Incorrect (current) Outcome:

[a, c, d]
[2, 3, 4]

Perhaps there is a better mechanism to describe these also, rather than relying on the pairing of two distinct arrays.

foreignKey({
  name: "whatever_fk",
  references: [
    {column: table.blah, foreignColumn: other.blah},
    {column: table.otherBlah, foreignColumn: other.otherBlah},
  ]
})

or better

foreignKey({
  name: 'whatever_fk',
  columns: [
    [table.ownerId, userProfileImages.ownerId],
    [table.profileImageId, userProfileImages.id],
  ],
}).onDelete('cascade')

or even better

foreignKey('whatever_fk')
  .from(table.blah).to(other.blah)
  .from(table.otherBlah).to(other.otherBlah)
  .onDelete('cascade')

@Mardoxx Mardoxx changed the title [BUG]: Composite/multi column foreign key constraints keep getting dropped and created [BUG]: Composite/multi column foreign key constraints keep getting dropped and created (critical introspection bug) Jan 24, 2025
@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

Related #3764 ?

@Mardoxx Mardoxx changed the title [BUG]: Composite/multi column foreign key constraints keep getting dropped and created (critical introspection bug) [BUG]: Multi column foreign key columns are sorted individually on introspection (critical bug) Jan 24, 2025
@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

const tableForeignKeys = await db.query(
`SELECT
con.contype AS constraint_type,
nsp.nspname AS constraint_schema,
con.conname AS constraint_name,
rel.relname AS table_name,
att.attname AS column_name,
fnsp.nspname AS foreign_table_schema,
frel.relname AS foreign_table_name,
fatt.attname AS foreign_column_name,
CASE con.confupdtype
WHEN 'a' THEN 'NO ACTION'
WHEN 'r' THEN 'RESTRICT'
WHEN 'n' THEN 'SET NULL'
WHEN 'c' THEN 'CASCADE'
WHEN 'd' THEN 'SET DEFAULT'
END AS update_rule,
CASE con.confdeltype
WHEN 'a' THEN 'NO ACTION'
WHEN 'r' THEN 'RESTRICT'
WHEN 'n' THEN 'SET NULL'
WHEN 'c' THEN 'CASCADE'
WHEN 'd' THEN 'SET DEFAULT'
END AS delete_rule
FROM
pg_catalog.pg_constraint con
JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
JOIN pg_catalog.pg_namespace nsp ON nsp.oid = con.connamespace
LEFT JOIN pg_catalog.pg_attribute att ON att.attnum = ANY (con.conkey)
AND att.attrelid = con.conrelid
LEFT JOIN pg_catalog.pg_class frel ON frel.oid = con.confrelid
LEFT JOIN pg_catalog.pg_namespace fnsp ON fnsp.oid = frel.relnamespace
LEFT JOIN pg_catalog.pg_attribute fatt ON fatt.attnum = ANY (con.confkey)
AND fatt.attrelid = con.confrelid
WHERE
nsp.nspname = '${tableSchema}'
AND rel.relname = '${tableName}'
AND con.contype IN ('f');`,
);
foreignKeysCount += tableForeignKeys.length;
if (progressCallback) {
progressCallback('fks', foreignKeysCount, 'fetching');
}
for (const fk of tableForeignKeys) {
// const tableFrom = fk.table_name;
const columnFrom: string = fk.column_name;
const tableTo = fk.foreign_table_name;
const columnTo: string = fk.foreign_column_name;
const schemaTo: string = fk.foreign_table_schema;
const foreignKeyName = fk.constraint_name;
const onUpdate = fk.update_rule?.toLowerCase();
const onDelete = fk.delete_rule?.toLowerCase();
if (typeof foreignKeysToReturn[foreignKeyName] !== 'undefined') {
foreignKeysToReturn[foreignKeyName].columnsFrom.push(columnFrom);
foreignKeysToReturn[foreignKeyName].columnsTo.push(columnTo);
} else {
foreignKeysToReturn[foreignKeyName] = {
name: foreignKeyName,
tableFrom: tableName,
tableTo,
schemaTo,
columnsFrom: [columnFrom],
columnsTo: [columnTo],
onDelete,

I don't think this query is correct.

Image

With this our columnsFrom and columnsTo arrays end up as

columnsFrom = [owner_id, owner_id, profile_image_id, profile_image_id]
columnsTo = [id, owner_id, id, owner_id]

When Setted these become [owner_id, profile_image_id] and [id, owner_id]

We need..
FOREIGN KEY ("owner_id","profile_image_id") REFERENCES ... ("owner_id","id")

Are some of the rows in that statement from the unique index?

@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

The issue is caused by how pg_constraint.conkey and pg_constraint.confkey store column references as arrays. When joined with pg_attribute, the query results in Cartesian-like duplication, leading to incorrect mappings.

SELECT
    con.contype AS constraint_type,
    nsp.nspname AS constraint_schema,
    con.conname AS constraint_name,
    rel.relname AS table_name,
    att.attname AS column_name,
    fnsp.nspname AS foreign_table_schema,
    frel.relname AS foreign_table_name,
    fatt.attname AS foreign_column_name,
    CASE con.confupdtype
        WHEN 'a' THEN 'NO ACTION'
        WHEN 'r' THEN 'RESTRICT'
        WHEN 'n' THEN 'SET NULL'
        WHEN 'c' THEN 'CASCADE'
        WHEN 'd' THEN 'SET DEFAULT'
    END AS update_rule,
    CASE con.confdeltype
        WHEN 'a' THEN 'NO ACTION'
        WHEN 'r' THEN 'RESTRICT'
        WHEN 'n' THEN 'SET NULL'
        WHEN 'c' THEN 'CASCADE'
        WHEN 'd' THEN 'SET DEFAULT'
    END AS delete_rule
FROM
    pg_catalog.pg_constraint con
    JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
    JOIN pg_catalog.pg_namespace nsp ON nsp.oid = con.connamespace
    JOIN pg_catalog.pg_class frel ON frel.oid = con.confrelid
    JOIN pg_catalog.pg_namespace fnsp ON fnsp.oid = frel.relnamespace
    CROSS JOIN LATERAL unnest(con.conkey, con.confkey) AS colnum(ref_colnum, foreign_colnum)
    JOIN pg_catalog.pg_attribute att ON att.attnum = colnum.ref_colnum AND att.attrelid = con.conrelid
    JOIN pg_catalog.pg_attribute fatt ON fatt.attnum = colnum.foreign_colnum AND fatt.attrelid = con.confrelid
WHERE
    nsp.nspname = '${tableSchema}' 
    AND rel.relname = '${tableName}' 
    AND con.contype IN ('f');
Image

no cap bussin frfr

similar fixes needed elsewhere most likely

@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

SELECT
    con.contype AS constraint_type,
    nsp.nspname AS constraint_schema,
    con.conname AS constraint_name,
    rel.relname AS table_name,
    att.attname AS column_name,
    fnsp.nspname AS foreign_table_schema,
    frel.relname AS foreign_table_name,
    fatt.attname AS foreign_column_name,
    CASE con.confupdtype
        WHEN 'a' THEN 'NO ACTION'
        WHEN 'r' THEN 'RESTRICT'
        WHEN 'n' THEN 'SET NULL'
        WHEN 'c' THEN 'CASCADE'
        WHEN 'd' THEN 'SET DEFAULT'
    END AS update_rule,
    CASE con.confdeltype
        WHEN 'a' THEN 'NO ACTION'
        WHEN 'r' THEN 'RESTRICT'
        WHEN 'n' THEN 'SET NULL'
        WHEN 'c' THEN 'CASCADE'
        WHEN 'd' THEN 'SET DEFAULT'
    END AS delete_rule
FROM
    pg_catalog.pg_constraint con
    JOIN pg_catalog.pg_class rel ON rel.oid = con.conrelid
    JOIN pg_catalog.pg_namespace nsp ON nsp.oid = con.connamespace
    JOIN LATERAL unnest(con.conkey, con.confkey) WITH ORDINALITY AS fk_map(attnum, fattnum, ord) ON TRUE
    LEFT JOIN pg_catalog.pg_attribute att 
        ON att.attnum = fk_map.attnum AND att.attrelid = con.conrelid
    LEFT JOIN pg_catalog.pg_class frel ON frel.oid = con.confrelid
    LEFT JOIN pg_catalog.pg_namespace fnsp ON fnsp.oid = frel.relnamespace
    LEFT JOIN pg_catalog.pg_attribute fatt 
        ON fatt.attnum = fk_map.fattnum AND fatt.attrelid = con.confrelid
WHERE
    nsp.nspname = '${tableSchema}' 
    AND rel.relname = '${tableName}' 
    AND con.contype IN ('f');
ORDER BY ord;
Image

Better.

@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

SELECT
    con.contype AS constraint_type,
    nsp.nspname AS constraint_schema,
    con.conname AS constraint_name,
    rel.relname AS table_name,
    att.attname AS column_name,
    fnsp.nspname AS foreign_table_schema,
    frel.relname AS foreign_table_name,
    fatt.attname AS foreign_column_name,
    CASE con.confupdtype
        WHEN 'a' THEN 'NO ACTION'
        WHEN 'r' THEN 'RESTRICT'
        WHEN 'n' THEN 'SET NULL'
        WHEN 'c' THEN 'CASCADE'
        WHEN 'd' THEN 'SET DEFAULT'
    END AS update_rule,
    CASE con.confdeltype
        WHEN 'a' THEN 'NO ACTION'
        WHEN 'r' THEN 'RESTRICT'
        WHEN 'n' THEN 'SET NULL'
        WHEN 'c' THEN 'CASCADE'
        WHEN 'd' THEN 'SET DEFAULT'
    END AS delete_rule
FROM pg_catalog.pg_constraint con
JOIN pg_catalog.pg_class rel 
    ON rel.oid = con.conrelid
JOIN pg_catalog.pg_namespace nsp 
    ON nsp.oid = con.connamespace

-- Important part: unnest con.conkey and con.confkey in parallel by index
-- The subscript "s(i)" will range from 1..array_length(conkey)
CROSS JOIN generate_subscripts(con.conkey, 1) AS s(i)

LEFT JOIN pg_catalog.pg_attribute att
    ON att.attrelid = con.conrelid
   AND att.attnum   = con.conkey[s.i]

LEFT JOIN pg_catalog.pg_class frel
    ON frel.oid = con.confrelid

LEFT JOIN pg_catalog.pg_namespace fnsp
    ON fnsp.oid = frel.relnamespace

LEFT JOIN pg_catalog.pg_attribute fatt
    ON fatt.attrelid = con.confrelid
   AND fatt.attnum   = con.confkey[s.i]

WHERE nsp.nspname = '${tableSchema}' 
    AND rel.relname = '${tableName}' 
    AND con.contype IN ('f')

Up to you to decide if you want to ORDER BY con.conname, s.i or not.

This should work for older version of postgres.

@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

This does indeed fix it, someone needs to do a PR now.

@Mardoxx
Copy link
Author

Mardoxx commented Jan 24, 2025

Okay done it for you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant