Re: Making index_set_state_flags() transactional

On 03.09.2020 11:04, Michael Paquier wrote:
> Hi all,
>
> For a couple of things I looked at lately, it would be really useful
> to make index_state_set_flags() transactional and replace its use of
> heap_inplace_update() by CatalogTupleUpdate():
> - When dropping an index used in a replica identity, we could make the
> reset of relreplident for the parent table consistent with the moment
> the index is marked as invalid:
> https://www.postgresql.org/message-id/20200831062304.GA6555@paquier.xyz
> - In order to make CREATE INDEX CONCURRENTLY work for partitioned
> tables, we need to be able to switch indisvalid for all the index
> partitions in one final transaction so as we have a consistent
> partition tree at any state of the operation.  Obviously,
> heap_inplace_update() is a problem here.
>
> The restriction we have in place now can be lifted thanks to 813fb03,
> that removed SnapshotNow, and as far as I looked at the code paths
> doing scans of pg_index, I don't see any reasons why we could not make
> this stuff transactional.  Perhaps that's just because nobody had use
> cases where it would be useful, and I have two of them lately.  Note
> that 3c84046 was the original commit that introduced the use of
> heap_inplace_update() and index_state_set_flags(), but we don't have
> SnapshotNow anymore.  Note as well that we already make the index
> swapping transactional in REINDEX CONCURRENTLY for the pg_index
> entries.
>
> I got the feeling that this is an issue different than the other
> threads where this was discussed, which is why I am beginning a new
> thread.  Any thoughts?  Attached is a proposal of patch.
>
> Thanks,
> --
> Michael
>
As this patch is needed to continue the work started in 
"reindex/cic/cluster of partitioned tables" thread, I took a look on it.

I agree that transactional update in index_set_state_flags() is good for 
both cases, that you mentioned and don't see any restrictions. It seems 
that not doing this before was just a loose end, as the comment from 
813fb03 patch clearly stated, that it is safe to make this update 
transactional.

The patch itself looks clear and committable.

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company