Re: partition tree inspection functions

[Amit Langote]

On 2018/10/30 10:33, Michael Paquier wrote:
> Thanks for the new version and using better index names.  I have
> reviewed and committed the patch, with a couple of things tweaked:
> - removal of the tests on the size, they don't seem useful as part of
> showing partition information.
> - no need to join on pg_class for the test of the normal table.
> - added some comments and reformulated some other comments.
> - added a test with a materialized view.
> - added a test with a NULL input (as that's a strict function).

Thank you!

Regards,
Amit

Re: partition tree inspection functions

[Michael Paquier]

On Mon, Oct 29, 2018 at 04:08:09PM +0900, Amit Langote wrote:
> 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.

Okay..  We could always reword the error message if there are any
complaints.

> I've fixed the documentation to mention regclass as the input type.  Also,
> I've also modified tests to not use ::regclass.

Thanks for the new version and using better index names.  I have
reviewed and committed the patch, with a couple of things tweaked:
- removal of the tests on the size, they don't seem useful as part of
showing partition information.
- no need to join on pg_class for the test of the normal table.
- added some comments and reformulated some other comments.
- added a test with a materialized view.
- added a test with a NULL input (as that's a strict function).
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

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 [1].
> 
> 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
>>> 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.

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
> you).
> 
> + 
> pg_partition_tree(oid)
> +   setof record
> 
> 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.

Thanks,
Amit
>From 1eb7589b4ded57752c4c285db03e2b0cc37b63dc Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Fri, 5 Oct 2018 14:41:17 +0900
Subject: [PATCH v17] Add pg_partition_tree to display information about
 partitions

This new function is useful to display a full tree of partitions with a
partitioned table given in output, and avoids the need of any complex
WITH RECURSIVE when looking at partition trees which are multi-level
deep.

It returns a set of records, one for each partition, containing the
partition's name, its immediate parent's name, a boolean value telling
if the relation is a leaf in the tree and an integer telling its level
in the partition tree with given table considered as root, beginning at
zero for the root, and incrementing by one each time the scan goes one
level down.

Author: Amit Langote
Reviewed-by: Jesper Pedersen, Michael Paquier
Discussion: 
https://postgr.es/m/8d00e51a-9a51-ad02-d53e-ba6bf50b2...@lab.ntt.co.jp
---
 doc/src/sgml/func.sgml   |  43 
 src/backend/utils/adt/Makefile   |   4 +-
 src/backend/utils/adt/partitionfuncs.c   | 150 +++
 src/include/catalog/pg_proc.dat  |   9 ++
 src/test/regress/expected/partition_info.out | 116 +
 src/test/regress/parallel_schedule   |   2 +-
 src/test/regress/serial_schedule |   1 +
 src/test/regress/sql/partition_info.sql  |  74 +
 8 files changed, 396 insertions(+), 3 deletions(-)
 create mode 100644 src/backend/utils/adt/partitionfuncs.c
 create mode 100644 src/test/regress/expected/partition_info.out
 create mode 100644 src/test/regress/sql/partition_info.sql

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 96d45419e5..58d7ea9da2 100644
--- a/doc/src/sgml/func.sgml
+++ 

Re: partition tree inspection functions

[Michael Paquier]

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).

+ 
pg_partition_tree(oid)
+   setof record

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


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/19 16:47, Michael Paquier wrote:
> On Fri, Oct 19, 2018 at 01:05:52PM +0900, Amit Langote wrote:
>> As I said above, the price of removing relhassubclass might be a bit
>> steep.  So, the other alternative I mentioned before is to set
>> relhassubclass correctly even for indexes if only for pg_partition_tree to
>> be able to use find_inheritance_children unchanged.
> 
> Playing devil's advocate a bit more...  Another alternative here would
> be to remove the fast path using relhassubclass from
> find_inheritance_children and instead have its callers check for it :)

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].

> 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.

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.

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

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/85d50b48-1b59-ae6c-e984-dd0b2926be3c%40lab.ntt.co.jp
>From f0c01ab41b35a5f21a90b0294d8216da78eb8882 Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 19 Oct 2018 17:05:00 +0900
Subject: [PATCH 1/2] Set relhassubclass on index parents

---
 src/backend/catalog/index.c|  5 
 src/backend/commands/indexcmds.c   |  4 +++
 src/test/regress/expected/indexing.out | 46 +-
 src/test/regress/sql/indexing.sql  | 11 ++--
 4 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f1ef4c319a..62cc6a5bb9 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -996,8 +996,13 @@ index_create(Relation heapRelation,
 
/* update pg_inherits, if needed */
if (OidIsValid(parentIndexRelid))
+   {
StoreSingleInheritance(indexRelationId, parentIndexRelid, 1);
 
+   /* Also, set the parent's relhassubclass. */
+   SetRelationHasSubclass(parentIndexRelid, true);
+   }
+
/*
 * Register constraint and dependencies for the index.
 *
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 3975f62c00..c392352871 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2608,6 +2608,10 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
systable_endscan(scan);
relation_close(pg_inherits, RowExclusiveLock);
 
+   /* If we added an index partition to parent, set its relhassubclass. */
+   if (OidIsValid(parentOid))
+   SetRelationHasSubclass(parentOid, true);
+
if (fix_dependencies)
{
ObjectAddress partIdx;
diff --git a/src/test/regress/expected/indexing.out 
b/src/test/regress/expected/indexing.out
index 225f4e9527..710b32192f 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1,25 +1,35 @@
 -- Creating an index on a partitioned table makes the partitions
 -- automatically get the index
 create table idxpart (a int, b int, c text) partition by range (a);
+-- relhassubclass of a partitioned index is false before creating its partition
+-- it will be set below once partitions get created
+create index check_relhassubclass_of_this on idxpart (a);
+select relhassubclass from pg_class where relname = 
'check_relhassubclass_of_this';
+ relhassubclass 
+
+ f
+(1 row)
+
+drop index check_relhassubclass_of_this;
 create table idxpart1 partition of idxpart for values from (0) to (10);
 create table idxpart2 partition of idxpart for values from (10) to (100)
partition by range (b);
 create table idxpart21 partition of idxpart2 for values from (0) to (100);
 create index on idxpart (a);
-select relname, relkind, inhparent::regclass
+select relname, relkind, relhassubclass, inhparent::regclass
 from pg_class left join pg_index ix on (indexrelid = oid)
left join pg_inherits on (ix.indexrelid = inhrelid)
where relname like 'idxpart%' order by relname;
- relname | relkind |   inhparent
--+-+
- idxpart | p   | 
- idxpart1| r   | 
- idxpart1_a_idx  | i   

Re: partition tree inspection functions

[Michael Paquier]

On Fri, Oct 19, 2018 at 01:05:52PM +0900, Amit Langote wrote:
> As I said above, the price of removing relhassubclass might be a bit
> steep.  So, the other alternative I mentioned before is to set
> relhassubclass correctly even for indexes if only for pg_partition_tree to
> be able to use find_inheritance_children unchanged.

Playing devil's advocate a bit more...  Another alternative here would
be to remove the fast path using relhassubclass from
find_inheritance_children and instead have its callers check for it :)

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.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/10 13:01, Michael Paquier wrote:
> 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.gd6...@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

I'm concerned about the 1st one.  Currently in expand_inherited_rtentry(),
checking relhassubclass allows us to short-circuit scanning pg_inherits to
find out if a table has children.  Note that expand_inherited_rtentry() is
called for *all* queries that contain a table so that we correctly
identify tables that would require inheritance planning.  So, removing
relhassubclass means a slight (?) degradation for *all* queries.

>> 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.

As I said above, the price of removing relhassubclass might be a bit
steep.  So, the other alternative I mentioned before is to set
relhassubclass correctly even for indexes if only for pg_partition_tree to
be able to use find_inheritance_children unchanged.

Thanks,
Amit

Re: partition tree inspection functions

[Alvaro Herrera]

On 2018-Oct-12, Robert Haas wrote:

> I think we should be striving to use types like regclass more often in
> system catalogs and views, not less often.

+1

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: partition tree inspection functions

[Robert Haas]

On Tue, Oct 2, 2018 at 11:37 PM Michael Paquier  wrote:
> Putting the new function pg_partition_children() in partition.c is a
> bad idea I think.  So instead I think that we should put that in a
> different location, say utils/adt/partitionfuncs.c.
>
> +   TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relid",
> +  REGCLASSOID, -1, 0);
> +   TupleDescInitEntry(tupdesc, (AttrNumber) 2, "parentid",
> +  REGCLASSOID, -1, 0);
> REGCLASSOID is used mainly for casting, so instead let's use OIDOID like
> any other system function.

-1.  REGCLASSOID is a lot easier for humans to read and can still be
joined to an OID column somewhere if needed.

I think we should be striving to use types like regclass more often in
system catalogs and views, not less often.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: partition tree inspection functions

[Michael Paquier]

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.gd6...@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


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/09 20:17, Michael Paquier wrote:
> On Tue, Oct 09, 2018 at 07:20:40PM +0900, Amit Langote wrote:
>> Sorry if I'm misunderstanding something, but why would we need a new
>> clone?  If we don't change pg_partition_tree() to only accept tables
>> (regular/partitioned/foreign tables) as input, then the same code can work
>> for indexes as well.  As long as we store partition relationship in
>> pg_inherits, same code should work for both.
> 
> Okay, I see what you are coming at.  However, please note that even if I
> can see the dependencies in pg_inherits for indexes, the patch still
> needs some work I am afraid if your intention is to do so:  
> CREATE TABLE ptif_test (a int, b int) PARTITION BY range (a);
> create index ptif_test_i on ptif_test (a);
> CREATE TABLE ptif_test0 PARTITION OF ptif_test
>   FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
> 
> =# select relid::regclass, parentrelid::regclass from
>  pg_partition_tree('ptif_test'::regclass);
>relid| parentrelid
> +-
>  ptif_test  | null
>  ptif_test0 | ptif_test
> (2 rows)
> =# select relid::regclass, parentrelid::regclass from
>  pg_partition_tree('ptif_test_i'::regclass);
> relid| parentrelid
> -+-
>  ptif_test_i | null
> (1 row)
> 
> In this case I would have expected ptif_test0_a_idx to be listed, with
> ptif_test_i as a parent.

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.  That's
because of this quick-return code at the beginning of it:

/*
 * Can skip the scan if pg_class shows the relation has never had a
 * subclass.
 */
if (!has_subclass(parentrelId))
return NIL;

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.

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.

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;

> isleaf is of course wrong if the input is a partitioned index, so more
> regression tests to cover those cases would be nice.

Yeah, I fixed that:

@@ -111,7 +111,8 @@ pg_partition_tree(PG_FUNCTION_ARGS)
 nulls[1] = true;

 /* isleaf */
-values[2] = BoolGetDatum(relkind != RELKIND_PARTITIONED_TABLE);
+values[2] = BoolGetDatum(relkind != RELKIND_PARTITIONED_TABLE &&
+ relkind != RELKIND_PARTITIONED_INDEX);

On 2018/10/09 20:25, Michael Paquier wrote:
> Also, the call to find_all_inheritors needs AccessShareLock...  NoLock
> is not secure.

I had thought about that, but don't remember why I made up my mind about
not taking any lock here.  Maybe we should take locks.  Fixed.

Attached updated patch.  I updated docs and tests to include partitioned
indexes.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/12085bc4-0bc6-0f3a-4c43-57fe0681772b%40lab.ntt.co.jp
>From 46d87e1c44e0a94b72da1393b2922e28f41c22c8 Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Fri, 5 Oct 2018 14:41:17 +0900
Subject: [PATCH v15] Add pg_partition_tree to display information about
 partitions

This new function is useful to display a full tree of partitions with a
partitioned table given in output, and avoids the need of any complex
WITH RECURSIVE when looking at partition trees which are multi-level
deep.

It returns a set of records, one for each partition, containing the
partition OID, its immediate parent OID, if the relation is a leaf in
the tree and its level in the partition tree with given table considered
as root, beginning at zero for the root, and incrementing by one each
time the scan goes one level down.

This commit also changes find_inheritance_children so that it can
be used with partitioned indexes.

Author: Amit Langote
Reviewed-by: Jesper Pedersen, Michael Paquier
Discussion: 
https://postgr.es/m/8d00e51a-9a51-ad02-d53e-ba6bf50b2...@lab.ntt.co.jp
---
 doc/src/sgml/func.sgml   |  43 
 

Re: partition tree inspection functions

[Michael Paquier]

On Tue, Oct 09, 2018 at 08:17:20PM +0900, Michael Paquier wrote:
> isleaf is of course wrong if the input is a partitioned index, so more
> regression tests to cover those cases would be nice.

Also, the call to find_all_inheritors needs AccessShareLock...  NoLock
is not secure.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Michael Paquier]

On Tue, Oct 09, 2018 at 07:20:40PM +0900, Amit Langote wrote:
> Sorry if I'm misunderstanding something, but why would we need a new
> clone?  If we don't change pg_partition_tree() to only accept tables
> (regular/partitioned/foreign tables) as input, then the same code can work
> for indexes as well.  As long as we store partition relationship in
> pg_inherits, same code should work for both.

Okay, I see what you are coming at.  However, please note that even if I
can see the dependencies in pg_inherits for indexes, the patch still
needs some work I am afraid if your intention is to do so:  
CREATE TABLE ptif_test (a int, b int) PARTITION BY range (a);
create index ptif_test_i on ptif_test (a);
CREATE TABLE ptif_test0 PARTITION OF ptif_test
  FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);

=# select relid::regclass, parentrelid::regclass from
 pg_partition_tree('ptif_test'::regclass);
   relid| parentrelid
+-
 ptif_test  | null
 ptif_test0 | ptif_test
(2 rows)
=# select relid::regclass, parentrelid::regclass from
 pg_partition_tree('ptif_test_i'::regclass);
relid| parentrelid
-+-
 ptif_test_i | null
(1 row)

In this case I would have expected ptif_test0_a_idx to be listed, with
ptif_test_i as a parent.

isleaf is of course wrong if the input is a partitioned index, so more
regression tests to cover those cases would be nice.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/09 19:05, Michael Paquier wrote:
> On Tue, Oct 09, 2018 at 06:41:59PM +0900, Amit Langote wrote:
>> Partitioned indexes, just like partitioned tables, don't have their own
>> storage, so pg_relation_size() cannot be used to obtain their size.  We
>> decided that the correct way to get the partitioned table's size is *not*
>> to modify pg_relation_size itself to internally find all partitions and
>> sum their sizes, but to provide a function that returns partitions and
>> tell users to write SQL queries around it to calculate the total size.
>> I'm saying that the new function should be able to be used with
>> partitioned indexes too.
> 
> I have to admit that I find the concept non-intuitive.  A partition tree
> is a set of partitions based on a root called a partitioned table.  A
> partitioned index is not a partition, it is a specific object which is
> associated to a partitioned table without any physical on-disk presence.
> An index is not a partition as well, it is an object associated to one
> relation.

