Re: heap_sync seems rather oblivious to partitioned tables (wal_level=minimal)

On Wed, Sep 19, 2018 at 01:14:10PM +1200, David Rowley wrote:
> On 19 September 2018 at 12:21, Tomas Vondra
> <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
>> So apparently CopyFrom() invokes heap_sync() on the partitioned
>> relation, which attempts to do mdimmedsync() only on the root. That
>> seems like a bug to me.

And the root should not have any physical storage either, so attempting
to sync it is wrong.

>> Obviously this only applies to wal_level=minimal. There are multiple
>> callers of heap_sync(), but only the COPY seems to be affected by this,
>> because the rest can't see partitioned tables.
>>
>> So it seems heap_sync() needs to be improved to sync all partitions.
>
> Wouldn't it be better to modify copy.c to just perform the heap_sync
> on just the partitions it touches?

Yeah, my gut is telling me that this would be the best approach for now,
still I am not sure that this is the best move in the long term. All
the other callers of heap_sync don't care about partitioned tables, so
we could add an assertion on RELKIND_PARTITIONED_TABLE. Still, one
crazy argument against that approach would be that we'd need to do the
same thing again if one day CTAS or matviews allow some sort of
automatic creation of partitions.
--
Michael