Re: partition tree inspection functions - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: partition tree inspection functions
Date
Msg-id 20181010040150.GD2204@paquier.xyz
Whole thread Raw
In response to Re: partition tree inspection functions  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Responses Re: partition tree inspection functions  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
List pgsql-hackers
On Wed, Oct 10, 2018 at 11:54:48AM +0900, Amit Langote wrote:
> I was partly wrong in saying that we wouldn't need any changes to support
> partitioned indexes here.  Actually, the core function
> find_inheritance_children wouldn't scan pg_inherits for us if we pass an
> (partitioned) index to it, even if the index may have children.
>
> It appears that we don't set relhassubclass for partitioned indexes (that
> is, on parent indexes), so the above the test is useless for indexes.

Aha, so that was it.

> Maybe, we need to start another thread to discuss whether we should be
> manipulating relhassubclass for index partitioning (I'd brought this up in
> the context of relispartition, btw [1]).  I'm not immediately sure if
> setting relhassubclass correctly for indexes will have applications beyond
> this one, but it might be something we should let others know so that we
> can hear more opinions.

This reminds of https://postgr.es/m/20180226053613.GD6927@paquier.xyz,
which has resulted in relhaspkey being removed from pg_class with
f66e8bf8.  I would much prefer if we actually removed it...  Now
relhassubclass is more tricky than relhaspkey as it gets used as a
fast-exit path for a couple of things:
1) prepunion.c when planning for child relations.
2) plancat.c
2) analyze.c
3) rewriteHandler.c

> For time being, I've modified that code as follows:
>
> @@ -68,9 +70,11 @@ find_inheritance_children(Oid parentrelId, LOCKMODE
> lockmode)
>
>      /*
>       * Can skip the scan if pg_class shows the relation has never had a
> -     * subclass.
> +     * subclass.  Since partitioned indexes never have their
> +     * relhassubclass set, we cannot skip the scan based on that.
>       */
> -    if (!has_subclass(parentrelId))
> +    if (get_rel_relkind(parentrelId) != RELKIND_PARTITIONED_INDEX &&
> +        !has_subclass(parentrelId))
>          return NIL;

I don't like living with this hack.  What if we simply nuked this
fast-path exit all together?  The only code path where this could
represent a performance issue is RelationBuildPartitionKey(), but we
expect a partitioned table to have children anyway, and there will be
few cases where partitioned tables have no partitions.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: background worker shudown (was Re: [HACKERS] Why does logicalreplication launcher exit with exit code 1?)
Next
From: Masahiko Sawada
Date:
Subject: Re: [HACKERS] Transactions involving multiple postgres foreignservers, take 2