I see it as follows: a partitioned index *does* have partitions and the
partitions in that case are index objects, whereas, a partitioned table's
partitions are table objects.  IOW, I see the word "partition" as
describing a relationship, not any specific object or kind of objects.

> I am not saying that there is no merit in that.  I honestly don't know,
> but being able to list all the partitioned tables and their partition
> tables looks enough to cover all the grounds discussed, and there is no
> need to work out more details for a new clone of find_all_inheritors and
> get_partition_ancestors.

Sorry if I'm misunderstanding something, but why would we need a new
clone?  If we don't change pg_partition_tree() to only accept tables
(regular/partitioned/foreign tables) as input, then the same code can work
for indexes as well.  As long as we store partition relationship in
pg_inherits, same code should work for both.

Thanks,
Amit

Re: partition tree inspection functions

[Michael Paquier]

On Tue, Oct 09, 2018 at 06:41:59PM +0900, Amit Langote wrote:
> Partitioned indexes, just like partitioned tables, don't have their own
> storage, so pg_relation_size() cannot be used to obtain their size.  We
> decided that the correct way to get the partitioned table's size is *not*
> to modify pg_relation_size itself to internally find all partitions and
> sum their sizes, but to provide a function that returns partitions and
> tell users to write SQL queries around it to calculate the total size.
> I'm saying that the new function should be able to be used with
> partitioned indexes too.

I have to admit that I find the concept non-intuitive.  A partition tree
is a set of partitions based on a root called a partitioned table.  A
partitioned index is not a partition, it is a specific object which is
associated to a partitioned table without any physical on-disk presence.
An index is not a partition as well, it is an object associated to one
relation.

I am not saying that there is no merit in that.  I honestly don't know,
but being able to list all the partitioned tables and their partition
tables looks enough to cover all the grounds discussed, and there is no
need to work out more details for a new clone of find_all_inheritors and
get_partition_ancestors.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/09 18:10, Michael Paquier wrote:
> On Tue, Oct 09, 2018 at 05:11:59PM +0900, Amit Langote wrote:
>> Hmm, how would one find the size of a partitioned index tree if we don't
>> allow indexes to be passed?
> 
> pg_total_relation_size() and pg_indexes_size() are designed for that
> purpose.  Anyway, the elements of a partition tree are things which can
> be directly attached to it, like a table or a foreign table, and not
> objects which are directly dependent on a given table, like indexes or
> toast tables.  So we have to add some filtering on the relkind.
> Indexes, partitioned indexes and sequences are three things which cannot
> be added as partitions.  But I think that we had better just make sure
> that the filtering allows RELKIND_RELATION, RELKIND_PARTITIONED_TABLE
> and RELKIND_FOREIGN_TABLE relations only.

I think the way I phrased my question may have been a bit confusing.  I
was pointing out that just like tables, indexes can now also form their
own partition tree.

Partitioned indexes, just like partitioned tables, don't have their own
storage, so pg_relation_size() cannot be used to obtain their size.  We
decided that the correct way to get the partitioned table's size is *not*
to modify pg_relation_size itself to internally find all partitions and
sum their sizes, but to provide a function that returns partitions and
tell users to write SQL queries around it to calculate the total size.
I'm saying that the new function should be able to be used with
partitioned indexes too.

Thanks,
Amit

Re: partition tree inspection functions

[Michael Paquier]

On Tue, Oct 09, 2018 at 05:11:59PM +0900, Amit Langote wrote:
> Hmm, how would one find the size of a partitioned index tree if we don't
> allow indexes to be passed?

pg_total_relation_size() and pg_indexes_size() are designed for that
purpose.  Anyway, the elements of a partition tree are things which can
be directly attached to it, like a table or a foreign table, and not
objects which are directly dependent on a given table, like indexes or
toast tables.  So we have to add some filtering on the relkind.
Indexes, partitioned indexes and sequences are three things which cannot
be added as partitions.  But I think that we had better just make sure
that the filtering allows RELKIND_RELATION, RELKIND_PARTITIONED_TABLE
and RELKIND_FOREIGN_TABLE relations only.

> Maybe, we should apply same rules as are used when describing a table or
> when getting its metadata via other means (pg_relation_size, etc.)?
> Afaics, there are no checks performed in that case:
>
> Should we do anything different in this case?

Yeah, perhaps we could live without any restriction.  There would be
still time to tweak that behavior in the v12 development cycle if there
are any complaints.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/06 15:26, Michael Paquier wrote:
> On Fri, Oct 05, 2018 at 08:22:49AM -0400, Jesper Pedersen wrote:
>> Looks good.
> 
> Actually, after sleeping on it, there could be potentially two problems:
> 1) We don't check the relkind of the relation.  For example it is
> possible to get a tree from an index, which is incorrect.  I would
> suggest to restrain the root relation to be either a relation, a
> partitioned table or a foreign table.

Hmm, how would one find the size of a partitioned index tree if we don't
allow indexes to be passed?

> 2) What about the users who can have a look at a tree?  Shouldn't we
> tighten that a bit more?  I am sure it is not fine to allow any user to
> look at what a partition tree looks like, hence only the owner of the
> root should be able to look at its tree, no?

Maybe, we should apply same rules as are used when describing a table or
when getting its metadata via other means (pg_relation_size, etc.)?
Afaics, there are no checks performed in that case:

create table foo (a int primary key);
create user mt;
revoke all on foo from mt;
set session authorization mt;
\d foo
Table "public.foo"
 Column │  Type   │ Collation │ Nullable │ Default
┼─┼───┼──┼─
 a  │ integer │   │ not null │
Indexes:
"foo_pkey" PRIMARY KEY, btree (a)

select pg_relation_size('foo');
 pg_relation_size
──
0
(1 row)

Should we do anything different in this case?

Thanks,
Amit

Re: partition tree inspection functions

[Michael Paquier]

On Fri, Oct 05, 2018 at 08:22:49AM -0400, Jesper Pedersen wrote:
> Looks good.

Actually, after sleeping on it, there could be potentially two problems:
1) We don't check the relkind of the relation.  For example it is
possible to get a tree from an index, which is incorrect.  I would
suggest to restrain the root relation to be either a relation, a
partitioned table or a foreign table.
2) What about the users who can have a look at a tree?  Shouldn't we
tighten that a bit more?  I am sure it is not fine to allow any user to
look at what a partition tree looks like, hence only the owner of the
root should be able to look at its tree, no?
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Jesper Pedersen]

On 10/5/18 2:52 AM, Michael Paquier wrote:

On Fri, Oct 05, 2018 at 03:31:49PM +0900, Amit Langote wrote:

Thanks for making those changes yourself and posting the new version.

Can you check the attached diff file for some updates to the documentation
part of the patch.  Other parts look fine.


OK, I merged that into my local branch.  From what I can see Mr Robot is
happy as well:
http://cfbot.cputube.org/amit-langote.html


Looks good.

Best regards,
 Jesper

Re: partition tree inspection functions

[Michael Paquier]

On Fri, Oct 05, 2018 at 03:31:49PM +0900, Amit Langote wrote:
> Thanks for making those changes yourself and posting the new version.
> 
> Can you check the attached diff file for some updates to the documentation
> part of the patch.  Other parts look fine.

OK, I merged that into my local branch.  From what I can see Mr Robot is
happy as well:
http://cfbot.cputube.org/amit-langote.html
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/05 14:56, Michael Paquier wrote:
> On Thu, Oct 04, 2018 at 05:18:07PM +0900, Michael Paquier wrote:
>> So it seems that I am clearly outvoted here ;)
>>
>> Okay, let's do as you folks propose.
> 
> And attached is a newer version with this isleaf stuff and the previous
> feedback from Amit integrated, as long as I recall about it.  The
> version is indented, and tutti-quanti.  Does that look fine to everybody
> here?

Thanks for making those changes yourself and posting the new version.

Can you check the attached diff file for some updates to the documentation
part of the patch.  Other parts look fine.

Thanks,
Amit
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index be315aaabb..826c59ecd9 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20209,14 +20209,14 @@ postgres=# SELECT * FROM 
pg_walfile_name_offset(pg_stop_backup());

pg_partition_tree(oid)
setof record

-List information about table in a partition tree for a given
+List information about tables in partition tree for a given
 partitioned table, which consists of one row for each partition and
 table itself.  Information provided includes the OID of the partition,
-the OID of its immediate parent, if the partition is a leaf and its
-level in the hierarchy.  The value of level begins at
-0 for the input table in its role as the root of
-the partition tree, 1 for its partitions,
-2 for their partitions, and so on.
+the OID of its immediate parent, a boolean value telling if the
+partition is a leaf, and an integer telling its level in the hierarchy.
+The value of level begins at 0 for the input table
+in its role as the root of the partition tree, 1 for
+its partitions, 2 for their partitions, and so on.

   
  

Re: partition tree inspection functions

[Pavel Stehule]

pá 5. 10. 2018 v 7:57 odesílatel Michael Paquier 
napsal:

> On Thu, Oct 04, 2018 at 05:18:07PM +0900, Michael Paquier wrote:
> > So it seems that I am clearly outvoted here ;)
> >
> > Okay, let's do as you folks propose.
>
> And attached is a newer version with this isleaf stuff and the previous
> feedback from Amit integrated, as long as I recall about it.  The
> version is indented, and tutti-quanti.  Does that look fine to everybody
> here?
>

looks well

Pavel


> The CF bot should normally pick up this patch, the previous garbage seen
> in one of the tests seems to come from the previous implementation which
> used strings...
> --
> Michael
>

Re: partition tree inspection functions

[Michael Paquier]

On Thu, Oct 04, 2018 at 05:18:07PM +0900, Michael Paquier wrote:
> So it seems that I am clearly outvoted here ;)
> 
> Okay, let's do as you folks propose.

And attached is a newer version with this isleaf stuff and the previous
feedback from Amit integrated, as long as I recall about it.  The
version is indented, and tutti-quanti.  Does that look fine to everybody
here?

The CF bot should normally pick up this patch, the previous garbage seen
in one of the tests seems to come from the previous implementation which
used strings...
--
Michael
From 11bf47ee3137b6a15699bfda50b0904f2e10a415 Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Fri, 5 Oct 2018 14:41:17 +0900
Subject: [PATCH] Add pg_partition_tree to display information about partitions

This new function is useful to display a full tree of partitions with a
partitioned table given in output, and avoids the need of any complex
WITH RECURSIVE when looking at partition trees which are multi-level
deep.

It returns a set of records, one for each partition, containing the
partition OID, its immediate parent OID, if the relation is a leaf in
the tree and its level in the partition tree with given table considered
as root, beginning at zero for the root, and incrementing by one each
time the scan goes one level down.

Author: Amit Langote
Reviewed-by: Jesper Pedersen, Michael Paquier
Discussion: https://postgr.es/m/8d00e51a-9a51-ad02-d53e-ba6bf50b2...@lab.ntt.co.jp
---
 doc/src/sgml/func.sgml   |  42 ++
 src/backend/utils/adt/Makefile   |   4 +-
 src/backend/utils/adt/partitionfuncs.c   | 137 +++
 src/include/catalog/catversion.h |   2 +-
 src/include/catalog/pg_proc.dat  |   9 ++
 src/test/regress/expected/partition_info.out |  67 +
 src/test/regress/parallel_schedule   |   2 +-
 src/test/regress/serial_schedule |   1 +
 src/test/regress/sql/partition_info.sql  |  42 ++
 9 files changed, 302 insertions(+), 4 deletions(-)
 create mode 100644 src/backend/utils/adt/partitionfuncs.c
 create mode 100644 src/test/regress/expected/partition_info.out
 create mode 100644 src/test/regress/sql/partition_info.sql

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f984d069e1..be315aaabb 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20197,6 +20197,48 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
 The function returns the number of new collation objects it created.

 
+   
+Partitioning Information Functions
+
+ 
+  Name Return Type Description
+ 
+
+  
+  
+   pg_partition_tree(oid)
+   setof record
+   
+List information about table in a partition tree for a given
+partitioned table, which consists of one row for each partition and
+table itself.  Information provided includes the OID of the partition,
+the OID of its immediate parent, if the partition is a leaf and its
+level in the hierarchy.  The value of level begins at
+0 for the input table in its role as the root of
+the partition tree, 1 for its partitions,
+2 for their partitions, and so on.
+   
+  
+ 
+
+   
+
+   
+To check the total size of the data contained in
+measurement table described in
+, one could use the
+following query:
+   
+
+
+=# SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
+ FROM pg_partition_tree('measurement'::regclass);
+ total_size 
+
+ 24 kB
+(1 row)
+
+
   
 
   
diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile
index 4b35dbb8bb..132ec7620c 100644
--- a/src/backend/utils/adt/Makefile
+++ b/src/backend/utils/adt/Makefile
@@ -20,8 +20,8 @@ OBJS = acl.o amutils.o arrayfuncs.o array_expanded.o array_selfuncs.o \
 	jsonfuncs.o like.o lockfuncs.o mac.o mac8.o misc.o nabstime.o name.o \
 	network.o network_gist.o network_selfuncs.o network_spgist.o \
 	numeric.o numutils.o oid.o oracle_compat.o \
-	orderedsetaggs.o pg_locale.o pg_lsn.o pg_upgrade_support.o \
-	pgstatfuncs.o \
+	orderedsetaggs.o partitionfuncs.o pg_locale.o pg_lsn.o \
+	pg_upgrade_support.o pgstatfuncs.o \
 	pseudotypes.o quote.o rangetypes.o rangetypes_gist.o \
 	rangetypes_selfuncs.o rangetypes_spgist.o rangetypes_typanalyze.o \
 	regexp.o regproc.o ri_triggers.o rowtypes.o ruleutils.o \
