Thread: Busted(?) optimization in ATAddForeignKeyConstraint
I happened to notice this comment in the logic in
ATAddForeignKeyConstraint that tries to decide if it can skip
revalidating a foreign-key constraint after a DDL change:
* Since we require that all collations share the same notion of
* equality (which they do, because texteq reduces to bitwise
* equality), we don't compare collation here.
Hasn't this been broken by the introduction of nondeterministic
collations?
regards, tom lane
On Fri, Jan 24, 2020 at 11:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I happened to notice this comment in the logic in > ATAddForeignKeyConstraint that tries to decide if it can skip > revalidating a foreign-key constraint after a DDL change: > > * Since we require that all collations share the same notion of > * equality (which they do, because texteq reduces to bitwise > * equality), we don't compare collation here. > > Hasn't this been broken by the introduction of nondeterministic > collations? Similar words appear in the comment for ri_GenerateQualCollation().
On 2020-01-23 23:11, Tom Lane wrote: > I happened to notice this comment in the logic in > ATAddForeignKeyConstraint that tries to decide if it can skip > revalidating a foreign-key constraint after a DDL change: > > * Since we require that all collations share the same notion of > * equality (which they do, because texteq reduces to bitwise > * equality), we don't compare collation here. > > Hasn't this been broken by the introduction of nondeterministic > collations? I'm not very familiar with the logic in this function, but I think this might be okay because the foreign-key equality comparisons are done with the collation of the primary key, which doesn't change here AFAICT. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-01-24 01:21, Thomas Munro wrote: > On Fri, Jan 24, 2020 at 11:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I happened to notice this comment in the logic in >> ATAddForeignKeyConstraint that tries to decide if it can skip >> revalidating a foreign-key constraint after a DDL change: >> >> * Since we require that all collations share the same notion of >> * equality (which they do, because texteq reduces to bitwise >> * equality), we don't compare collation here. >> >> Hasn't this been broken by the introduction of nondeterministic >> collations? > > Similar words appear in the comment for ri_GenerateQualCollation(). The calls to this function are all conditional on !get_collation_isdeterministic(). The comment should perhaps be changed. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 2020-01-23 23:11, Tom Lane wrote:
>> I happened to notice this comment in the logic in
>> ATAddForeignKeyConstraint that tries to decide if it can skip
>> revalidating a foreign-key constraint after a DDL change:
>> * Since we require that all collations share the same notion of
>> * equality (which they do, because texteq reduces to bitwise
>> * equality), we don't compare collation here.
>> Hasn't this been broken by the introduction of nondeterministic
>> collations?
> I'm not very familiar with the logic in this function, but I think this
> might be okay because the foreign-key equality comparisons are done with
> the collation of the primary key, which doesn't change here AFAICT.
If we're depending on that, we should just remove the comment and compare
the collations. Seems far less likely to break.
Even if there's a reason not to do the comparison, the comment needs
an update.
regards, tom lane