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
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
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
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
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
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
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
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
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,
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,
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
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
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
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
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
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
>
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
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
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
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.
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
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
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
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
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.
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
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
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
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
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
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
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
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.
>
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
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
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
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
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
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
>>
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,
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
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
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
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*
l_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
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
edule
index 42632be675..7e374c2daa 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -188,6 +188,7 @@ test: reloptions
test: hash_part
test: indexing
test: partition_aggregate
+test: partition_info
test: event_trigger
test: fast_default
test: stats
diff --gi
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
le
index 42632be675..7e374c2daa 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -188,6 +188,7 @@ test: reloptions
test: hash_part
test: indexing
test: partition_aggregate
+test: partition_info
test: event_trigger
test: fast_default
test: stats
diff --git a
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*
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
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
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
lid);
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_tr
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
>>
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
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
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
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
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
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
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.
);
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 r
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.
+
rc/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 sho
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.
--
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
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.
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
>
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
70 matches
Mail list logo
