Re: BUG #15565: truncate bug with tables which have temp table inherited

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, digoal(at)126(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org, PG Bug reporting form <noreply(at)postgresql(dot)org>
Subject: Re: BUG #15565: truncate bug with tables which have temp table inherited
Date: 2018-12-26 01:51:27
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-bugs

On Tue, Dec 25, 2018 at 05:46:28PM +0900, Amit Langote wrote:
> On 2018/12/25 17:03, Michael Paquier wrote:
>> Nope, it doesn't. heap_close ought to not normally release the lock
>> either until the transaction has committed.
> Note that expand_inherited_rtentry does release the lock.
> /*
> * It is possible that the parent table has children that are temp
> * tables of other backends. We cannot safely access such tables
> * (because of buffering issues), and the best thing to do seems
> * to be to silently ignore them.
> */
> if (childOID != parentOID && RELATION_IS_OTHER_TEMP(newrelation))
> {
> heap_close(newrelation, lockmode);
> continue;
> }

Oh, good point here. Both David and you have been touching this area
of the code way more than myself lately.

>> The patch clobbers
>> something that truncate_check_activity() already checks, which is not
>> elegant.
> Indeed, I missed truncate_check_activity. Although, if we want to fix
> this behavior like I'm proposing (ignore child tables that are temporary
> tables of other sessions), it may not be such a good idea to do it by
> modifying truncate_check_activity to deal specially with such tables,
> because that would unnecessarily complicate its interface.

I got to think more about that point, and indeed I agree with your
point to complicate truncate_check_activity more than necessary as it
still gets used for CASCADE and parent relations, so what you are
proposing is acceptable to me.

>> I am wondering as well if we could take this occasion for
>> having better isolation testing when it comes to inheritance trees
>> mixing relation persistency. At least for the TRUNCATE case it would
>> be nice to have something.
> Yeah, perhaps.

Let's bite the bullet then. Attached is a more advanced patch which
is based on what you previously sent, except that I don't like much
the fact of copying AccessExclusiveLock around, so let's save it into
a separate variable. I hope that's clearer. I have also designed a
set of isolation tests which adds more coverage for inheritance trees
mixing persistence across sessions which I also used to check the
patch. This test suite could always be expanded later on, but I think
that's already a step in the good direction.


Attachment Content-Type Size
truncate-ignore-other-session-temp-children-v3.patch text/x-diff 12.5 KB

In response to


Browse pgsql-bugs by date

  From Date Subject
Next Message Michael Paquier 2018-12-26 01:55:28 Re: BUG #15564: Setup sets wrong data type for value in Windows Registry
Previous Message Panagiotis Drivilas 2018-12-26 00:59:38 Re: BUG #15564: Setup sets wrong data type for value in Windows Registry