diff --git a/src/backend/utils/adt/partitionfuncs.c b/src/backend/utils/adt/partitionfuncs.c
new file mode 100644
index 00..c1743524c8
--- /dev/null
+++ b/src/backend/utils/adt/partitionfuncs.c
@@ -0,0 +1,137 @@
+/*-
+ *
+ * partitionfuncs.c
+ *	  Functions for accessing partition-related metadata
+ *
+ * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ 

Re: partition tree inspection functions

[Michael Paquier]

On Thu, Oct 04, 2018 at 04:53:02PM +0900, Amit Langote wrote:
> As mentioned in my other reply, that might be considered as asking the
> user to know inner details like relkind.  Also, if a database has many
> partitioned tables with lots of partitions, the pg_class join might get
> expensive.  OTOH, computing and returning it with other fields of
> pg_partition_tree is essentially free.

So it seems that I am clearly outvoted here ;)

Okay, let's do as you folks propose.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/04 9:27, Michael Paquier wrote:
> On Wed, Oct 03, 2018 at 08:12:59AM -0400, Jesper Pedersen wrote:
>> Removing isleaf would require extra round trips to the server to get
>> that information. So, I think we should keep it.
> 
> I don't really get your point about extra round trips with the server,
> and getting the same level of information is as simple as a join between
> the result set of pg_partition_tree() and pg_class (better to add schema
> qualification and aliases to relations by the way):
> =# SELECT relid::regclass,
>   parentrelid::regclass, level,
>   relkind != 'p' AS isleaf
>  FROM pg_partition_tree('ptif_test'::regclass), pg_class
>  WHERE oid = relid;
> relid| parentrelid | level | isleaf
> -+-+---+
>  ptif_test   | null| 0 | f
>  ptif_test0  | ptif_test   | 1 | f
>  ptif_test1  | ptif_test   | 1 | f
>  ptif_test2  | ptif_test   | 1 | t
>  ptif_test01 | ptif_test0  | 2 | t
>  ptif_test11 | ptif_test1  | 2 | t
> (6 rows)

As mentioned in my other reply, that might be considered as asking the
user to know inner details like relkind.  Also, if a database has many
partitioned tables with lots of partitions, the pg_class join might get
expensive.  OTOH, computing and returning it with other fields of
pg_partition_tree is essentially free.

Thanks,
Amit

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/03 12:37, Michael Paquier wrote:
> On Mon, Oct 01, 2018 at 04:27:57PM +0900, Amit Langote wrote:
>> Yeah, maybe there is no reason to delay proceeding with
>> pg_partition_children which provides a useful functionality.
> 
> So, I have been looking at your patch, and there are a couple of things
> which could be improved.

Thanks for reviewing and updating the patch.

> Putting the new function pg_partition_children() in partition.c is a
> bad idea I think.  So instead I think that we should put that in a
> different location, say utils/adt/partitionfuncs.c.

Okay, sounds like a good idea.

> +   TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relid",
> +  REGCLASSOID, -1, 0);
> +   TupleDescInitEntry(tupdesc, (AttrNumber) 2, "parentid",
> +  REGCLASSOID, -1, 0);
> REGCLASSOID is used mainly for casting, so instead let's use OIDOID like
> any other system function.

Check.

> +   values[2] = psprintf("%d", level);
> +   values[3] = psprintf("%c", relkind == RELKIND_PARTITIONED_TABLE ?
> +   'f' :
> +   't');
> Using Datum objects is more elegant in this context.

Agreed.  I think I'd just copied the psprintf code from some other function.

> isleaf is not particularly useful as it can just be fetched with a join
> on pg_class/relkind.  So I think that we had better remove it.

That's a bit imposing on the users to know about relkind, but maybe that's
okay.

> I have cleaned up a bit tests, removing duplicates and most of the
> things which touched the size of relations to have something more
> portable.

Thanks for that.

> We could have a flavor using a relation name in input with qualified
> names handled properly (see pg_get_viewdef_name for example), not sure
> if that's really mandatory so I left that out.

Having to always use the typecast (::regclass) may be a bit annoying, but
we can always add that version later.

> I have also added some
> comments here and there.  The docs could be worded a bit better still.
> 
> My result is the patch attached.  What do you think?

I looked at the updated patch and couldn't resist making some changes,
which see in the attached diff file.  Also attached is the updated patch.

Thanks,
Amit
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d41c09b68b..6dfa3dc977 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20209,12 +20209,13 @@ postgres=# SELECT * FROM 
pg_walfile_name_offset(pg_stop_backup());

pg_partition_tree(oid)
setof record

-List information about a partition tree for the given partitioned
-table, consisting of one row for each partition in a tree.  The
-information available is the OID of the partition, the OID of its
-immediate partitioned table, and its level in the hierarchy,
-beginning at 0 for the top-most parent, and
-incremented by 1 for each level up.
+List information about table in a partition tree for a given
+partitioned table, which consists of one row for each partition and
+table itself.  Information provided includes the OID of the partition,
+the OID of its immediate parent, and its level in the hierarchy.
+The value of level begins at 0 for the input table
+in its role as the root of the partition tree, 1 for
+its partitions, 2 for their partitions, and so on.

   
  
diff --git a/src/backend/utils/adt/partitionfuncs.c 
b/src/backend/utils/adt/partitionfuncs.c
index fc0a904967..41c57cac2d 100644
--- a/src/backend/utils/adt/partitionfuncs.c
+++ b/src/backend/utils/adt/partitionfuncs.c
@@ -1,7 +1,7 @@
 /*-
  *
  * partitionfuncs.c
- *   Functions for accessing partitioning data
+ *   Functions for accessing partitioning related metadata
  *
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
diff --git a/src/test/regress/sql/partition_info.sql 
b/src/test/regress/sql/partition_info.sql
index 2ad75882b0..d9d96a284c 100644
--- a/src/test/regress/sql/partition_info.sql
+++ b/src/test/regress/sql/partition_info.sql
@@ -17,11 +17,24 @@ SELECT relid::regclass, parentrelid::regclass, level,
 pg_relation_size(relid) = 0 AS is_empty
   FROM pg_partition_tree('ptif_test'::regclass);
 
--- children of the main tree
-SELECT relid::regclass, parentrelid::regclass, level
-  FROM pg_partition_tree('ptif_test0'::regclass);
-SELECT relid::regclass, parentrelid::regclass, level
-  FROM pg_partition_tree('ptif_test01'::regclass);
+-- check that it works correctly even if it's passed other tables in the tree
+
+-- passing an intermediate level partitioned table
+SELECT relid::regclass, parentrelid::regclass, level, relkind <> 'p' AS isleaf
+  FROM 

Re: partition tree inspection functions

[Michael Paquier]

On Wed, Oct 03, 2018 at 08:12:59AM -0400, Jesper Pedersen wrote:
> Removing isleaf would require extra round trips to the server to get
> that information. So, I think we should keep it.

I don't really get your point about extra round trips with the server,
and getting the same level of information is as simple as a join between
the result set of pg_partition_tree() and pg_class (better to add schema
qualification and aliases to relations by the way):
=# SELECT relid::regclass,
  parentrelid::regclass, level,
  relkind != 'p' AS isleaf
 FROM pg_partition_tree('ptif_test'::regclass), pg_class
 WHERE oid = relid;
relid| parentrelid | level | isleaf
-+-+---+
 ptif_test   | null| 0 | f
 ptif_test0  | ptif_test   | 1 | f
 ptif_test1  | ptif_test   | 1 | f
 ptif_test2  | ptif_test   | 1 | t
 ptif_test01 | ptif_test0  | 2 | t
 ptif_test11 | ptif_test1  | 2 | t
(6 rows)
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Michael,

On 10/2/18 11:37 PM, Michael Paquier wrote:

isleaf is not particularly useful as it can just be fetched with a join
on pg_class/relkind.  So I think that we had better remove it.



Removing isleaf would require extra round trips to the server to get 
that information. So, I think we should keep it.


Best regards,
 Jesper

Re: partition tree inspection functions

[Michael Paquier]

On Mon, Oct 01, 2018 at 04:27:57PM +0900, Amit Langote wrote:
> Yeah, maybe there is no reason to delay proceeding with
> pg_partition_children which provides a useful functionality.

So, I have been looking at your patch, and there are a couple of things
which could be improved.

Putting the new function pg_partition_children() in partition.c is a
bad idea I think.  So instead I think that we should put that in a
different location, say utils/adt/partitionfuncs.c.

+   TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relid",
+  REGCLASSOID, -1, 0);
+   TupleDescInitEntry(tupdesc, (AttrNumber) 2, "parentid",
+  REGCLASSOID, -1, 0);
REGCLASSOID is used mainly for casting, so instead let's use OIDOID like
any other system function.

+   values[2] = psprintf("%d", level);
+   values[3] = psprintf("%c", relkind == RELKIND_PARTITIONED_TABLE ?
+   'f' :
+   't');
Using Datum objects is more elegant in this context.

isleaf is not particularly useful as it can just be fetched with a join
on pg_class/relkind.  So I think that we had better remove it.

I have cleaned up a bit tests, removing duplicates and most of the
things which touched the size of relations to have something more
portable.

We could have a flavor using a relation name in input with qualified
names handled properly (see pg_get_viewdef_name for example), not sure
if that's really mandatory so I left that out.  I have also added some
comments here and there.  The docs could be worded a bit better still.

My result is the patch attached.  What do you think?
--
Michael
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9a7f683658..d41c09b68b 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20197,6 +20197,46 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
 The function returns the number of new collation objects it created.

 
+   
+Partitioning Information Functions
+
+ 
+  Name Return Type Description
+ 
+
+  
+  
+   pg_partition_tree(oid)
+   setof record
+   
+List information about a partition tree for the given partitioned
+table, consisting of one row for each partition in a tree.  The
+information available is the OID of the partition, the OID of its
+immediate partitioned table, and its level in the hierarchy,
+beginning at 0 for the top-most parent, and
+incremented by 1 for each level up.
+   
+  
+ 
+
+   
+
+   
+To check the total size of the data contained in
+measurement table described in
+, one could use the
+following query:
+   
+
+
+=# SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
+ FROM pg_partition_tree('measurement');
+ total_size 
+
+ 24 kB
+(1 row)
+
+
   
 
   
diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile
index 4b35dbb8bb..132ec7620c 100644
--- a/src/backend/utils/adt/Makefile
+++ b/src/backend/utils/adt/Makefile
@@ -20,8 +20,8 @@ OBJS = acl.o amutils.o arrayfuncs.o array_expanded.o array_selfuncs.o \
 	jsonfuncs.o like.o lockfuncs.o mac.o mac8.o misc.o nabstime.o name.o \
 	network.o network_gist.o network_selfuncs.o network_spgist.o \
 	numeric.o numutils.o oid.o oracle_compat.o \
-	orderedsetaggs.o pg_locale.o pg_lsn.o pg_upgrade_support.o \
-	pgstatfuncs.o \
+	orderedsetaggs.o partitionfuncs.o pg_locale.o pg_lsn.o \
+	pg_upgrade_support.o pgstatfuncs.o \
 	pseudotypes.o quote.o rangetypes.o rangetypes_gist.o \
 	rangetypes_selfuncs.o rangetypes_spgist.o rangetypes_typanalyze.o \
 	regexp.o regproc.o ri_triggers.o rowtypes.o ruleutils.o \
diff --git a/src/backend/utils/adt/partitionfuncs.c b/src/backend/utils/adt/partitionfuncs.c
new file mode 100644
index 00..fc0a904967
--- /dev/null
+++ b/src/backend/utils/adt/partitionfuncs.c
@@ -0,0 +1,128 @@
+/*-
+ *
+ * partitionfuncs.c
+ *		  Functions for accessing partitioning data
+ *
+ * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *		  src/backend/utils/adt/partitionfuncs.c
+ *
+ *-
+ */
+
+#include "postgres.h"
+
+#include "access/htup_details.h"
+#include "catalog/partition.h"
+#include "catalog/pg_inherits.h"
+#include "catalog/pg_type.h"
+#include "funcapi.h"
+#include "utils/fmgrprotos.h"
+
+/*
+ * pg_partition_tree
+ *
+ * Produce a view with one row per member of a partition tree, beginning
+ * from the top-most parent given by the caller.  This gives information
+ * about each partition, its immediate partitioned parent and its level in
+ * the hierarchy.
+ */
+Datum
+pg_partition_tree(PG_FUNCTION_ARGS)
+{
+#define 

Re: partition tree inspection functions

[Amit Langote]

On 2018/10/01 15:27, Michael Paquier wrote:
> On Mon, Oct 01, 2018 at 03:16:32PM +0900, Amit Langote wrote:
>> I wasn't able to respond to some of issues that Jesper brought up with the
>> approach taken by the latest patch whereby there is no separate
>> pg_partition_level function.  He said that such a function would be useful
>> to get the information about the individual leaf partitions, but I was no
>> longer sure of providing such a function separately.
> 
> Perhaps that could be debated separately as well?  From what I can see
> what's available would unlock the psql patch which would like to add
> support for \dP, or show the size of partitions more easily.

Yeah, maybe there is no reason to delay proceeding with
pg_partition_children which provides a useful functionality.

> I am also
> not completely sure that I see the use-case for pg_partition_level or
> even pg_partition_root_parent as usually in their schemas users append
> rather similar relation names to the parent and the children.  Or
> perhaps not?

We can continue discussing that once we're done dealing with
pg_partition_children and then some other patches that are pending due to
it such as Pavel's.

Thanks,
Amit

Re: partition tree inspection functions

[Michael Paquier]

On Mon, Oct 01, 2018 at 03:16:32PM +0900, Amit Langote wrote:
> I wasn't able to respond to some of issues that Jesper brought up with the
> approach taken by the latest patch whereby there is no separate
> pg_partition_level function.  He said that such a function would be useful
> to get the information about the individual leaf partitions, but I was no
> longer sure of providing such a function separately.

Perhaps that could be debated separately as well?  From what I can see
what's available would unlock the psql patch which would like to add
support for \dP, or show the size of partitions more easily.  I am also
not completely sure that I see the use-case for pg_partition_level or
even pg_partition_root_parent as usually in their schemas users append
rather similar relation names to the parent and the children.  Or
perhaps not?
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

Hi,

On 2018/10/01 15:03, Michael Paquier wrote:
> On Thu, Aug 09, 2018 at 01:05:56PM +0900, Amit Langote wrote:
>> Attached updated patch.
> 
> So, except if I am missing something, what we have here is a patch which
> has been debatted quite a bit and has semantics which look nice.

Thanks.

>  Any
> objections if we move forward with this patch?

I wasn't able to respond to some of issues that Jesper brought up with the
approach taken by the latest patch whereby there is no separate
pg_partition_level function.  He said that such a function would be useful
to get the information about the individual leaf partitions, but I was no
longer sure of providing such a function separately.

> +-- all tables in the tree
> +select *, pg_relation_size(relid) as size from
> pg_partition_children('ptif_test');
> +relid|  parentid  | level | isleaf | size
> +-++---++---
> + ptif_test   || 0 | f  | 0
> + ptif_test0  | ptif_test  | 1 | f  | 0
> + ptif_test1  | ptif_test  | 1 | f  | 0
> + ptif_test2  | ptif_test  | 1 | t  | 16384
> + ptif_test01 | ptif_test0 | 2 | t  | 24576
> 
> One thing is that this test depends on the page size.  There are already
> plan modifications if running the regress tests with a size other than
> 8kB, but I don't think that we should make that worse, so I would
> suggest to replace to use "pg_relation_size(relid) > 0" instead.

Might be a good idea, will do.

> I have moved the patch to next CF for now.

Thank you, I'll submit an updated version soon.

Regards,
Amit

Re: partition tree inspection functions

[Michael Paquier]

On Thu, Aug 09, 2018 at 01:05:56PM +0900, Amit Langote wrote:
> Attached updated patch.

So, except if I am missing something, what we have here is a patch which
has been debatted quite a bit and has semantics which look nice.  Any
objections if we move forward with this patch?

+-- all tables in the tree
+select *, pg_relation_size(relid) as size from
pg_partition_children('ptif_test');
+relid|  parentid  | level | isleaf | size
+-++---++---
+ ptif_test   || 0 | f  | 0
+ ptif_test0  | ptif_test  | 1 | f  | 0
+ ptif_test1  | ptif_test  | 1 | f  | 0
+ ptif_test2  | ptif_test  | 1 | t  | 16384
+ ptif_test01 | ptif_test0 | 2 | t  | 24576

One thing is that this test depends on the page size.  There are already
plan modifications if running the regress tests with a size other than
8kB, but I don't think that we should make that worse, so I would
suggest to replace to use "pg_relation_size(relid) > 0" instead.

I have moved the patch to next CF for now.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

Thanks Thomas for notifying.

On 2018/08/08 20:47, Thomas Munro wrote:
> On Wed, Aug 8, 2018 at 11:21 PM, Thomas Munro
>  wrote:
>> On Tue, Aug 7, 2018 at 7:32 PM, Amit Langote
>>  wrote:
>>> Attached updated patch adds isleaf to pg_partition_children's output.
>>
>> Hmm, I wonder where this garbage is coming from:
> 
> partition.c:437:3: error: array index 3 is past the end of the array
> (which contains 3 elements) [-Werror,-Warray-bounds]

Oops, fixed.

> That'll do it.  Also:
> 
> partition.c:409:19: error: implicit declaration of function
> 'get_rel_relkind' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]

