Re: should ConstraintRelationId ins/upd cause relcache invals? - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: should ConstraintRelationId ins/upd cause relcache invals?
Date
Msg-id 201901212325.llc6mkbbd2fb@alvherre.pgsql
Whole thread Raw
In response to Re: should ConstraintRelationId ins/upd cause relcache invals?  (Andres Freund <andres@anarazel.de>)
Responses Re: should ConstraintRelationId ins/upd cause relcache invals?  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
Hello

On 2019-Jan-21, Andres Freund wrote:

> On 2019-01-21 19:40:17 -0300, Alvaro Herrera wrote:
> > On 2019-Jan-21, Tom Lane wrote:
> > 
> > > Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > 
> > > > At https://postgr.es/m/201901182216.nr5clsxrn624@alvherre.pgsql I posted
> > > > a simplistic for the specific problem I found by calling
> > > > CacheInvalidateRelcache in the problem spot.  But I'm wondering if the
> > > > correct fix isn't to have CacheInvalidateHeapTuple deal with FK
> > > > pg_constraint tuples instead, per the attached patch.
> > > 
> > > +1, this is safer than expecting retail relcache inval calls to be
> > > added in all the right places.
> > 
> > Thanks, pushed.
> 
> Given https://www.postgresql.org/message-id/20190121193300.gknn7p4pmmjg7nqf%40alap3.anarazel.de
> and the concerns voiced in the thread quoted therein, I'm a bit
> surprised that you just went ahead with this, and backpatched it to boot.

Sigh.

I don't understand why the arguments about a different patch apply to
this one.  The invalidation I added is for pg_constraint, not pg_index.

Tom argues that pg_index can be updated for reasons other than
creation/deletion of the index, and that the relcache entry should not
be invalidated in those cases.  Maybe that's right, particularly given
that the relcache entry only holds a list of index OIDs, not properties
of each index.  For foreign keys the relcache entry keeps a lot more
than the OIDs; and pg_constraint rows are not updated very much anyway.

OTOH I lean towards your side of the argument in the other thread and I
don't quite understand why you gave up on it.  Tom didn't respond to
your last argumentat, and neither did you indicate that you were
convinced by Tom's argumentation.  My conclusion is that you disagreed
but decided not to push the issue any further, to get the thing done.
What I did here was the same: just get the thing done.

I could just as easily revert this commit and push a lone
CacheInvalidateRelcache where it is needed by the other fix I just
pushed, but that seemed to me the wrong thing to do.  Or I could spend a
few hours figuring out test cases that fail in 9.6 with the lack of
invalidation, but I don't have those hours and the bug isn't mine
anyway.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


pgsql-hackers by date:

Previous
From: Paul Ramsey
Date:
Subject: Re: Changing SQL Inlining Behaviour (or...?)
Next
From: Tom Lane
Date:
Subject: Re: RelationGetIndexAttrBitmap() small deviation between comment and code