Re: altering a column's collation leaves an invalid foreign key
jian he <jian.universality@gmail.com>
From: jian he <jian.universality@gmail.com>
To: Paul Jungwirth <pj@illuminatedcomputing.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-04-13T13:13:00Z
Lists: pgsql-hackers
Attachments
- v3-0001-ALTER-COLUMN-SET-DATA-TYPE-Re-check-foreign-key-t.patch (text/x-patch) patch v3-0001
On Fri, Apr 12, 2024 at 5:06 PM jian he <jian.universality@gmail.com> wrote: > > On Tue, Mar 26, 2024 at 1:00 PM jian he <jian.universality@gmail.com> wrote: > > > > On Mon, Mar 25, 2024 at 2:47 PM Paul Jungwirth > > <pj@illuminatedcomputing.com> wrote: > > > > > > On 3/23/24 10:04, Paul Jungwirth wrote: > > > > Perhaps if the previous collation was nondeterministic we should force a re-check. > > > > > > Here is a patch implementing this. It was a bit more fuss than I expected, so maybe someone has a > > > better way. I think I found a simple way. the logic is: * ATExecAlterColumnType changes one column once at a time. * one column can only have one collation. so we don't need to store a list of collation oid. * ATExecAlterColumnType we can get the new collation (targetcollid) and original collation info. * RememberAllDependentForRebuilding will check the column dependent, whether this column is referenced by a foreign key or not information is recorded. so AlteredTableInfo->changedConstraintOids have the primary key and foreign key oids. * ATRewriteCatalogs will call ATPostAlterTypeCleanup (see the comments in ATRewriteCatalogs) * for tab->changedConstraintOids (foreign key, primary key) will call ATPostAlterTypeParse, so for foreign key (con->contype == CONSTR_FOREIGN) will call TryReuseForeignKey. * in TryReuseForeignKey, we can pass the information that our primary key old collation is nondeterministic and old collation != new collation to the foreign key constraint. so foreign key can act accordingly at ATAddForeignKeyConstraint (old_check_ok). based on the above logic, I add one bool in struct AlteredTableInfo, one bool in struct Constraint. bool in AlteredTableInfo is for storing it, later passing it to struct Constraint. we need bool in Constraint because ATAddForeignKeyConstraint needs it.
Commits
-
Fix error code for referential action RESTRICT
- 086c84b23d99 18.0 landed
-
doc: Improve description of referential actions
- 1e08905842fb 18.0 landed
-
Add tests for foreign keys with case-insensitive collations
- 4a2dbfc6be45 18.0 landed
-
Fix collation handling for foreign keys
- 9321d2fdf808 18.0 landed
-
Clarify a foreign key error message
- d7a2b5bd8718 18.0 landed