Fixed too.

Attached updated patch.

Thanks,
Amit
From de9a597f8cfc1e8707c047ec7ec760ef01244a03 Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 3 Aug 2018 17:06:05 +0900
Subject: [PATCH v11] Add pg_partition_children to report partitions

It returns set of records one for each partition containing the
partition name, parent name, and level in the partition tree with
given table as root
---
 doc/src/sgml/func.sgml   | 37 
 src/backend/catalog/partition.c  | 90 
 src/include/catalog/pg_proc.dat  |  9 +++
 src/test/regress/expected/partition_info.out | 84 ++
 src/test/regress/parallel_schedule   |  2 +-
 src/test/regress/serial_schedule |  1 +
 src/test/regress/sql/partition_info.sql  | 35 +++
 7 files changed, 257 insertions(+), 1 deletion(-)
 create mode 100644 src/test/regress/expected/partition_info.out
 create mode 100644 src/test/regress/sql/partition_info.sql

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index edc9be92a6..6b10aa3b3d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19995,6 +19995,43 @@ postgres=# SELECT * FROM 
pg_walfile_name_offset(pg_stop_backup());
 The function returns the number of new collation objects it created.

 
+   
+Partitioning Information Functions
+
+ 
+  Name Return Type 
Description
+ 
+
+  
+  
+   
pg_partition_children(regclass)
+   setof record
+   
+List name, parent name, level, and whether it's a leaf partition for
+each partition contained in the partition tree with given root table,
+including the root table itself
+   
+  
+ 
+
+   
+
+   
+To check the total size of the data contained in
+measurement table described in
+, one could use the
+following query:
+   
+
+
+select pg_size_pretty(sum(pg_relation_size(relid))) as total_size from 
pg_partition_children('measurement');
+ total_size 
+
+ 24 kB
+(1 row)
+
+
+
   
 
   
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 558022647c..6061bfc369 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -23,13 +23,17 @@
 #include "catalog/partition.h"
 #include "catalog/pg_inherits.h"
 #include "catalog/pg_partitioned_table.h"
+#include "catalog/pg_type.h"
+#include "funcapi.h"
 #include "nodes/makefuncs.h"
 #include "optimizer/clauses.h"
 #include "optimizer/prep.h"
 #include "optimizer/var.h"
 #include "partitioning/partbounds.h"
 #include "rewrite/rewriteManip.h"
+#include "utils/builtins.h"
 #include "utils/fmgroids.h"
+#include "utils/lsyscache.h"
 #include "utils/partcache.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
@@ -357,3 +361,89 @@ get_proposed_default_constraint(List *new_part_constraints)
 
return make_ands_implicit(defPartConstraint);
 }
+
+Datum
+pg_partition_children(PG_FUNCTION_ARGS)
+{
+   Oid rootrelid = PG_GETARG_OID(0);
+   FuncCallContext *funccxt;
+   ListCell **next;
+
+   if (SRF_IS_FIRSTCALL())
+   {
+   MemoryContext oldcxt;
+   TupleDesc   tupdesc;
+   List   *partitions;
+
+   funccxt = SRF_FIRSTCALL_INIT();
+   oldcxt = MemoryContextSwitchTo(funccxt->multi_call_memory_ctx);
+
+   partitions = find_all_inheritors(rootrelid, NoLock, NULL);
+
+   tupdesc = CreateTemplateTupleDesc(4, false);
+   TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relid",
+  REGCLASSOID, -1, 0);
+   TupleDescInitEntry(tupdesc, (AttrNumber) 2, "parentid",
+  REGCLASSOID, -1, 0);
+   TupleDescInitEntry(tupdesc, (AttrNumber) 3, "level",
+  INT4OID, -1, 0);
+   TupleDescInitEntry(tupdesc, (AttrNumber) 4, "isleaf",
+  BOOLOID, -1, 0);
+
+   next = (ListCell **) palloc(sizeof(ListCell *));
+   *next = list_head(partitions);
+
+   funccxt->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+   funccxt->user_fctx = (void *) next;

Re: partition tree inspection functions

[Thomas Munro]

On Wed, Aug 8, 2018 at 11:21 PM, Thomas Munro
 wrote:
> On Tue, Aug 7, 2018 at 7:32 PM, Amit Langote
>  wrote:
>> Attached updated patch adds isleaf to pg_partition_children's output.
>
> Hmm, I wonder where this garbage is coming from:

partition.c:437:3: error: array index 3 is past the end of the array
(which contains 3 elements) [-Werror,-Warray-bounds]

That'll do it.  Also:

partition.c:409:19: error: implicit declaration of function
'get_rel_relkind' is invalid in C99
[-Werror,-Wimplicit-function-declaration]

-- 
Thomas Munro
http://www.enterprisedb.com

Re: partition tree inspection functions

[Thomas Munro]

On Tue, Aug 7, 2018 at 7:32 PM, Amit Langote
 wrote:
> Attached updated patch adds isleaf to pg_partition_children's output.

Hi Amit,

Hmm, I wonder where this garbage is coming from:

6201   select *, pg_relation_size(relid) as size from
pg_partition_children('ptif_test');
6202 ! ERROR:  invalid byte sequence for encoding "UTF8": 0x8b

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.8126

-- 
Thomas Munro
http://www.enterprisedb.com

Re: partition tree inspection functions

[Jesper Pedersen]

Hi,

On 08/07/2018 03:32 AM, Amit Langote wrote:

Do we need a pg_partition_level that expects the individual partition OID
to be passed to it or can we do with the information we get from the
revised pg_partition_children?  In earlier revisions,
pg_partition_children returned only the partition OIDs, so we needed to
provide pg_partition_* functions for getting the parent, root parent,
level, etc. separately.  I mean to ask if is there a need for having these
functions separately if the revised pg_partition_children already outputs
that information?



I'm thinking of the case where we only have information about a leaf 
partition, and we need its root partition and the actual level in the 
partition tree.


If we had

 SELECT pg_partition_root_parent('leafpart');

and

 SELECT pg_partition_level('leafpart');

-- we don't need the pg_partition_level('leafpart', 'parentpart') 
function now.


We can use pg_partition_children() for the rest.


pg_partition_leaf_children()'s output can be obtained as follows, after
adding isleaf column to pg_partition_children's output:



Yes, this is great.

Thanks !

Best regards,
 Jesper

Re: partition tree inspection functions

[Amit Langote]

Hi,

On 2018/08/03 21:35, Jesper Pedersen wrote:
> Hi Amit,
> 
> On 08/03/2018 04:28 AM, Amit Langote wrote:
>> That's a good idea, thanks.
>>
>> Actually, by the time I sent the last version of the patch or maybe few
>> versions before that, I too had started thinking if we shouldn't just have
>> a SETOF RECORD function like you've outlined here, but wasn't sure of the
>> fields it should have.  (relid, parentid, level) seems like a good start,
>> or maybe that's just what we need.
>>
> 
> I think there should be a column that identifies leaf partitions (bool
> isleaf), otherwise it isn't obvious in complex scenarios.

Ah, getting isleaf directly from pg_partition_children would be better
than an application figuring that out by itself.

>> Note that the level that's returned for each table is computed wrt the
>> root table passed to the function and not the actual root partition.
> 
> If you are given a leaf partition as input, then you will have to keep
> executing the query until you find the root, and count those. So, I think
> it should be either be the level to the root, or there should be another
> column that lists that (rootlevel).

The function pg_partition_children is to get partitions found under a
given root table.  If you pass a leaf partition to it, then there is
nothing under, just the leaf partition itself, and its level wrt itself is
0.  That's what Robert said too, to which you replied:

On 2018/08/03 22:11, Jesper Pedersen wrote:
> We had the 2 pg_partition_level() functions and
> pg_partition_leaf_children() in v8, so it would be good to get those back.

Do we need a pg_partition_level that expects the individual partition OID
to be passed to it or can we do with the information we get from the
revised pg_partition_children?  In earlier revisions,
pg_partition_children returned only the partition OIDs, so we needed to
provide pg_partition_* functions for getting the parent, root parent,
level, etc. separately.  I mean to ask if is there a need for having these
functions separately if the revised pg_partition_children already outputs
that information?

pg_partition_leaf_children()'s output can be obtained as follows, after
adding isleaf column to pg_partition_children's output:

select * from pg_partition_children('') where isleaf;


Attached updated patch adds isleaf to pg_partition_children's output.

Thanks,
Amit
From a5c9388b984ed7e70d3ed074c9ef3eb6de975d4a Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 3 Aug 2018 17:06:05 +0900
Subject: [PATCH v10] Add pg_partition_children to report partitions

It returns set of records one for each partition containing the
partition name, parent name, and level in the partition tree with
given table as root
---
 doc/src/sgml/func.sgml   | 37 
 src/backend/catalog/partition.c  | 89 
 src/include/catalog/pg_proc.dat  |  9 +++
 src/test/regress/expected/partition_info.out | 84 ++
 src/test/regress/parallel_schedule   |  2 +-
 src/test/regress/serial_schedule |  1 +
 src/test/regress/sql/partition_info.sql  | 35 +++
 7 files changed, 256 insertions(+), 1 deletion(-)
 create mode 100644 src/test/regress/expected/partition_info.out
 create mode 100644 src/test/regress/sql/partition_info.sql

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index edc9be92a6..6b10aa3b3d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19995,6 +19995,43 @@ postgres=# SELECT * FROM 
pg_walfile_name_offset(pg_stop_backup());
 The function returns the number of new collation objects it created.

 
+   
+Partitioning Information Functions
+
+ 
+  Name Return Type 
Description
+ 
+
+  
+  
+   
pg_partition_children(regclass)
+   setof record
+   
+List name, parent name, level, and whether it's a leaf partition for
+each partition contained in the partition tree with given root table,
+including the root table itself
+   
+  
+ 
+
+   
+
+   
+To check the total size of the data contained in
+measurement table described in
+, one could use the
+following query:
+   
+
+
+select pg_size_pretty(sum(pg_relation_size(relid))) as total_size from 
pg_partition_children('measurement');
+ total_size 
+
+ 24 kB
+(1 row)
+
+
+
   
 
   
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 558022647c..5c919a5bee 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -23,12 +23,15 @@
 #include "catalog/partition.h"
 #include "catalog/pg_inherits.h"
 #include "catalog/pg_partitioned_table.h"
+#include "catalog/pg_type.h"
+#include "funcapi.h"
 #include "nodes/makefuncs.h"
 #include "optimizer/clauses.h"
 #include "optimizer/prep.h"
 #include "optimizer/var.h"
 #include "partitioning/partbounds.h"
 #include "rewrite/rewriteManip.h"
+#include 

Re: partition tree inspection functions

[Jesper Pedersen]

Hi,

On 08/03/2018 08:59 AM, Robert Haas wrote:

On Fri, Aug 3, 2018 at 8:35 AM, Jesper Pedersen
 wrote:

If you are given a leaf partition as input, then you will have to keep
executing the query until you find the root, and count those. So, I think it
should be either be the level to the root, or there should be another column
that lists that (rootlevel).


I disagree.  I think Amit has got the right semantics -- it gives you
everything rooted at the partition you name, relative to that root.
We could have another function which, given the OID of a partition,
returns the topmost parent (or the immediate parent), but I think that
if you say "tell me all the partitions of X", it should just tell you
about stuff that's under X, regardless of what's over X.



We had the 2 pg_partition_level() functions and 
pg_partition_leaf_children() in v8, so it would be good to get those back.


Best regards,
 Jesper

Re: partition tree inspection functions

[Robert Haas]

On Fri, Aug 3, 2018 at 8:35 AM, Jesper Pedersen
 wrote:
> If you are given a leaf partition as input, then you will have to keep
> executing the query until you find the root, and count those. So, I think it
> should be either be the level to the root, or there should be another column
> that lists that (rootlevel).

I disagree.  I think Amit has got the right semantics -- it gives you
everything rooted at the partition you name, relative to that root.
We could have another function which, given the OID of a partition,
returns the topmost parent (or the immediate parent), but I think that
if you say "tell me all the partitions of X", it should just tell you
about stuff that's under X, regardless of what's over X.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Amit,

On 08/03/2018 04:28 AM, Amit Langote wrote:

That's a good idea, thanks.

Actually, by the time I sent the last version of the patch or maybe few
versions before that, I too had started thinking if we shouldn't just have
a SETOF RECORD function like you've outlined here, but wasn't sure of the
fields it should have.  (relid, parentid, level) seems like a good start,
or maybe that's just what we need.



I think there should be a column that identifies leaf partitions (bool 
isleaf), otherwise it isn't obvious in complex scenarios.




Note that the level that's returned for each table is computed wrt the
root table passed to the function and not the actual root partition.



If you are given a leaf partition as input, then you will have to keep 
executing the query until you find the root, and count those. So, I 
think it should be either be the level to the root, or there should be 
another column that lists that (rootlevel).


Best regards,
 Jesper

Re: partition tree inspection functions

[Amit Langote]

On 2018/08/01 22:21, Robert Haas wrote:
> On Thu, Jul 26, 2018 at 4:47 AM, Amit Langote
>  wrote:
>> Alright, I have replaced pg_partition_tree_tables with
>> pg_partition_children with an 'include_all' argument, as you suggested,
>> but I implemented it as an optional argument.  So, one would use that
>> argument only if need to get *all* partitions.  I have also added a
>> pg_partition_leaf_children() that returns just the leaf partitions, which
>> wasn't there in the previous versions.
>>
>> Further, I've added a pg_partition_level that returns the level of a
>> partition in the partition tree wrt to the root of the *whole* partition
>> tree.  But maybe we want this function to accept one more argument,
>> 'rootoid', the OID of the root table against which to measure the level?
> 
> I have another idea.  Suppose we just have one function, and that
> function a set of records, and each record contains (1) the OID of a
> table, (2) the OID of the immediate parent or NULL for the root, and
> (3) the level (0 = root, 1 = child, 2 = grandchild, etc.).
> 
> So then to get the immediate children you would say:
> 
> SELECT * FROM pg_whatever() WHERE level = 1
> 
> And to get everything you would just say:
> 
> SELECT * FROM pg_whatever();
> 
> And if you wanted grandchildren or everything but the root or whatever
> you could just adjust the WHERE clause.
> 
> By including the OID of the immediate parent, there's enough
> information for application code to draw an actual graph if it wants,
> which doesn't work so well if you just know the levels.

That's a good idea, thanks.

Actually, by the time I sent the last version of the patch or maybe few
versions before that, I too had started thinking if we shouldn't just have
a SETOF RECORD function like you've outlined here, but wasn't sure of the
fields it should have.  (relid, parentid, level) seems like a good start,
or maybe that's just what we need.

I tried to implement such a function.  Example usage:

create table q (a int, b int, c int) partition by list (a);
create table q1 partition of q for values in (1) partition by hash (b);
create table q11 partition of q1 for values with (modulus 1, remainder 0)
partition by hash (c);
create table q111 partition of q11 for values with (modulus 1, remainder 0);
create table q2 partition of q for values in (2);
insert into q select i%2+1, i, i from generate_series(1, 1000) i;

select * from pg_partition_children('q');
 relid │ parentid │ level
───┼──┼───
 q │  │ 0
 q1│ q│ 1
 q2│ q│ 1
 q11   │ q1   │ 2
 q111  │ q11  │ 3
(5 rows)

select * from pg_partition_children('q') where level > 0;
 relid │ parentid │ level
───┼──┼───
 q1│ q│ 1
 q2│ q│ 1
 q11   │ q1   │ 2
 q111  │ q11  │ 3
(4 rows)

select * from pg_partition_children('q') where level = 1;
 relid │ parentid │ level
───┼──┼───
 q1│ q│ 1
 q2│ q│ 1
(2 rows)

select *, pg_relation_size(relid) as size from pg_partition_children('q');
 relid │ parentid │ level │ size
───┼──┼───┼───
 q │  │ 0 │ 0
 q1│ q│ 1 │ 0
 q2│ q│ 1 │ 24576
 q11   │ q1   │ 2 │ 0
 q111  │ q11  │ 3 │ 24576
(5 rows)

select sum(pg_relation_size(relid)) as size from pg_partition_children('q');
 size
───
 49152
(1 row)

select *, pg_relation_size(relid) as size from pg_partition_children('q1');
 relid │ parentid │ level │ size
───┼──┼───┼───
 q1│ q│ 0 │ 0
 q11   │ q1   │ 1 │ 0
 q111  │ q11  │ 2 │ 24576
(3 rows)

select *, pg_relation_size(relid) as size from pg_partition_children('q11');
 relid │ parentid │ level │ size
───┼──┼───┼───
 q11   │ q1   │ 0 │ 0
 q111  │ q11  │ 1 │ 24576
(2 rows)

select *, pg_relation_size(relid) as size from pg_partition_children('q111');
 relid │ parentid │ level │ size
───┼──┼───┼───
 q111  │ q11  │ 0 │ 24576
(1 row)

Note that the level that's returned for each table is computed wrt the
root table passed to the function and not the actual root partition.

I have updated the patch to include just this one function, its
documentation, and tests.

Regards,
Amit
From 5dee35a24ea96b8bc62578f533dec00dcdc56476 Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 3 Aug 2018 17:06:05 +0900
Subject: [PATCH v9] Add pg_partition_children to report partitions

It returns set of records one for each partition containing the
partition name, parent name, and level in the partition tree with
given table as root
---
 doc/src/sgml/func.sgml   | 33 +++
 src/backend/catalog/partition.c  | 83 
 src/include/catalog/pg_proc.dat  |  7 +++
 src/test/regress/expected/partition_info.out | 75 

Re: partition tree inspection functions

[Robert Haas]

On Thu, Jul 26, 2018 at 4:47 AM, Amit Langote
 wrote:
> Alright, I have replaced pg_partition_tree_tables with
> pg_partition_children with an 'include_all' argument, as you suggested,
> but I implemented it as an optional argument.  So, one would use that
> argument only if need to get *all* partitions.  I have also added a
> pg_partition_leaf_children() that returns just the leaf partitions, which
> wasn't there in the previous versions.
>
> Further, I've added a pg_partition_level that returns the level of a
> partition in the partition tree wrt to the root of the *whole* partition
> tree.  But maybe we want this function to accept one more argument,
> 'rootoid', the OID of the root table against which to measure the level?

I have another idea.  Suppose we just have one function, and that
function a set of records, and each record contains (1) the OID of a
table, (2) the OID of the immediate parent or NULL for the root, and
(3) the level (0 = root, 1 = child, 2 = grandchild, etc.).

So then to get the immediate children you would say:

SELECT * FROM pg_whatever() WHERE level = 1

And to get everything you would just say:

SELECT * FROM pg_whatever();

And if you wanted grandchildren or everything but the root or whatever
you could just adjust the WHERE clause.

By including the OID of the immediate parent, there's enough
information for application code to draw an actual graph if it wants,
which doesn't work so well if you just know the levels.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: partition tree inspection functions

[Amit Langote]

arent',
+  proname => 'pg_partition_root_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_root_parent' },
+
+# function to get the partition parent
+{ oid => '3424', descr => 'oid of the partition immediate parent',
+  proname => 'pg_partition_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_parent' },
+
+# function to get the level of a partition in the partition tree of given root
+{ oid => '3425', descr => 'level of a partition in the partition tree for 
given root table',
+  proname => 'pg_partition_level', prorettype => 'int4',
+  proargtypes => 'regclass regclass', prosrc => 'pg_partition_level' },
+
+# function to get the level of a partition in the whole partition tree
+{ oid => '3426', descr => 'level of a partition in the partition tree',
+  proname => 'pg_partition_level', prolang => '14', prorettype => 'int4',
+  proargtypes => 'regclass',
+  prosrc => 'select pg_catalog.pg_partition_level($1, \'0\')' },
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3427', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_children', prorettype => 'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass bool',
+  proallargtypes => '{regclass,bool,regclass}',
+  proargmodes => '{i,i,o}',
+  proargnames => '{relid,include_all,relid}',
+  prosrc => 'pg_partition_children' }
+
+# function to get OIDs of immediate
+{ oid => '3428', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_children', prolang => '14', prorettype => 
'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}',
+  prosrc => 'select pg_catalog.pg_partition_children($1, \'false\')' }
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3429', descr => 'get OIDs of leaf partitions in a partition tree',
+  proname => 'pg_partition_leaf_children', prorettype => 'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}',
+  prosrc => 'pg_partition_leaf_children' }
+
 ]
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index e55ea4035b..d396d17ff1 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -126,6 +126,7 @@ extern char *get_rel_name(Oid relid);
 extern Oid get_rel_namespace(Oid relid);
 extern Oid get_rel_type_id(Oid relid);
 extern char get_rel_relkind(Oid relid);
