|From:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Michael Paquier <michael(at)paquier(dot)xyz>|
|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|
|Views:||Raw Message | Whole Thread | Download mbox|
On 2018/10/29 12:59, Michael Paquier wrote:
> 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 .
> And what is present as patch 0001 on this thread has been committed as
> 55853d6, so we are good for this part.
Thank you for that. :)
>>> 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
>> 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.
Oh, how did I forget to do that! Removed.
> + 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..
Hmm, I think we mention the word "partitioned" in the error message only
if partitioning is required to perform an operation but it's absent (for
example, trying to attach partition to a non-partitioned table) or if its
presence prevents certain operation from being performed (for example,
calling pgrowlocks() on a partitioned table). Neither seems true in this
case. One can pass a relation of any of the types mentioned in the above
error message to pg_partition_tree and get some output from it.
> 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
> + <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 ;)
I've fixed the documentation to mention regclass as the input type. Also,
I've also modified tests to not use ::regclass.
|Next Message||Pavel Stehule||2018-10-29 07:22:31||Re: why commutator doesn't work?|
|Previous Message||Michael Paquier||2018-10-29 07:06:00||Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line|