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