Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On Mon, Jan 29, 2018 at 10:08:51AM +0900, Amit Langote wrote: > On 2018/01/27 3:32, Robert Haas wrote: >> If it has >> either partitions or inheritance children, find_all_inheritors will >> return them. Otherwise, I think it'll just return the input OID >> itself. So I don't quite see, if we're going to add a convenience >> function here, why wouldn't just define it to return the same results >> as find_all_inheritors does? > > So if all we're doing is trying to make find_all_inheritors() accessible > to users through SQL, it makes sense to call it > pg_get_inheritance_tables() rather than pg_partition_tree_tables(). I was looking again at this stuff this morning, noticing that find_all_inheritors() is particularly used in check_default_allows_bound() for partitions, so complaint withdrawn. The renaming as you propose here looks sensible as well. -- Michael signature.asc Description: PGP signature
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On 2018/01/27 3:32, Robert Haas wrote: > On Fri, Jan 26, 2018 at 7:45 AM, Michael Paquier > wrote: >> There could be value in having a version dedicated to inheritance trees >> as well, true enough. As well as value in having something that shows >> both. Still let's not forget that partition sets are structured so as >> the parents have no data, so I see more value in having only partitions >> listed, without the INHERIT part. Opinions from others are of course >> welcome. > > Well, partitioning and inheritance can't be mixed. A given table has > either partitions OR inheritance children OR neither. That's true. > If it has > either partitions or inheritance children, find_all_inheritors will > return them. Otherwise, I think it'll just return the input OID > itself. So I don't quite see, if we're going to add a convenience > function here, why wouldn't just define it to return the same results > as find_all_inheritors does? So if all we're doing is trying to make find_all_inheritors() accessible to users through SQL, it makes sense to call it pg_get_inheritance_tables() rather than pg_partition_tree_tables(). Thanks, Amit
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On Fri, Jan 26, 2018 at 7:45 AM, Michael Paquier wrote: > There could be value in having a version dedicated to inheritance trees > as well, true enough. As well as value in having something that shows > both. Still let's not forget that partition sets are structured so as > the parents have no data, so I see more value in having only partitions > listed, without the INHERIT part. Opinions from others are of course > welcome. Well, partitioning and inheritance can't be mixed. A given table has either partitions OR inheritance children OR neither. If it has either partitions or inheritance children, find_all_inheritors will return them. Otherwise, I think it'll just return the input OID itself. So I don't quite see, if we're going to add a convenience function here, why wouldn't just define it to return the same results as find_all_inheritors does? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On Fri, Jan 26, 2018 at 07:00:43PM +0900, Amit Langote wrote: > I wonder what pg_partition_tree_tables() should return when passed a table > that doesn't have partitions under it? Return a 1-member set containing > itself? Yes. A table alone is itself part of a partition set, so the result should be made of only itself. > I also mean for tables that may inheritance children established > through plain old inheritance. There could be value in having a version dedicated to inheritance trees as well, true enough. As well as value in having something that shows both. Still let's not forget that partition sets are structured so as the parents have no data, so I see more value in having only partitions listed, without the INHERIT part. Opinions from others are of course welcome. -- Michael signature.asc Description: PGP signature
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On 2018/01/22 11:44, Michael Paquier wrote: > On Sun, Jan 21, 2018 at 07:16:38PM +1300, David Rowley wrote: >> On 20 January 2018 at 23:14, Michael Paquier >> wrote: >>> If pg_partition_tree_tables() uses the top of the partition as input, >>> all the tree should show up. If you use a leaf, anything under the leaf >>> should show up. If a leaf is defined and it has no underlying leaves, >>> then only this outmost leaf should be listed. >> >> hmm, that's thoroughly confusing. Just in case anyone else is stuck on >> that, I just need to mention that a leaf is the does not have >> branches, in nature or computer science. > > OK, sorry if my words are confusing. Imagine that you have the following > partition set: > >p > / \ > / \ > p1 p2 > / \ >/\ > p21 p22 > > If pg_partition_tree_tables() uses 'p' as input argument, then I would > imagine that it should return p, p1, p2, p21 and p22. If 'p2' is used, > then p2, p21 and p22 are the results. If either one of p1, p21 or p22 is > used as input, then the result is respectively p1, p21 or p22. > > Amit's patch seems to be doing that kind of logic by using > find_all_inheritors, which is good. We need to make the difference > between relations that are part of a partition set and the other ones > which are part of an INHERIT link, and, at least it seems to me, the > patch is not careful with that. I haven't tested what is proposed > though, but this portion likely needs more thoughts. Yeah, I think I completely missed that part. I wonder what pg_partition_tree_tables() should return when passed a table that doesn't have partitions under it? Return a 1-member set containing itself? I also mean for tables that may inheritance children established through plain old inheritance. Thanks, Amit
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On Sun, Jan 21, 2018 at 07:16:38PM +1300, David Rowley wrote: > On 20 January 2018 at 23:14, Michael Paquier > wrote: >> If pg_partition_tree_tables() uses the top of the partition as input, >> all the tree should show up. If you use a leaf, anything under the leaf >> should show up. If a leaf is defined and it has no underlying leaves, >> then only this outmost leaf should be listed. > > hmm, that's thoroughly confusing. Just in case anyone else is stuck on > that, I just need to mention that a leaf is the does not have > branches, in nature or computer science. OK, sorry if my words are confusing. Imagine that you have the following partition set: p / \ / \ p1 p2 / \ /\ p21 p22 If pg_partition_tree_tables() uses 'p' as input argument, then I would imagine that it should return p, p1, p2, p21 and p22. If 'p2' is used, then p2, p21 and p22 are the results. If either one of p1, p21 or p22 is used as input, then the result is respectively p1, p21 or p22. Amit's patch seems to be doing that kind of logic by using find_all_inheritors, which is good. We need to make the difference between relations that are part of a partition set and the other ones which are part of an INHERIT link, and, at least it seems to me, the patch is not careful with that. I haven't tested what is proposed though, but this portion likely needs more thoughts. -- Michael signature.asc Description: PGP signature
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On 20 January 2018 at 23:14, Michael Paquier wrote: > If pg_partition_tree_tables() uses the top of the partition as input, > all the tree should show up. If you use a leaf, anything under the leaf > should show up. If a leaf is defined and it has no underlying leaves, > then only this outmost leaf should be listed. hmm, that's thoroughly confusing. Just in case anyone else is stuck on that, I just need to mention that a leaf is the does not have branches, in nature or computer science. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On Fri, Jan 19, 2018 at 06:28:41PM +0900, Amit Langote wrote:
> Do you mean pg_partition_tree(regclass), that returns all partitions in
> the partition tree whose root is passed as the parameter?
>
> Perhaps, like the following (roughly implemented in the attached)?
>
> select pg_partition_root(p) as root_parent,
> pg_partition_parent(p) as parent,
> p as relname,
> pg_total_relation_size(p) as size
> frompg_partition_tree_tables('p') p
> order by 4;
> root_parent | parent | relname | size
> -++-+-
> p || p | 0
> p | p | p123| 0
> p | p123 | p12 | 0
> p | p123 | p3 | 3653632
> p | p12| p1 | 3653632
> p | p12| p2 | 3653632
> (6 rows)
It seems that you caught the idea. As long as each leaf and root is
listed uniquely, that would be acceptable to me, and useful for users.
If pg_partition_tree_tables() uses the top of the partition as input,
all the tree should show up. If you use a leaf, anything under the leaf
should show up. If a leaf is defined and it has no underlying leaves,
then only this outmost leaf should be listed.
+/*
+ * Returns Oids of tables in a publication.
+ */
Close enough, but that's not a publication.
>> Documentation, as well as regression tests, would be welcome :)
>
> OK, I will add those things in the next version.
Thanks.
--
Michael
signature.asc
Description: PGP signature
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
Thanks for taking a look.
On 2018/01/19 14:39, Michael Paquier wrote:
> On Thu, Jan 18, 2018 at 06:54:18PM +0900, Amit Langote wrote:
>> I think having pg_partition_root() and pg_partition_parent() will give
>> users enough to get useful views as follows:
>
> So... pg_partition_root() gives you access to the highest relation in
> the hierarchy, and pg_partition_parent() gives you access to the direct
> parent.
Right.
>> drop table p;
>> create table p (a int) partition by list (a);
>> create table p123 partition of p for values in (1, 2, 3) partition by list
> (a);
>> create table p12 partition of p1 for values in (1, 2) partition by list (a);
>> create table p12 partition of p123 for values in (1, 2) partition by list
>> (a);
>> create table p1 partition of p12 for values in (1);
>> create table p2 partition of p12 for values in (2);
>> create table p3 partition of p123 for values in (3);
>
> You need to reorder those queries, the creation of the first p12 would
> fail as p1 does not exist at this point.
Oops. I had copy-pasted above commands from the psql's \s output and
ended up copying the command I didn't intend to. Here it is again, but
without the mistake I made in my last email:
drop table p;
create table p (a int) partition by list (a);
create table p123 partition of p for values in (1, 2, 3) partition by list
(a);
create table p12 partition of p123 for values in (1, 2) partition by list (a);
create table p1 partition of p12 for values in (1);
create table p2 partition of p12 for values in (2);
create table p3 partition of p123 for values in (3);
> Wouldn't also a
> pg_partition_tree() be useful? You could shape it as a function which
> returns all regclass partitions in the tree as unique entries. Combined
> with pg_partition_parent() it can be powerful as it returns NULL for the
> partition at the top of the tree. So I think that we could live without
> pg_partition_root(). At the end, let's design something which makes
> unnecessary the use of WITH RECURSIVE when looking at a full partition
> tree to ease the user's life.
Do you mean pg_partition_tree(regclass), that returns all partitions in
the partition tree whose root is passed as the parameter?
Perhaps, like the following (roughly implemented in the attached)?
select pg_partition_root(p) as root_parent,
pg_partition_parent(p) as parent,
p as relname,
pg_total_relation_size(p) as size
frompg_partition_tree_tables('p') p
order by 4;
root_parent | parent | relname | size
-++-+-
p || p | 0
p | p | p123| 0
p | p123 | p12 | 0
p | p123 | p3 | 3653632
p | p12| p1 | 3653632
p | p12| p2 | 3653632
(6 rows)
> Documentation, as well as regression tests, would be welcome :)
OK, I will add those things in the next version.
Thanks,
Amit
>From 50dfb02bd3ea833d8b18fc5d3d54e863fbc223e4 Mon Sep 17 00:00:00 2001
From: amit
Date: Tue, 16 Jan 2018 19:02:13 +0900
Subject: [PATCH] Add assorted partition reporting functions
---
src/backend/catalog/partition.c | 117 +++-
src/backend/utils/cache/lsyscache.c | 22 +++
src/include/catalog/partition.h | 1 +
src/include/catalog/pg_proc.h | 12
src/include/utils/lsyscache.h | 1 +
5 files changed, 152 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 8adc4ee977..ac92bbfa71 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -32,6 +32,7 @@
#include "catalog/pg_type.h"
#include "commands/tablecmds.h"
#include "executor/executor.h"
+#include "funcapi.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
@@ -181,6 +182,7 @@ static int partition_bound_bsearch(PartitionKey key,
static int get_partition_bound_num_indexes(PartitionBoundInfo b);
static int get_greatest_modulus(PartitionBoundInfo b);
static uint64 compute_hash_value(PartitionKey key, Datum *values, bool
*isnull);
+static Oid get_partition_parent_internal(Oid relid, bool recurse_to_root);
/* SQL-callable function for use in hash partition CHECK constraints */
PG_FUNCTION_INFO_V1(satisfies_hash_partition);
@@ -1362,7 +1364,7 @@ check_default_allows_bound(Relation parent, Relation
default_rel,
/*
* get_partition_parent
*
- * Returns inheritance parent of a partition by scanning pg_inherits
+ * Returns inheritance parent of a partition.
*
* Note: Because this function assumes that the relation whose OID is passed
* as an argument will have precisely one parent, it should only be called
@@ -1371,6 +1373,37 @@ check_default_allows_bound(Relation parent, Relation
default_rel,
Oid
get_partition_parent(Oid relid)
{
+ if (!get_rel_relispartition(relid))
+ return InvalidOid;
+
+ return get_partitio
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On Thu, Jan 18, 2018 at 06:54:18PM +0900, Amit Langote wrote: > I think having pg_partition_root() and pg_partition_parent() will give > users enough to get useful views as follows: So... pg_partition_root() gives you access to the highest relation in the hierarchy, and pg_partition_parent() gives you access to the direct parent. > drop table p; > create table p (a int) partition by list (a); > create table p123 partition of p for values in (1, 2, 3) partition by list (a); > create table p12 partition of p1 for values in (1, 2) partition by list (a); > create table p12 partition of p123 for values in (1, 2) partition by list (a); > create table p1 partition of p12 for values in (1); > create table p2 partition of p12 for values in (2); > create table p3 partition of p123 for values in (3); You need to reorder those queries, the creation of the first p12 would fail as p1 does not exist at this point. Wouldn't also a pg_partition_tree() be useful? You could shape it as a function which returns all regclass partitions in the tree as unique entries. Combined with pg_partition_parent() it can be powerful as it returns NULL for the partition at the top of the tree. So I think that we could live without pg_partition_root(). At the end, let's design something which makes unnecessary the use of WITH RECURSIVE when looking at a full partition tree to ease the user's life. Documentation, as well as regression tests, would be welcome :) -- Michael signature.asc Description: PGP signature
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On 2018/01/02 22:45, Peter Eisentraut wrote:
> On 12/28/17 16:24, David Rowley wrote:
>>> select pg_partition_root(c.oid), c.relname, pg_table_size(c.oid)
>>> from pg_class c
>>> order by 1
>>>
>>> select pg_partition_root(c.oid), sum(pg_table_size(c.oid))
>>> from pg_class c
>>> group by 1
>>
>> That seems much nicer. I assume "root" would mean the top level
>> partitioned table. If so, would we also want
>> pg_partition_parent(regclass)? Or maybe something to control the
>> number of "levels-up" the function would run for. If we had that then
>> maybe -1 could mean "go until you find a table with no parent".
>
> Hmm, we need to think through some scenarios for what one would really
> want to do with this functionality.
>
> Clearly, the existing behavior is useful for management tasks like bloat
> and vacuum monitoring.
>
> And on the other hand you might want to have a logical view of, how big
> is this partitioned table altogether.
>
> But what are the uses for dealing with partial partition hierarchies?
> How easy do we need to make that?
I think having pg_partition_root() and pg_partition_parent() will give
users enough to get useful views as follows:
drop table p;
create table p (a int) partition by list (a);
create table p123 partition of p for values in (1, 2, 3) partition by list
(a);
create table p12 partition of p1 for values in (1, 2) partition by list (a);
create table p12 partition of p123 for values in (1, 2) partition by list (a);
create table p1 partition of p12 for values in (1);
create table p2 partition of p12 for values in (2);
create table p3 partition of p123 for values in (3);
insert into p select 1 from generate_series(1, 100);
insert into p select 2 from generate_series(1, 100);
insert into p select 3 from generate_series(1, 100);
select pg_partition_root(oid) as root_parent,
pg_partition_parent(oid) as parent,
relname as relname,
pg_total_relation_size(oid) as size
frompg_class
where relnamespace = 'public'::regnamespace
order by 4;
root_parent | parent | relname | size
-++-+--
p || p |0
p | p | p123|0
p | p123 | p12 |0
p | p12| p1 | 8192
p | p12| p2 | 8192
p | p123 | p3 | 8192
(6 rows)
select pg_partition_root(oid) as root_parent,
sum(pg_total_relation_size(oid)) as size
frompg_class
where relnamespace = 'public'::regnamespace
group by 1
order by 1;
root_parent | size
-+---
p | 24576
(1 row)
Attached a WIP patch.
Thanks,
Amit
From b1c0973c2b363d03b4d074d324560048f48ad5a7 Mon Sep 17 00:00:00 2001
From: amit
Date: Tue, 16 Jan 2018 19:02:13 +0900
Subject: [PATCH v1] Add a pg_partition_root() and pg_partition_parent()
---
src/backend/catalog/partition.c | 37 -
src/backend/utils/adt/misc.c| 34 ++
src/backend/utils/cache/lsyscache.c | 22 ++
src/include/catalog/partition.h | 1 +
src/include/catalog/pg_proc.h | 8
src/include/utils/lsyscache.h | 1 +
6 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 8adc4ee977..cf5f971b91 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -181,6 +181,7 @@ static int partition_bound_bsearch(PartitionKey key,
static int get_partition_bound_num_indexes(PartitionBoundInfo b);
static int get_greatest_modulus(PartitionBoundInfo b);
static uint64 compute_hash_value(PartitionKey key, Datum *values, bool
*isnull);
+static Oid get_partition_parent_internal(Oid relid, bool recurse_to_root);
/* SQL-callable function for use in hash partition CHECK constraints */
PG_FUNCTION_INFO_V1(satisfies_hash_partition);
@@ -1362,7 +1363,7 @@ check_default_allows_bound(Relation parent, Relation
default_rel,
/*
* get_partition_parent
*
- * Returns inheritance parent of a partition by scanning pg_inherits
+ * Returns inheritance parent of a partition.
*
* Note: Because this function assumes that the relation whose OID is passed
* as an argument will have precisely one parent, it should only be called
@@ -1371,6 +1372,37 @@ check_default_allows_bound(Relation parent, Relation
default_rel,
Oid
get_partition_parent(Oid relid)
{
+ if (!get_rel_relispartition(relid))
+ return InvalidOid;
+
+ return get_partition_parent_internal(relid, false);
+}
+
+/*
+ * get_partition_root_parent
+ *
+ * Returns root inheritance ancestor of a partition.
+ */
+Oid
+get_partition_root_parent(Oid relid)
+{
+ if (!get_rel_relispartition(relid))
+ return InvalidOid;
+
+ return get_partition_parent_internal(relid, true);
+}
+
+/*
+ * get_partition_parent_internal
+ *
+ * Returns inheritance parent of a partition by scan
Re: [Sender Address Forgery]Re: pg_(total_)relation_size and partitioned tables
On 2018/01/17 22:51, Peter Eisentraut wrote: > I'm setting this patch to Returned with feedback. OK. Sorry that I couldn't reply here since the CF started. I have written some code to implement the pg_partition_root() idea you mentioned upthread earlier this week, but hasn't been able to share it on the list yet. I will post it soon and create a new entry in the next commit fest. Thanks, Amit