+extern char get_rel_relispartition(Oid relid);
 extern Oid get_rel_tablespace(Oid relid);
 extern char get_rel_persistence(Oid relid);
 extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
diff --git a/src/test/regress/expected/partition_info.out 
b/src/test/regress/expected/partition_info.out
new file mode 100644
index 00..e93069913c
--- /dev/null
+++ b/src/test/regress/expected/partition_info.out
@@ -0,0 +1,204 @@
+--
+-- Tests to show partition tree inspection functions
+--
+create table ptif_test (a int, b int) partition by range (a);
+create table ptif_test0 partition of ptif_test for values from (minvalue) to 
(0) partition by list (b);
+create table ptif_test01 partition of ptif_test0 for values in (1);
+create table ptif_test1 partition of ptif_test for values from (0) to (100) 
partition by list (b);
+create table ptif_test11 partition of ptif_test1 for values in (1);
+create table ptif_test2 partition of ptif_test for values from (100) to 
(maxvalue);
+insert into ptif_test select i, 1 from generate_series(-500, 500) i;
+select pg_partition_parent('ptif_test0') as parent;
+  parent   
+---
+ ptif_test
+(1 row)
+
+select pg_partition_parent('ptif_test01') as parent;
+   parent   
+
+ ptif_test0
+(1 row)
+
+select pg_partition_root_parent('ptif_test01') as root_parent;
+ root_parent 
+-
+ ptif_test
+(1 row)
+
+-- pg_partition_level where partition level wrt whole-tree root is returned
+select pg_partition_level('ptif_test01') as level; -- 2
+ level 
+---
+ 2
+(1 row)
+
+select pg_partition_level('ptif_test0') as level;  -- 1
+ level 
+---
+ 1
+(1 row)
+
+select pg_partition_level('ptif_test') as level;   -- 0
+ level 
+---
+ 0
+(1 row)
+
+select pg_partition_level('ptif_test01', 'ptif_test') as level;-- 2
+ level 
+---
+ 2
+(1 row)
+
+select pg_partition_level('ptif_test0', 'ptif_test') as level; -- 1
+ level 
+---
+ 1
+(1 row)
+
+select pg_partition_level('ptif_test', 'ptif_test') as level;  -- 0
+ level 
+---
+ 0
+(1 row)
+
+select pg_partition_level('ptif_test01', '

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Amit,

On 07/30/2018 05:21 AM, Amit Langote wrote:

As 0 is a valid return value for root nodes I think we should use -1
instead for these cases.


Makes sense, changed to be that way.



Looks good, the documentation for pg_partition_level could be expanded 
to describe the -1 scenario.


New status: Ready for Committer

Best regards,
 Jesper

Re: partition tree inspection functions

[Amit Langote]

nction to get the root partition parent
+{ oid => '3423', descr => 'oid of the partition root parent',
+  proname => 'pg_partition_root_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_root_parent' },
+
+# function to get the partition parent
+{ oid => '3424', descr => 'oid of the partition immediate parent',
+  proname => 'pg_partition_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_parent' },
+
+# function to get the level of a partition in the partition tree of given root
+{ oid => '3425', descr => 'level of a partition in the partition tree for 
given root table',
+  proname => 'pg_partition_level', prorettype => 'int4',
+  proargtypes => 'regclass regclass', prosrc => 'pg_partition_level' },
+
+# function to get the level of a partition in the whole partition tree
+{ oid => '3426', descr => 'level of a partition in the partition tree',
+  proname => 'pg_partition_level', prolang => '14', prorettype => 'int4',
+  proargtypes => 'regclass',
+  prosrc => 'select pg_catalog.pg_partition_level($1, \'0\')' },
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3427', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_children', prorettype => 'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass bool',
+  proallargtypes => '{regclass,bool,regclass}',
+  proargmodes => '{i,i,o}',
+  proargnames => '{relid,include_all,relid}',
+  prosrc => 'pg_partition_children' }
+
+# function to get OIDs of immediate
+{ oid => '3428', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_children', prolang => '14', prorettype => 
'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}',
+  prosrc => 'select pg_catalog.pg_partition_children($1, \'false\')' }
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3429', descr => 'get OIDs of leaf partitions in a partition tree',
+  proname => 'pg_partition_leaf_children', prorettype => 'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}',
+  prosrc => 'pg_partition_leaf_children' }
+
 ]
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index e55ea4035b..d396d17ff1 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -126,6 +126,7 @@ extern char *get_rel_name(Oid relid);
 extern Oid get_rel_namespace(Oid relid);
 extern Oid get_rel_type_id(Oid relid);
 extern char get_rel_relkind(Oid relid);
+extern char get_rel_relispartition(Oid relid);
 extern Oid     get_rel_tablespace(Oid relid);
 extern char get_rel_persistence(Oid relid);
 extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
