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-29 03:59:42
Message-ID: 20181029035942.GG14242@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 19, 2018 at 06:55:09PM +0900, Amit Langote wrote:
> Yeah, we could make it the responsibility of the callers of
> find_all_inheritors and find_inheritance_children to check relhassubclass
> as an optimization and remove any reference to relhassubclass from
> pg_inherits.c. Although we can write such a patch, it seems like it'd be
> bigger than the patch to ensure the correct value of relhassubclass for
> indexes, which I just posted on the other thread [1].

And what is present as patch 0001 on this thread has been committed as
55853d6, so we are good for this part.

>> Anyway, it seems that you are right here. Just setting relhassubclass
>> for partitioned indexes feels more natural with what's on HEAD now.
>> Even if I'd like to see all those hypothetical columns in pg_class go
>> away, that cannot happen without a close lookup at the performance
>> impact.
>
> Okay, I updated the patch on this thread.

Thanks for the new version.

> Since the updated patch depends on the correct value of relhassubclass
> being set for indexes, this patch should be applied on top of the other
> patch. I've attached here both.

- if (!has_subclass(parentrelId))
+ if (get_rel_relkind(parentrelId) != RELKIND_PARTITIONED_INDEX &&
+ !has_subclass(parentrelId))
return NIL;

You don't need this bit anymore, relhassubclass is now set for
partitioned indexes.

+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a table, a foreign table, or an index",
+ get_rel_name(rootrelid))));
Should this also list "partitioned tables and partitioned indexes"? The
style is heavy, but that maps with what pgstattuple does..

The tests should include also something for a leaf index when fed to
pg_partition_tree() (in order to control the index names you could just
attach an index to a partition after creating it, but I leave that up to
you).

+ <entry><literal><function>pg_partition_tree(<type>oid</type>)</function></literal></entry>
+ <entry><type>setof record</type></entry>

The change to regclass has not been reflected yet in the documentation
and the implementation, because...

> Another change I made is something Robert and Alvaro seem to agree about
> -- to use regclass instead of oid type as input/output columns.

... I am in minority here, it feels lonely ;)
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2018-10-29 04:02:53 why commutator doesn't work?
Previous Message Michael Paquier 2018-10-29 03:33:51 Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER