Re: ATTACH/DETACH PARTITION CONCURRENTLY - Mailing list pgsql-hackers

From Amit Langote
Subject Re: ATTACH/DETACH PARTITION CONCURRENTLY
Date
Msg-id c770239c-7126-46e5-ea67-0688e6358a8b@lab.ntt.co.jp
Whole thread Raw
In response to Re: ATTACH/DETACH PARTITION CONCURRENTLY  (Michael Paquier <michael@paquier.xyz>)
Responses Re: ATTACH/DETACH PARTITION CONCURRENTLY
List pgsql-hackers
On 2018/11/15 14:38, Michael Paquier wrote:
> On Thu, Nov 15, 2018 at 01:38:55PM +0900, Amit Langote wrote:
>> I've fixed 0001 again to re-order the code so that allocations happen the
>> correct context and now tests pass with the rebased patches.
> 
> I have been looking at 0001, and it seems to me that you make even more
> messy the current situation.  Coming to my point: do we have actually
> any need to set rel->rd_pdcxt and rel->rd_partdesc at all if a relation
> has no partitions?  It seems to me that we had better set rd_pdcxt and
> rd_partdesc to NULL in this case.

As things stand today, rd_partdesc of a partitioned table must always be
non-NULL.  In fact, there are many places in the backend code that Assert it:

tablecmds.c: ATPrepDropNotNull()

    if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
    {
        PartitionDesc partdesc = RelationGetPartitionDesc(rel);

        Assert(partdesc != NULL);

prepunion.c: expand_partitioned_rtentry()

    PartitionDesc partdesc = RelationGetPartitionDesc(parentrel);

    check_stack_depth();

    /* A partitioned table should always have a partition descriptor. */
    Assert(partdesc);

plancat.c: set_relation_partition_info()

    partdesc = RelationGetPartitionDesc(relation);
    partkey = RelationGetPartitionKey(relation);
    rel->part_scheme = find_partition_scheme(root, relation);
    Assert(partdesc != NULL && rel->part_scheme != NULL);

Maybe there are others in a different form.

If there are no partitions, nparts is 0, and other fields are NULL, though
rd_partdesc itself is never NULL.

If we want to redesign that and allow it to be NULL until some code in the
backend wants to use it, then maybe we can consider doing what you say.
But, many non-trivial operations on partitioned tables require the
PartitionDesc, so there is perhaps not much point to designing it such
that rd_partdesc is set only when needed, because it will be referenced
sooner than later.  Maybe, we can consider doing that sort of thing for
boundinfo, because it's expensive to build, and not all operations want
the canonicalized bounds.

Thanks,
Amit



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: doc fix for pg_stat_activity.backend_type
Next
From: Amit Kapila
Date:
Subject: Re: New function pg_stat_statements_reset_query() to reset statisticsof a specific query