diff --git a/src/test/regress/expected/partition_info.out 
b/src/test/regress/expected/partition_info.out
new file mode 100644
index 00..e93069913c
--- /dev/null
+++ b/src/test/regress/expected/partition_info.out
@@ -0,0 +1,204 @@
+--
+-- Tests to show partition tree inspection functions
+--
+create table ptif_test (a int, b int) partition by range (a);
+create table ptif_test0 partition of ptif_test for values from (minvalue) to 
(0) partition by list (b);
+create table ptif_test01 partition of ptif_test0 for values in (1);
+create table ptif_test1 partition of ptif_test for values from (0) to (100) 
partition by list (b);
+create table ptif_test11 partition of ptif_test1 for values in (1);
+create table ptif_test2 partition of ptif_test for values from (100) to 
(maxvalue);
+insert into ptif_test select i, 1 from generate_series(-500, 500) i;
+select pg_partition_parent('ptif_test0') as parent;
+  parent   
+---
+ ptif_test
+(1 row)
+
+select pg_partition_parent('ptif_test01') as parent;
+   parent   
+
+ ptif_test0
+(1 row)
+
+select pg_partition_root_parent('ptif_test01') as root_parent;
+ root_parent 
+-
+ ptif_test
+(1 row)
+
+-- pg_partition_level where partition level wrt whole-tree root is returned
+select pg_partition_level('ptif_test01') as level; -- 2
+ level 
+---
+ 2
+(1 row)
+
+select pg_partition_level('ptif_test0') as level;  -- 1
+ level 
+---
+ 1
+(1 row)
+
+select pg_partition_level('ptif_test') as level;   -- 0
+ level 
+---
+ 0
+(1 row)
+
+select pg_partition_level('ptif_test01', 'ptif_test') as level;-- 2
+ level 
+---
+ 2
+(1 row)
+
+select pg_partition_level('ptif_test0', 'ptif_test') as level; -- 1
+ level 
+---
+ 1
+(1 row)
+
+select pg_partition_level('ptif_test', 'pt

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Amit,

On 07/26/2018 10:33 PM, Amit Langote wrote:

Optional parameter sounds good, so made it get_partition_level(regclass [
, regclass ]) in the updated patch.  Although, adding that argument is not
without possible surprises its result might evoke.  Like, what happens if
you try to find the level of the root table by passing a leaf partition
oid for the root table argument, or pass a totally unrelated table for the
root table argument.  For now, I've made the function return 0 for such cases.



As 0 is a valid return value for root nodes I think we should use -1 
instead for these cases.


Otherwise looks good.

Best regards,
 Jesper

Re: partition tree inspection functions

[Amit Langote]

   result = reltup->relispartition;
+   ReleaseSysCache(tp);
+
+   return result;
+}
+
+/*
  * get_rel_tablespace
  *
  * Returns the pg_tablespace OID associated with a given relation.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index a14651010f..e1d190f81a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10206,4 +10206,52 @@
   proisstrict => 'f', prorettype => 'bool', proargtypes => 'oid int4 int4 any',
   proargmodes => '{i,i,i,v}', prosrc => 'satisfies_hash_partition' },
 
+# function to get the root partition parent
+{ oid => '3423', descr => 'oid of the partition root parent',
+  proname => 'pg_partition_root_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_root_parent' },
+
+# function to get the partition parent
+{ oid => '3424', descr => 'oid of the partition immediate parent',
+  proname => 'pg_partition_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_parent' },
+
+# function to get the level of a partition in the partition tree of given root
+{ oid => '3425', descr => 'level of a partition in the partition tree for 
given root table',
+  proname => 'pg_partition_level', prorettype => 'int4',
+  proargtypes => 'regclass regclass', prosrc => 'pg_partition_level' },
+
+# function to get the level of a partition in the whole partition tree
+{ oid => '3426', descr => 'level of a partition in the partition tree',
+  proname => 'pg_partition_level', prolang => '14', prorettype => 'int4',
+  proargtypes => 'regclass',
+  prosrc => 'select pg_catalog.pg_partition_level($1, \'0\')' },
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3427', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_children', prorettype => 'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass bool',
+  proallargtypes => '{regclass,bool,regclass}',
+  proargmodes => '{i,i,o}',
+  proargnames => '{relid,include_all,relid}',
+  prosrc => 'pg_partition_children' }
+
+# function to get OIDs of immediate
+{ oid => '3428', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_children', prolang => '14', prorettype => 
'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}',
+  prosrc => 'select pg_catalog.pg_partition_children($1, \'false\')' }
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3429', descr => 'get OIDs of leaf partitions in a partition tree',
+  proname => 'pg_partition_leaf_children', prorettype => 'regclass',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}',
+  prosrc => 'pg_partition_leaf_children' }
+
 ]
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index e55ea4035b..d396d17ff1 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -126,6 +126,7 @@ extern char *get_rel_name(Oid relid);
 extern Oid get_rel_namespace(Oid relid);
 extern Oid get_rel_type_id(Oid relid);
 extern char get_rel_relkind(Oid relid);
+extern char get_rel_relispartition(Oid relid);
 extern Oid get_rel_tablespace(Oid relid);
 extern char get_rel_persistence(Oid relid);
 extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
diff --git a/src/test/regress/expected/partition_info.out 
b/src/test/regress/expected/partition_info.out
new file mode 100644
index 00..d34f3031c9
--- /dev/null
+++ b/src/test/regress/expected/partition_info.out
@@ -0,0 +1,204 @@
+--
+-- Tests to show partition tree inspection functions
+--
+create table ptif_test (a int, b int) partition by range (a);
+create table ptif_test0 partition of ptif_test for values from (minvalue) to 
(0) partition by list (b);
+create table ptif_test01 partition of ptif_test0 for values in (1);
+create table ptif_test1 partition of ptif_test for values from (0) to (100) 
partition by list (b);
+create table ptif_test11 partition of ptif_test1 for values in (1);
+create table ptif_test2 partition of ptif_test for values from (100) to 
(maxvalue);
+insert into ptif_test select i, 1 from generate_series(-500, 500) i;
+select pg_partition_parent('ptif_test0') as parent;
+  parent   
+---
+ ptif_test
+(1 row)
+
+select pg_partition_parent('ptif_test01') as parent;
+   parent   
+
+ ptif_test0
+(1 row)
+
+select pg_partition_root_parent('ptif_test01') as root_parent;
+ root_parent 
+-
+ ptif_test
+(1 row)
+
+-- pg_partition_level whe

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Amit,

On 07/26/2018 04:47 AM, Amit Langote wrote:

Alright, I have replaced pg_partition_tree_tables with
pg_partition_children with an 'include_all' argument, as you suggested,
but I implemented it as an optional argument.  So, one would use that
argument only if need to get *all* partitions.  I have also added a
pg_partition_leaf_children() that returns just the leaf partitions, which
wasn't there in the previous versions.



Great.


Further, I've added a pg_partition_level that returns the level of a
partition in the partition tree wrt to the root of the *whole* partition
tree.  But maybe we want this function to accept one more argument,
'rootoid', the OID of the root table against which to measure the level?



I don't think that is needed, or it should at least be an optional 
parameter.



Maybe call it pg_partition_tree_leaf_count() or some such then?


That could work.


OK, I fixed it to return just the count of leaf partitions and renamed it
as such (pg_partition_children_leaf_count), but wonder if it's been made
redundant by the addition of pg_partition_leaf_children.



I think with pg_partition_leaf_children that we don't need the _count 
method, called pg_partition_tree_leaf_count in the docs, as we can just 
do a COUNT().


Best regards,
 Jesper

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Amit,

On 07/19/2018 10:27 PM, Amit Langote wrote:

On 2018/07/19 23:18, Jesper Pedersen wrote:

I'm thinking about how to best use these functions to generate a graph
that represents the partition hierarchy.

What about renaming pg_partition_tree_tables() to pg_partition_children(),
and have it work like

select * from pg_partition_children('p', true);
-
  p
  p0
  p1
  p00
  p01
  p10
  p11
(7 rows)

select * from pg_partition_children('p', false);
-
  p0
  p1
(2 rows)

e.g. if 'bool include_all' is true all nodes under the node, including
itself, are fetched. With false only nodes directly under the node,
excluding itself, are returned. If there are no children NULL is returned.


That's a big change to make to what this function does, but if that's
what's useful we could make it.  As an alternative, wouldn't it help to
implement the idea that Dilip mentioned upthread of providing a function
to report the level of a given table in the partition hierarchy -- 0 for
root, 1 for its partitions and so on?



Yes, Dilip's idea could work. I just don't think that 
pg_partition_tree_tables() as is would have a benefit over time.



Basically, as also discussed before, users can already use SQL to get the
information they want out of the relevant catalogs (pg_inherits, etc.).
But, such user queries might not be very future-proof as we might want to
change the catalog organization in the future, so we'd like to provide
users a proper interface to begin with.  Keeping that in mind, it'd be
better to think carefully about what we ought to be doing here.  Input
like yours is greatly helpful for that.



We could have the patch include pg_partition_root_parent and 
pg_partition_parent, and leave the rest for a future CommitFest such 
that more people could provide feedback on what they would like to see 
in this space.



Maybe a function like pg_partition_number_of_partitions() could be of
benefit to count the number of actual partitions in a tree. Especially
useful in complex scenarios,

   select pg_partition_number_of_partitions('p') as number;

     number
   -
    4
   (1 row)


Okay, adding one more function at this point may not be asking for too
much.  Although, select count(*) from pg_partition_tree_tables('p') would
give you the count, a special function seems nice.


Yeah, but I was thinking that the function would only return the number of
actual tables that contains data, e.g. not include 'p', 'p0' and 'p1' in
the count; otherwise you could use 'select count(*) from
pg_partition_children('p', true)' like you said.


Maybe call it pg_partition_tree_leaf_count() or some such then?



That could work.

Best regards,
 Jesper

Re: partition tree inspection functions

[Amit Langote]

Hi Jesper,

On 2018/07/19 23:18, Jesper Pedersen wrote:
> I'm thinking about how to best use these functions to generate a graph
> that represents the partition hierarchy.
> 
> What about renaming pg_partition_tree_tables() to pg_partition_children(),
> and have it work like
> 
> select * from pg_partition_children('p', true);
> -
>  p
>  p0
>  p1
>  p00
>  p01
>  p10
>  p11
> (7 rows)
> 
> select * from pg_partition_children('p', false);
> -
>  p0
>  p1
> (2 rows)
> 
> e.g. if 'bool include_all' is true all nodes under the node, including
> itself, are fetched. With false only nodes directly under the node,
> excluding itself, are returned. If there are no children NULL is returned.

That's a big change to make to what this function does, but if that's
what's useful we could make it.  As an alternative, wouldn't it help to
implement the idea that Dilip mentioned upthread of providing a function
to report the level of a given table in the partition hierarchy -- 0 for
root, 1 for its partitions and so on?

Basically, as also discussed before, users can already use SQL to get the
information they want out of the relevant catalogs (pg_inherits, etc.).
But, such user queries might not be very future-proof as we might want to
change the catalog organization in the future, so we'd like to provide
users a proper interface to begin with.  Keeping that in mind, it'd be
better to think carefully about what we ought to be doing here.  Input
like yours is greatly helpful for that.

>>> Maybe a function like pg_partition_number_of_partitions() could be of
>>> benefit to count the number of actual partitions in a tree. Especially
>>> useful in complex scenarios,
>>>
>>>   select pg_partition_number_of_partitions('p') as number;
>>>
>>>     number
>>>   -
>>>    4
>>>   (1 row)
>>
>> Okay, adding one more function at this point may not be asking for too
>> much.  Although, select count(*) from pg_partition_tree_tables('p') would
>> give you the count, a special function seems nice.
> 
> Yeah, but I was thinking that the function would only return the number of
> actual tables that contains data, e.g. not include 'p', 'p0' and 'p1' in
> the count; otherwise you could use 'select count(*) from
> pg_partition_children('p', true)' like you said.

Maybe call it pg_partition_tree_leaf_count() or some such then?

Thanks,
Amit

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Amit,

On 07/19/2018 04:39 AM, Amit Langote wrote:

I think pg_partition_tree_tables should have an option to exclude the
table that is being queried from the result (bool include_self).


Doesn't sound too bad, so added include_self.



I'm thinking about how to best use these functions to generate a graph 
that represents the partition hierarchy.


What about renaming pg_partition_tree_tables() to 
pg_partition_children(), and have it work like


select * from pg_partition_children('p', true);
-
 p
 p0
 p1
 p00
 p01
 p10
 p11
(7 rows)

select * from pg_partition_children('p', false);
-
 p0
 p1
(2 rows)

e.g. if 'bool include_all' is true all nodes under the node, including 
itself, are fetched. With false only nodes directly under the node, 
excluding itself, are returned. If there are no children NULL is returned.



Maybe a function like pg_partition_number_of_partitions() could be of
benefit to count the number of actual partitions in a tree. Especially
useful in complex scenarios,

  select pg_partition_number_of_partitions('p') as number;

    number
  -
   4
  (1 row)


Okay, adding one more function at this point may not be asking for too
much.  Although, select count(*) from pg_partition_tree_tables('p') would
give you the count, a special function seems nice.



Yeah, but I was thinking that the function would only return the number 
of actual tables that contains data, e.g. not include 'p', 'p0' and 'p1' 
in the count; otherwise you could use 'select count(*) from 
pg_partition_children('p', true)' like you said.


Thanks for considering.

Best regards,
 Jesper

Re: partition tree inspection functions

[Amit Langote]

+   funcctx->user_fctx = (void *) lc;
+
+   MemoryContextSwitchTo(oldcontext);
+   }
+
+   /* stuff done on every call of the function */
+   funcctx = SRF_PERCALL_SETUP();
+   lc = (ListCell **) funcctx->user_fctx;
+
+   while (*lc != NULL)
+   {
+   Oid partoid = lfirst_oid(*lc);
+
+   *lc = lnext(*lc);
+   SRF_RETURN_NEXT(funcctx, ObjectIdGetDatum(partoid));
+   }
+
+   SRF_RETURN_DONE(funcctx);
+}
+
+/*
+ * Returns number of tables in a partition tree
+ */
+Datum
+pg_partition_tree_number_of_tables(PG_FUNCTION_ARGS)
+{
+   Oid reloid = PG_GETARG_OID(0);
+   List   *partitions = find_all_inheritors(reloid, NoLock, NULL);
+   int result = list_length(partitions);
+
+   list_free(partitions);
+   PG_RETURN_INT32(result);
+}
diff --git a/src/backend/utils/cache/lsyscache.c 
b/src/backend/utils/cache/lsyscache.c
index bba595ad1d..19262c6c4d 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -1821,6 +1821,28 @@ get_rel_relkind(Oid relid)
 }
 
 /*
+ * get_rel_relispartition
+ *
+ * Returns the value of pg_class.relispartition for a given 
relation.
+ */
+char
+get_rel_relispartition(Oid relid)
+{
+   HeapTuple   tp;
+   Form_pg_class reltup;
+   boolresult;
+
+   tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+   if (!HeapTupleIsValid(tp))
+   elog(ERROR, "cache lookup failed for relation %u", relid);
+   reltup = (Form_pg_class) GETSTRUCT(tp);
+   result = reltup->relispartition;
+   ReleaseSysCache(tp);
+
+   return result;
+}
+
+/*
  * get_rel_tablespace
  *
  * Returns the pg_tablespace OID associated with a given relation.
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index 1f49e5d3a9..a2b11f40bc 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -32,6 +32,7 @@ typedef struct PartitionDescData
 
 extern Oid get_partition_parent(Oid relid);
 extern List *get_partition_ancestors(Oid relid);
+extern Oid get_partition_root_parent(Oid relid);
 extern List *map_partition_varattnos(List *expr, int fromrel_varno,
Relation to_rel, Relation 
from_rel,
bool *found_whole_row);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index a14651010f..5835a9aaf2 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10206,4 +10206,28 @@
   proisstrict => 'f', prorettype => 'bool', proargtypes => 'oid int4 int4 any',
   proargmodes => '{i,i,i,v}', prosrc => 'satisfies_hash_partition' },
 
+# function to get the root partition parent
+{ oid => '3423', descr => 'oid of the partition root parent',
+  proname => 'pg_partition_root_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_root_parent' },
+
+# function to get the partition parent
+{ oid => '3424', descr => 'oid of the partition immediate parent',
+  proname => 'pg_partition_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_parent' },
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3425', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_tree_tables', prorettype => '2205',
+  prorows => '100', proretset => 't', proargtypes => 'regclass bool',
+  proallargtypes => '{regclass,bool,regclass}',
+  proargmodes => '{i,i,o}',
+  proargnames => '{relid,include_self,relid}',
+  prosrc => 'pg_partition_tree_tables' }
+
+# function to get the number of tables in a given partition tree
+{ oid => '3426', descr => 'number of tables in the partition tree',
+  proname => 'pg_partition_tree_number_of_tables', prorettype => 'int4',
+  proargtypes => 'regclass', prosrc => 'pg_partition_tree_number_of_tables' },
+
 ]
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index e55ea4035b..d396d17ff1 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -126,6 +126,7 @@ extern char *get_rel_name(Oid relid);
 extern Oid get_rel_namespace(Oid relid);
 extern Oid get_rel_type_id(Oid relid);
 extern char get_rel_relkind(Oid relid);
+extern char get_rel_relispartition(Oid relid);
 extern Oid get_rel_tablespace(Oid relid);
 extern char get_rel_persistence(Oid relid);
 extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
diff --git a/src/test/regress/expected/partition_info.out 
b/src/test/regress/expected/partition_info.out
new file mode 100644
index 00..745b9fcbb5
--- /dev/null
+++ b/src/test/regress/expected/partition_info.out
@@ -0,0 +1,88 @@
+--
+--

Re: partition tree inspection functions

[Amit Langote]

Hi Dilip,

Sorry it took me a while to reply.

On 2018/06/29 14:30, Dilip Kumar wrote:
> On Tue, Jun 26, 2018 at 10:38 AM, Amit Langote wrote:
>> As discussed a little while back [1] and also recently mentioned [2], here
>> is a patch that adds a set of functions to inspect the details of a
>> partition tree.  There are three functions:
>>
>> pg_partition_parent(regclass) returns regclass
>> pg_partition_root_parent(regclass) returns regclass
>> pg_partition_tree_tables(regclass) returns setof regclass
>>
>>
>> selectp as relname,
>>   pg_partition_parent(p) as parent,
>>   pg_partition_root_parent(p) as root_parent
>> from  pg_partition_tree_tables('p') p;
>>  relname | parent | root_parent
>> -++-
>>  p   || p
>>  p0  | p  | p
>>  p1  | p  | p
>>  p00 | p0 | p
>>  p01 | p0 | p
>>  p10 | p1 | p
>>  p11 | p1 | p
>> (7 rows)
>>
> 
> Is it a good idea to provide a function or an option which can provide
> partitions detail in hierarchical order?
> 
> i.e
> relname level
> p 0
> p0   1
> p00 2
> p01 2
> p1   1

Yeah, might be a good idea.  We could have a function
pg_partition_tree_level(OID) which will return the level of the table
that's passed to it the way you wrote above, meaning 0 for the root
parent, 1 for the root's immediate partitions, 2 for their partitions, and
so on.

Thanks,
Amit

Re: partition tree inspection functions

[Jesper Pedersen]

Hi Amit,

On 06/28/2018 01:49 AM, Amit Langote wrote:

OK, I've added an example below the table of functions added by the patch.

Attached updated patch.



You forgot to remove the test output in create_table.out, so check-world 
is failing.


In pg_partition_parent

+   else
+   /* Not a partition, return NULL. */
+   PG_RETURN_NULL();

