Re: altering a column's collation leaves an invalid foreign key
От | jian he |
---|---|
Тема | Re: altering a column's collation leaves an invalid foreign key |
Дата | |
Msg-id | CACJufxEE_dma4zLh2ydfAfRQPWmJ=M6iV2J-T20BpXm9V0Dzvw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: altering a column's collation leaves an invalid foreign key (jian he <jian.universality@gmail.com>) |
Ответы |
Re: altering a column's collation leaves an invalid foreign key
(jian he <jian.universality@gmail.com>)
|
Список | pgsql-hackers |
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. > > > > + /* test follows the one in ri_FetchConstraintInfo() */ > + if (ARR_NDIM(arr) != 1 || > + ARR_HASNULL(arr) || > + ARR_ELEMTYPE(arr) != INT2OID) > + elog(ERROR, "conkey is not a 1-D smallint array"); > + attarr = (AttrNumber *) ARR_DATA_PTR(arr); > + > + /* stash a List of the collation Oids in our Constraint node */ > + for (i = 0; i < numkeys; i++) > + con->old_collations = lappend_oid(con->old_collations, > + list_nth_oid(changedCollationOids, attarr[i] - 1)); > > I don't understand the "ri_FetchConstraintInfo" comment. I still don't understand this comment. > > +static void > +RememberCollationForRebuilding(AttrNumber attnum, AlteredTableInfo *tab) > +{ > + Oid typid; > + int32 typmod; > + Oid collid; > + ListCell *lc; > + > + /* Fill in the list with InvalidOid if this is our first visit */ > + if (tab->changedCollationOids == NIL) > + { > + int len = RelationGetNumberOfAttributes(tab->rel); > + int i; > + > + for (i = 0; i < len; i++) > + tab->changedCollationOids = lappend_oid(tab->changedCollationOids, > + InvalidOid); > + } > + > + get_atttypetypmodcoll(RelationGetRelid(tab->rel), attnum, > + &typid, &typmod, &collid); > + > + lc = list_nth_cell(tab->changedCollationOids, attnum - 1); > + lfirst_oid(lc) = collid; > +} > > do we need to check if `collid` is a valid collation? > like: > > if (!OidIsValid(collid)) > { > lc = list_nth_cell(tab->changedCollationOids, attnum - 1); > lfirst_oid(lc) = collid; > } now I think RememberCollationForRebuilding is fine. no need further change. in ATAddForeignKeyConstraint. if (old_check_ok) { /* * When a pfeqop changes, revalidate the constraint. We could * permit intra-opfamily changes, but that adds subtle complexity * without any concrete benefit for core types. We need not * assess ppeqop or ffeqop, which RI_Initial_Check() does not use. */ old_check_ok = (pfeqop == lfirst_oid(old_pfeqop_item)); old_pfeqop_item = lnext(fkconstraint->old_conpfeqop, old_pfeqop_item); } maybe we can do the logic right below it. like: if (old_check_ok) { * All deterministic collations use bitwise equality to resolve * ties, but if the previous collation was nondeterministic, * we must re-check the foreign key, because some references * that use to be "equal" may not be anymore. If we have * InvalidOid here, either there was no collation or the * attribute didn't change. old_check_ok = (old_collation == InvalidOid || get_collation_isdeterministic(old_collation)); } then we don't need to cram it with the other `if (old_check_ok){}`. do we need to add an entry in https://wiki.postgresql.org/wiki/PostgreSQL_17_Open_Items#Open_Issues ?
В списке pgsql-hackers по дате отправления: