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

On 2018/12/25 17:03, Michael Paquier wrote:
> On Tue, Dec 25, 2018 at 08:27:19PM +1300, David Rowley wrote:
>> I glanced at this and was confused at where "newrelation" comes from
>> and also the single parameter heap_close(). It does not look like it
>> would compile.
> 
> 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;
        }

> 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 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.

Thanks,
Amit