Re: partition tree inspection functions

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeevan Ladhe <jeevan(dot)ladhe(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: partition tree inspection functions
Date: 2018-10-10 04:01:50
Message-ID: 20181010040150.GD2204@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2018-10-10 04:34:24 Re: [HACKERS] Transactions involving multiple postgres foreign servers, take 2
Previous Message Thomas Munro 2018-10-10 03:59:29 Re: background worker shudown (was Re: [HACKERS] Why does logical replication launcher exit with exit code 1?)