I would just remove the "else" such that PG_RETURN_NULL() is fall-through.

I think pg_partition_tree_tables should have an option to exclude the 
table that is being queried from the result (bool include_self).


Maybe a function like pg_partition_number_of_partitions() could be of 
benefit to count the number of actual partitions in a tree. Especially 
useful in complex scenarios,


 select pg_partition_number_of_partitions('p') as number;

   number
 -
  4
 (1 row)

New status: WfA

Best regards,
 Jesper

Re: partition tree inspection functions

[Dilip Kumar]

On Tue, Jun 26, 2018 at 10:38 AM, Amit Langote
 wrote:
> Hi.
>
> As discussed a little while back [1] and also recently mentioned [2], here
> is a patch that adds a set of functions to inspect the details of a
> partition tree.  There are three functions:
>
> pg_partition_parent(regclass) returns regclass
> pg_partition_root_parent(regclass) returns regclass
> pg_partition_tree_tables(regclass) returns setof regclass
>
>
> selectp as relname,
>   pg_partition_parent(p) as parent,
>   pg_partition_root_parent(p) as root_parent
> from  pg_partition_tree_tables('p') p;
>  relname | parent | root_parent
> -++-
>  p   || p
>  p0  | p  | p
>  p1  | p  | p
>  p00 | p0 | p
>  p01 | p0 | p
>  p10 | p1 | p
>  p11 | p1 | p
> (7 rows)
>

Is it a good idea to provide a function or an option which can provide
partitions detail in hierarchical order?

i.e
relname level
p 0
p0   1
p00 2
p01 2
p1   1



-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Re: partition tree inspection functions

[Amit Langote]

On 2018/06/28 22:01, Ashutosh Bapat wrote:
> On Thu, Jun 28, 2018 at 11:19 AM, Amit Langote
>  wrote:
>>>
>>> It would have imagined that just creating a new file, say
>>> partition_desc.sql or similar is nicer.
>>
>> How about partition_info.sql because they're testing partitioning
>> information functions?  partition_desc reminded me of PartitionDesc, an
>> internal structure used in the partitioning codem which made me a bit
>> uncomfortable.
> 
> I think we should just add calls to these functions/views wherever we
> are creating/altering or deleting objects to test a partition tree. I
> serves two purposes, testing the objects created/modified and testing
> the functions. Adding a new test file means we have to craft new
> objects, which are sometimes readily available in some other test
> files. Of course, we might find that certain cases are not covered by
> existing tests, but then that also means that those cases are not
> covered by object modification/creation tests as well.

I think that makes sense.  I couldn't assess by looking at tests for
various functions listed on 9.25. System Information Functions whether
there is some established practice about adding tests for them and/or
about where to put them.

For this particular set of functions, insert.sql may be a good place as it
has many tests involving multi-level partitioned tables.

Thanks,
Amit

Re: partition tree inspection functions

[Peter Eisentraut]

On 6/28/18 13:30, Michael Paquier wrote:
> On Thu, Jun 28, 2018 at 12:37:14PM +0200, Peter Eisentraut wrote:
>> I'm thinking, an SQL query might be more efficient if you want to
>> qualify the query further.  For example, give me all tables in this tree
>> that match '2018'.  If you wrote your functions as SQL-language
>> functions, the optimizer could perhaps inline them and optimize them
>> further.
> 
> Are you thinking about SQL functions here?  Here is an example of query
> able to fetch an entire partition tree.
> WITH RECURSIVE partition_info
>   (relid,
>relname,
>relsize,
>relispartition,
>relkind) AS (
> SELECT oid AS relid,
>relname,
>pg_relation_size(oid) AS relsize,
>relispartition,
>relkind
> FROM pg_catalog.pg_class
>   WHERE relname = 'your_parent_table_name_here' AND
> relkind = 'p'
[...]

Yes, this kind of thing should be more efficient than building the
entire tree in a C function and then filtering it afterwards.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: partition tree inspection functions

[Ashutosh Bapat]

On Thu, Jun 28, 2018 at 11:19 AM, Amit Langote
 wrote:
>>
>> It would have imagined that just creating a new file, say
>> partition_desc.sql or similar is nicer.
>
> How about partition_info.sql because they're testing partitioning
> information functions?  partition_desc reminded me of PartitionDesc, an
> internal structure used in the partitioning codem which made me a bit
> uncomfortable.

I think we should just add calls to these functions/views wherever we
are creating/altering or deleting objects to test a partition tree. I
serves two purposes, testing the objects created/modified and testing
the functions. Adding a new test file means we have to craft new
objects, which are sometimes readily available in some other test
files. Of course, we might find that certain cases are not covered by
existing tests, but then that also means that those cases are not
covered by object modification/creation tests as well.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: partition tree inspection functions

[Michael Paquier]

On Thu, Jun 28, 2018 at 12:37:14PM +0200, Peter Eisentraut wrote:
> I'm thinking, an SQL query might be more efficient if you want to
> qualify the query further.  For example, give me all tables in this tree
> that match '2018'.  If you wrote your functions as SQL-language
> functions, the optimizer could perhaps inline them and optimize them
> further.

Are you thinking about SQL functions here?  Here is an example of query
able to fetch an entire partition tree.
WITH RECURSIVE partition_info
  (relid,
   relname,
   relsize,
   relispartition,
   relkind) AS (
SELECT oid AS relid,
   relname,
   pg_relation_size(oid) AS relsize,
   relispartition,
   relkind
FROM pg_catalog.pg_class
WHERE relname = 'your_parent_table_name_here' AND
  relkind = 'p'
  UNION ALL
SELECT
 c.oid AS relid,
 c.relname AS relname,
 pg_relation_size(c.oid) AS relsize,
 c.relispartition AS relispartition,
 c.relkind AS relkind
FROM partition_info AS p,
 pg_catalog.pg_inherits AS i,
 pg_catalog.pg_class AS c
WHERE p.relid = i.inhparent AND
 c.oid = i.inhrelid AND
 c.relispartition
  )
SELECT * FROM partition_info;

Getting the direct parent is immediate, and getting the top-most parent
would be rather similar to that.  Not much elegant in my opinion, but
that's mainly a matter of taste?
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Peter Eisentraut]

On 6/28/18 10:59, Amit Langote wrote:
> On 2018/06/28 17:40, Peter Eisentraut wrote:
>> On 6/26/18 07:08, Amit Langote wrote:
>>> As discussed a little while back [1] and also recently mentioned [2], here
>>> is a patch that adds a set of functions to inspect the details of a
>>> partition tree.  There are three functions:
>>>
>>> pg_partition_parent(regclass) returns regclass
>>> pg_partition_root_parent(regclass) returns regclass
>>> pg_partition_tree_tables(regclass) returns setof regclass
>>
>> Does this add anything over writing a recursive query on pg_inherits?
> 
> As far as the information output is concerned, it doesn't.

I'm thinking, an SQL query might be more efficient if you want to
qualify the query further.  For example, give me all tables in this tree
that match '2018'.  If you wrote your functions as SQL-language
functions, the optimizer could perhaps inline them and optimize them
further.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: partition tree inspection functions

[Amit Langote]

_INIT();
+
+   /* switch to memory context appropriate for multiple function 
calls */
+   oldcontext = 
MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+   partoids = find_all_inheritors(reloid, NoLock, NULL);
+   lc = (ListCell **) palloc(sizeof(ListCell *));
+   *lc = list_head(partoids);
+   funcctx->user_fctx = (void *) lc;
+
+   MemoryContextSwitchTo(oldcontext);
+   }
+
+   /* stuff done on every call of the function */
+   funcctx = SRF_PERCALL_SETUP();
+   lc = (ListCell **) funcctx->user_fctx;
+
+   while (*lc != NULL)
+   {
+   Oid partoid = lfirst_oid(*lc);
+
+   *lc = lnext(*lc);
+   SRF_RETURN_NEXT(funcctx, ObjectIdGetDatum(partoid));
+   }
+
+   SRF_RETURN_DONE(funcctx);
+}
diff --git a/src/backend/utils/cache/lsyscache.c 
b/src/backend/utils/cache/lsyscache.c
index bba595ad1d..19262c6c4d 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -1821,6 +1821,28 @@ get_rel_relkind(Oid relid)
 }
 
 /*
+ * get_rel_relispartition
+ *
+ * Returns the value of pg_class.relispartition for a given 
relation.
+ */
+char
+get_rel_relispartition(Oid relid)
+{
+   HeapTuple   tp;
+   Form_pg_class reltup;
+   boolresult;
+
+   tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+   if (!HeapTupleIsValid(tp))
+   elog(ERROR, "cache lookup failed for relation %u", relid);
+   reltup = (Form_pg_class) GETSTRUCT(tp);
+   result = reltup->relispartition;
+   ReleaseSysCache(tp);
+
+   return result;
+}
+
+/*
  * get_rel_tablespace
  *
  * Returns the pg_tablespace OID associated with a given relation.
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index 1f49e5d3a9..a2b11f40bc 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -32,6 +32,7 @@ typedef struct PartitionDescData
 
 extern Oid get_partition_parent(Oid relid);
 extern List *get_partition_ancestors(Oid relid);
+extern Oid get_partition_root_parent(Oid relid);
 extern List *map_partition_varattnos(List *expr, int fromrel_varno,
Relation to_rel, Relation 
from_rel,
bool *found_whole_row);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 40d54ed030..b4725b8634 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10206,4 +10206,22 @@
   proisstrict => 'f', prorettype => 'bool', proargtypes => 'oid int4 int4 any',
   proargmodes => '{i,i,i,v}', prosrc => 'satisfies_hash_partition' },
 
+# function to get the root partition parent
+{ oid => '3423', descr => 'oid of the partition root parent',
+  proname => 'pg_partition_root_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_root_parent' },
+
+# function to get the partition parent
+{ oid => '3424', descr => 'oid of the partition immediate parent',
+  proname => 'pg_partition_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_parent' },
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3425', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_tree_tables', prorettype => '2205',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}', prosrc => 'pg_partition_tree_tables' }
+
 ]
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index e55ea4035b..d396d17ff1 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -126,6 +126,7 @@ extern char *get_rel_name(Oid relid);
 extern Oid get_rel_namespace(Oid relid);
 extern Oid get_rel_type_id(Oid relid);
 extern char get_rel_relkind(Oid relid);
+extern char get_rel_relispartition(Oid relid);
 extern Oid get_rel_tablespace(Oid relid);
 extern char get_rel_persistence(Oid relid);
 extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
diff --git a/src/test/regress/expected/create_table.out 
b/src/test/regress/expected/create_table.out
index 672719e5d5..f702518d4e 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -909,3 +909,63 @@ ERROR:  cannot create a temporary relation as partition of 
permanent relation "p
 create temp table temp_part partition of temp_parted default; -- ok
 drop table perm_parted cascade;
 drop table temp_parted cascade;
+-- tests to show partition tree inspection functions
+create table ptif_test (a int, b int) partition by range (a);

Re: partition tree inspection functions

[Michael Paquier]

On Thu, Jun 28, 2018 at 11:50:13AM +0900, Amit Langote wrote:
> For now, I've added them to create_table.sql, but maybe that's not where
> they really belong.  Attached updated patch with tests.

It would have imagined that just creating a new file, say
partition_desc.sql or similar is nicer.

+   ancestors = get_partition_ancestors(relid);
+   result = llast_oid(ancestors);
+   list_free(ancestors);
Relying on the fact that the top-most parent should be the last one in
the list is brittle in my opinion.

What this patch proposes is:
- pg_partition_root_parent to get the top-most parent within a partition
tree for a partition.
- pg_partition_parent to get the direct parent for a partition.
- pg_partition_tree_tables to get a full list of all the children
underneath.

As the goal is to facilitate the life of users so as they don't have to
craft any WITH RECURSIVE, I think that we could live with that.

+   
+If the table passed to pg_partition_root_parent is not
+a partition, the same table is returned as the result.  Result of
+pg_partition_tree_tables also contains the table
+that's passed to it as the first row.
+   
Okay for that part as well.

I haven't yet looked at the code in details, but what you are proposing
here looks sound.  Could you think about adding an example in the docs
about how to use them?  Say for a measurement table here is a query to
get the full size a partition tree takes..  That's one idea.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

de/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10206,4 +10206,22 @@
   proisstrict => 'f', prorettype => 'bool', proargtypes => 'oid int4 int4 any',
   proargmodes => '{i,i,i,v}', prosrc => 'satisfies_hash_partition' },
 
+# function to get the root partition parent
+{ oid => '3423', descr => 'oid of the partition root parent',
+  proname => 'pg_partition_root_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_root_parent' },
+
+# function to get the partition parent
+{ oid => '3424', descr => 'oid of the partition immediate parent',
+  proname => 'pg_partition_parent', prorettype => 'regclass',
+  proargtypes => 'regclass', prosrc => 'pg_partition_parent' },
+
+# function to get OIDs of all tables in a given partition tree
+{ oid => '3425', descr => 'get OIDs of tables in a partition tree',
+  proname => 'pg_partition_tree_tables', prorettype => '2205',
+  prorows => '100', proretset => 't', proargtypes => 'regclass',
+  proallargtypes => '{regclass,regclass}',
+  proargmodes => '{i,o}',
+  proargnames => '{relid,relid}', prosrc => 'pg_partition_tree_tables' }
+
 ]
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index e55ea4035b..d396d17ff1 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -126,6 +126,7 @@ extern char *get_rel_name(Oid relid);
 extern Oid get_rel_namespace(Oid relid);
 extern Oid get_rel_type_id(Oid relid);
 extern char get_rel_relkind(Oid relid);
+extern char get_rel_relispartition(Oid relid);
 extern Oid get_rel_tablespace(Oid relid);
 extern char get_rel_persistence(Oid relid);
 extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
diff --git a/src/test/regress/expected/create_table.out 
b/src/test/regress/expected/create_table.out
index 672719e5d5..f702518d4e 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -909,3 +909,63 @@ ERROR:  cannot create a temporary relation as partition of 
permanent relation "p
 create temp table temp_part partition of temp_parted default; -- ok
 drop table perm_parted cascade;
 drop table temp_parted cascade;
+-- tests to show partition tree inspection functions
+create table ptif_test (a int, b int) partition by range (a);
+create table ptif_test0 partition of ptif_test for values from (minvalue) to 
(0) partition by list (b);
+create table ptif_test01 partition of ptif_test0 for values in (1);
+create table ptif_test1 partition of ptif_test for values from (0) to 
(maxvalue) partition by list (b);
+create table ptif_test11 partition of ptif_test1 for values in (1);
+insert into ptif_test select i, 1 from generate_series(-5, 5) i;
+select pg_partition_parent('ptif_test0') as parent;
+  parent   
+---
+ ptif_test
+(1 row)
+
+select pg_partition_parent('ptif_test01') as parent;
+   parent   
+
+ ptif_test0
+(1 row)
+
+select pg_partition_root_parent('ptif_test01') as root_parent;
+ root_parent 
+-
+ ptif_test
+(1 row)
+
+select p as relname,
+   pg_partition_parent(p) as parent,
+   pg_partition_root_parent(p) as root_parent
+from   pg_partition_tree_tables('ptif_test') p;
+   relname   |   parent   | root_parent 
+-++-
+ ptif_test   || ptif_test
+ ptif_test0  | ptif_test  | ptif_test
+ ptif_test1  | ptif_test  | ptif_test
+ ptif_test01 | ptif_test0 | ptif_test
+ ptif_test11 | ptif_test1 | ptif_test
+(5 rows)
+
+select p as relname,
+   pg_partition_parent(p) as parent,
+   pg_partition_root_parent(p) as root_parent,
+   pg_relation_size(p) as size
+from   pg_partition_tree_tables('ptif_test') p;
+   relname   |   parent   | root_parent | size 
+-++-+--
+ ptif_test   || ptif_test   |0
+ ptif_test0  | ptif_test  | ptif_test   |0
+ ptif_test1  | ptif_test  | ptif_test   |0
+ ptif_test01 | ptif_test0 | ptif_test   | 8192
+ ptif_test11 | ptif_test1 | ptif_test   | 8192
+(5 rows)
+
+select sum(pg_relation_size(p)) as total_size
+from   pg_partition_tree_tables('ptif_test') p;
+ total_size 
+
+  16384
+(1 row)
+
+drop table ptif_test;
diff --git a/src/test/regress/sql/create_table.sql 
b/src/test/regress/sql/create_table.sql
index 78944950fe..d0a083dd69 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -735,3 +735,27 @@ create temp table temp_part partition of perm_parted 
default; -- error
 create temp table temp_part partition of temp_parted default; -- ok
 drop table perm_parted cascade;
 drop table temp_parted cascade;
+
+-- tests to show partition tree inspection functions
+create table ptif_test (a int, b int) partition by range (a);
+create table ptif_test0 partition of ptif_test for values from (minvalue)

Re: partition tree inspection functions

[Michael Paquier]

On Tue, Jun 26, 2018 at 04:57:53PM +0900, Amit Langote wrote:
> Thanks Jeevan for reviewing.

I would like to make things more user-friendly in this area, but could
you add a couple of tests to show up how things work?  I just had a very
quick glance at what's proposed at the top of the thread.
--
Michael


signature.asc
Description: PGP signature

Re: partition tree inspection functions

[Amit Langote]

On 2018/06/26 16:54, Jeevan Ladhe wrote:
> Hi Amit,
> 
> On Tue, Jun 26, 2018 at 1:07 PM, Amit Langote > wrote:
> 
>> On 2018/06/26 14:08, Amit Langote wrote:
>>> Hi.
>>>
>>> As discussed a little while back [1] and also recently mentioned [2],
>> here
>>> is a patch that adds a set of functions to inspect the details of a
>>> partition tree.  There are three functions:
>>>
>>> pg_partition_parent(regclass) returns regclass
>>> pg_partition_root_parent(regclass) returns regclass
>>> pg_partition_tree_tables(regclass) returns setof regclass
>>>
>>
> 
> I quickly tried applying your patch. Created couple of tables,
> subpartitions with
> mix of range and list partitions, and I see these 3 functions are working as
> documented.
> 
> Also, the patch does not have any 'make check' failures.
> 
> I will do the further code review and post if any comments.

Thanks Jeevan for reviewing.

Thanks,
Amit

Re: partition tree inspection functions

[Jeevan Ladhe]

Hi Amit,

On Tue, Jun 26, 2018 at 1:07 PM, Amit Langote  wrote:

> On 2018/06/26 14:08, Amit Langote wrote:
> > Hi.
> >
> > As discussed a little while back [1] and also recently mentioned [2],
> here
> > is a patch that adds a set of functions to inspect the details of a
> > partition tree.  There are three functions:
> >
> > pg_partition_parent(regclass) returns regclass
> > pg_partition_root_parent(regclass) returns regclass
> > pg_partition_tree_tables(regclass) returns setof regclass
> >
>

I quickly tried applying your patch. Created couple of tables,
subpartitions with
mix of range and list partitions, and I see these 3 functions are working as
documented.

Also, the patch does not have any 'make check' failures.

I will do the further code review and post if any comments.

Regards,
Jeevan Ladhe

Re: partition tree inspection functions

[Amit Langote]

On 2018/06/26 14:08, Amit Langote wrote:
> Hi.
> 
> As discussed a little while back [1] and also recently mentioned [2], here
> is a patch that adds a set of functions to inspect the details of a
> partition tree.  There are three functions:
> 
> pg_partition_parent(regclass) returns regclass
> pg_partition_root_parent(regclass) returns regclass
> pg_partition_tree_tables(regclass) returns setof regclass
> 
> Here is an example showing how one may want to use them.
> 
> create table p (a int, b int) partition by range (a);
> create table p0 partition of p for values from (minvalue) to (0) partition
> by hash (b);
> create table p00 partition of p0 for values with (modulus 2, remainder 0);
> create table p01 partition of p0 for values with (modulus 2, remainder 1);
> create table p1 partition of p for values from (0) to (maxvalue) partition
> by hash (b);
> create table p10 partition of p1 for values with (modulus 2, remainder 0);
> create table p11 partition of p1 for values with (modulus 2, remainder 1);
> insert into p select i, i from generate_series(-5, 5) i;
> 
> select pg_partition_parent('p0') as parent;
>  parent
> 
>  p
> (1 row)
> 
> Time: 1.469 ms
> select pg_partition_parent('p01') as parent;
>  parent
> 
>  p0
> (1 row)
> 
> Time: 1.330 ms
> select pg_partition_root_parent('p01') as root_parent;
>  root_parent
> -
>  p
> (1 row)
> 
> selectp as relname,
>   pg_partition_parent(p) as parent,
>   pg_partition_root_parent(p) as root_parent
> from  pg_partition_tree_tables('p') p;
>  relname | parent | root_parent
> -++-
>  p   || p
>  p0  | p  | p
>  p1  | p  | p
>  p00 | p0 | p
>  p01 | p0 | p
>  p10 | p1 | p
>  p11 | p1 | p
> (7 rows)
> 
> selectp as relname,
>   pg_partition_parent(p) as parent,
>   pg_partition_root_parent(p) as root_parent,
>   pg_relation_size(p) as size
> from  pg_partition_tree_tables('p') p;
>  relname | parent | root_parent | size
> -++-+--
>  p   || p   |0
>  p0  | p  | p   |0
>  p1  | p  | p   |0
>  p00 | p0 | p   | 8192
>  p01 | p0 | p   | 8192
>  p10 | p1 | p   | 8192
>  p11 | p1 | p   | 8192
> (7 rows)
> 
> 
> selectsum(pg_relation_size(p)) as total_size
> from  pg_partition_tree_tables('p') p;
>  total_size
> -
>32768
> (1 row)
> 
> Feedback is welcome!

Added this to July CF.

Thanks,
Amit

partition tree inspection functions

[Amit Langote]

Hi.

As discussed a little while back [1] and also recently mentioned [2], here
is a patch that adds a set of functions to inspect the details of a
partition tree.  There are three functions:

pg_partition_parent(regclass) returns regclass
pg_partition_root_parent(regclass) returns regclass
pg_partition_tree_tables(regclass) returns setof regclass

Here is an example showing how one may want to use them.

create table p (a int, b int) partition by range (a);
create table p0 partition of p for values from (minvalue) to (0) partition
by hash (b);
create table p00 partition of p0 for values with (modulus 2, remainder 0);
create table p01 partition of p0 for values with (modulus 2, remainder 1);
create table p1 partition of p for values from (0) to (maxvalue) partition
by hash (b);
create table p10 partition of p1 for values with (modulus 2, remainder 0);
create table p11 partition of p1 for values with (modulus 2, remainder 1);
insert into p select i, i from generate_series(-5, 5) i;

select pg_partition_parent('p0') as parent;
 parent

 p
(1 row)

Time: 1.469 ms
select pg_partition_parent('p01') as parent;
 parent

 p0
(1 row)

Time: 1.330 ms
select pg_partition_root_parent('p01') as root_parent;
 root_parent
-
 p
(1 row)

selectp as relname,
  pg_partition_parent(p) as parent,
  pg_partition_root_parent(p) as root_parent
from  pg_partition_tree_tables('p') p;
 relname | parent | root_parent
-++-
 p   || p
 p0  | p  | p
 p1  | p  | p
 p00 | p0 | p
 p01 | p0 | p
 p10 | p1 | p
 p11 | p1 | p
(7 rows)

selectp as relname,
  pg_partition_parent(p) as parent,
  pg_partition_root_parent(p) as root_parent,
  pg_relation_size(p) as size
from  pg_partition_tree_tables('p') p;
 relname | parent | root_parent | size
-++-+--
 p   || p   |0
 p0  | p  | p   |0
 p1  | p  | p   |0
 p00 | p0 | p   | 8192
 p01 | p0 | p   | 8192
 p10 | p1 | p   | 8192
 p11 | p1 | p   | 8192
(7 rows)


selectsum(pg_relation_size(p)) as total_size
from  pg_partition_tree_tables('p') p;
 total_size
-
   32768
(1 row)

Feedback is welcome!

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/flat/495cec7e-f8d9-7e13-4807-90dbf4eec4ea%40lab.ntt.co.jp

[2]
https://www.postgresql.org/message-id/18e000e8-9bcc-1bb5-2f50-56d434c8be1f%40lab.ntt.co.jp
From 190158bd4b937b1978bfa29e8e9801fa04e0df0d Mon Sep 17 00:00:00 2001
From: amit 
Date: Tue, 16 Jan 2018 19:02:13 +0900
Subject: [PATCH v1] Add assorted partition reporting functions

---
 doc/src/sgml/func.sgml  |  34 ++
 src/backend/catalog/partition.c | 129 ++--
 src/backend/utils/cache/lsyscache.c |  22 ++
 src/include/catalog/partition.h |   1 +
 src/include/catalog/pg_proc.dat |  18 +
 src/include/utils/lsyscache.h   |   1 +
 6 files changed, 201 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 5dce8ef178..df621d1e17 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19995,6 +19995,40 @@ postgres=# SELECT * FROM 
pg_walfile_name_offset(pg_stop_backup());
 The function returns the number of new collation objects it created.

 
+   
+Partitioning Information Functions
+
+ 
+  Name Return Type 
Description
+ 
+
+ 
+  
+   
pg_partition_root_parent(regclass)
+   regclass
+   get root table of partition tree of which the table is 
part
+  
+  
+   
pg_partition_parent(regclass)
+   regclass
+   get parent table if the table is a partition, 
NULL otherwise
+  
+  
+   
pg_partition_tree_tables(regclass)
+   setof regclass
+   get all tables in partition tree under given root table
+  
+ 
+
+   
+
+   
+If the table passed to pg_partition_root_parent is not
+a partition, the same table is returned as the result.  Result of
+pg_partition_tree_tables also contains the table
+that's passed to it as the first row.
+   
+
   
 
   
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 558022647c..5b3e8d52c5 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -23,13 +23,16 @@
 #include "catalog/partition.h"
 #include "catalog/pg_inherits.h"
 #include "catalog/pg_partitioned_table.h"
+#include "funcapi.h"
 #include "nodes/makefuncs.h"
 #include "optimizer/clauses.h"
 #include "optimizer/prep.h"
 #include "optimizer/var.h"
 #include "partitioning/partbounds.h"
 #include "rewrite/rewriteManip.h"
+#include "utils/builtins.h"
 #include "utils/fmgroids.h"
+#include "utils/lsyscache.h"
 #include "utils/partcache.h"
 #include "utils/rel.h"
 #include