Re: [HACKERS] Documentation improvements for partitioning
On Mon, Apr 3, 2017 at 12:52 AM, Amit Langotewrote: > I noticed what looks like a redundant line (or an incomplete sentence) in > the paragraph about multi-column partition keys. In the following text: > > + partitioning, if desired. Of course, this will often result in a > larger > + number of partitions, each of which is individually smaller. > + criteria. Using fewer columns may lead to coarser-grained > + A query accessing the partitioned table will have > + to scan fewer partitions if the conditions involve some or all of > these > > This: > > + criteria. Using fewer columns may lead to coarser-grained > > looks redundant. But maybe we can keep that sentence by completing it, > which the attached patch does as follows: > > + number of partitions, each of which is individually smaller. On the > + other hand, using fewer columns may lead to a coarser-grained > + partitioning criteria with smaller number of partitions. > > The patch also fixes some typos I noticed. Thanks for the corrections. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/04/01 6:37, Robert Haas wrote: > On Tue, Mar 28, 2017 at 6:35 AM, Amit Langote >wrote: >> Attached updated patch. > > Committed with editing here and there. I just left out the thing > about UNION ALL views, which seemed to brief a treatment to deserve > its own subsection. Maybe a longer explanation of that is worthwhile > or maybe it's not, but that can be a separate patch. Thanks for committing. I noticed what looks like a redundant line (or an incomplete sentence) in the paragraph about multi-column partition keys. In the following text: + partitioning, if desired. Of course, this will often result in a larger + number of partitions, each of which is individually smaller. + criteria. Using fewer columns may lead to coarser-grained + A query accessing the partitioned table will have + to scan fewer partitions if the conditions involve some or all of these This: + criteria. Using fewer columns may lead to coarser-grained looks redundant. But maybe we can keep that sentence by completing it, which the attached patch does as follows: + number of partitions, each of which is individually smaller. On the + other hand, using fewer columns may lead to a coarser-grained + partitioning criteria with smaller number of partitions. The patch also fixes some typos I noticed. Thanks, Amit diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 5109778196..340c961b3f 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2932,7 +2932,7 @@ VALUES ('Albany', NULL, NULL, 'NY'); tables and partitions. For example, a partition cannot have any parents other than the partitioned table it is a partition of, nor can a regular table inherit from a partitioned table making the latter its parent. -That means partitioned table and partitions do not participate in +That means partitioned tables and partitions do not participate in inheritance with regular tables. Since a partition hierarchy consisting of the partitioned table and its partitions is still an inheritance hierarchy, all the normal rules of inheritance apply as described in @@ -3036,11 +3036,12 @@ CREATE TABLE measurement ( You may decide to use multiple columns in the partition key for range partitioning, if desired. Of course, this will often result in a larger - number of partitions, each of which is individually smaller. - criteria. Using fewer columns may lead to coarser-grained - A query accessing the partitioned table will have - to scan fewer partitions if the conditions involve some or all of these - columns. For example, consider a table range partitioned using columns + number of partitions, each of which is individually smaller. On the + other hand, using fewer columns may lead to a coarser-grained + partitioning criteria with smaller number of partitions. A query + accessing the partitioned table will have to scan fewer partitions if + the conditions involve some or all of these columns. + For example, consider a table range partitioned using columns lastname and firstname (in that order) as the partition key. @@ -3167,8 +3168,8 @@ CREATE INDEX ON measurement_y2008m01 (logdate); - The simplest option for removing old data is simply to drop the partition - that is no longer necessary: + The simplest option for removing old data is to drop the partition that + is no longer necessary: DROP TABLE measurement_y2006m02; @@ -3595,8 +3596,8 @@ DO INSTEAD Partition Maintenance - To remove old data quickly, simply to drop the partition that is no - longer necessary: + To remove old data quickly, simply drop the partition that is no longer + necessary: DROP TABLE measurement_y2006m02; @@ -3692,7 +3693,7 @@ ANALYZE measurement; Triggers or rules will be needed to route rows to the desired partition, unless the application is explicitly aware of the partitioning scheme. Triggers may be complicated to write, and will -be much slower than the tuple routing performed interally by +be much slower than the tuple routing performed internally by declarative partitioning. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Tue, Mar 28, 2017 at 6:35 AM, Amit Langotewrote: > Attached updated patch. Committed with editing here and there. I just left out the thing about UNION ALL views, which seemed to brief a treatment to deserve its own subsection. Maybe a longer explanation of that is worthwhile or maybe it's not, but that can be a separate patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/03/28 0:23, Robert Haas wrote: > On Thu, Mar 9, 2017 at 8:23 PM, Amit Langote wrote: >> Attached updated patches. > > Committed 0002, 0003. Thanks a lot for committing these and reviewing 0001. > I think the section on BRIN in 0001 is just silly. BRIN is a very > useful index type, possibly more useful than anything except btree, > but I think documenting it as an alternative method of partitioning is > over the top. Okay, removing the BRIN part from this patch for now. > + The following forms of partitioning can be implemented in > + PostgreSQL: > > Any form of partitioning can be implemented, at least to some degree, > using inheritance or UNION ALL views. I think what this should say is > that PostgreSQL has native support for list and range partitioning, > and then it can go on top say that if this built-in support is not > suitable for a particular use case (either because you need some other > partitioning scheme or due to some other limitation), inheritance or > UNION ALL views can be used instead, adding flexibility but losing > some of the performance benefits of built-in declarative partitioning. You're right. I've updated the text to sound like what you said here. > > Partitions may have their own indexes, constraints and default values, > -distinct from other partitions. Partitions do not inherit indexes from > -the partitioned table. > +distinct from other partitions. Partitions do not currently inherit > +indexes from the partitioned table. > + > + > + > +See for more details creating > partitioned > +tables and partitions. > > > I don't think we should add "currently"; that amounts to speculation > about what will happen in future versions. Also, I favor collapsing > these into one paragraph. A single-sentence paragraph tends to look > OK when you're reading the SGML directly, but it looks funny in the > rendered version. Done. > > +sub-partitioning. It is not currently possible to > +alter a regular table into a partitioned table or vice versa. However, > +it is possible to add a regular or partitioned table containing data into > +a partition of a partitioned table, or remove a partition; see > > I think we should say "as a partition" rather than "into a partition", > assuming you're talking about ATTACH PARTITION here. Right, fixed. > > - partitioned tables. For example, specifying ONLY > - when querying data from a partitioned table would not make much sense, > - because all the data is contained in partitions, so this raises an > - error. Specifying ONLY when modifying schema is > - not desirable in certain cases with partitioned tables where it may be > - fine for regular inheritance parents (for example, dropping a column > - from only the parent); an error will be thrown in that case. > + partitioned tables. Specifying ONLY when modifying > + schema is not desirable in certain cases with partitioned tables > + whereas it may be fine for regular inheritance parents (for example, > + dropping a column from only the parent); an error will be thrown in > + that case. > > I don't see why this is an improvement. Because we neither raise an error nor ignore it if ONLY is specified when querying data from a partitioned table. create table p (a int, b char) partition by list (a); create table p1 partition of p for values in (1); insert into p values (1); select * from only p; a | b ---+--- (0 rows) explain select * from only p; QUERY PLAN --- Result (cost=0.00..0.00 rows=0 width=12) One-Time Filter: false (2 rows) IOW, querying behavior is same as regular inheritance. I rewrote the paragraph as follows: The ONLY notation used to exclude child tables will cause an error for partitioned tables in the case of schema-modifying commands such as most ALTER TABLE commands. For example, dropping a column from only the parent does not make sense for partitioned tables. > -data inserted into the partitioned table cannot be routed to foreign > table > -partitions. > +data inserted into the partitioned table is currently not routed to > foreign > +table partitions. > > Again, let's not speculate about the future. > > + Note that it is currently not supported to propagate index definition > + from the master partitioned table to its partitions; in fact, it is > + not possible to define indexes on partitioned tables in the first > + place. This might change in future releases. > > Same here. > > +There are currently the following limitations of using partitioned > tables: > > And here. Better to write "The following limitations apply to > partitioned tables:" Fixed all of these. > + It is currently not possible to define indexes on partitioned tables > + that
Re: [HACKERS] Documentation improvements for partitioning
On Thu, Mar 9, 2017 at 8:23 PM, Amit Langotewrote: > On 2017/03/10 3:26, Robert Haas wrote: >> I think you might have the titles for 0002 and 0003 backwards. > > Oops, you're right. > >> On Fri, Mar 3, 2017 at 2:51 AM, Amit Langote wrote: >>> 0002: some cosmetic fixes to create_table.sgml >> >> I think this sentence may be unclear to some readers: >> >> + One might however want to set it for only some partitions, >> + which is possible by doing SET NOT NULL on >> individual >> + partitions. >> >> I think you could replace this with something like: Even if there is >> no NOT NULL constraint on the parent, such a constraint >> can still be added to individual partitions, if desired; that is, the >> children can disallow nulls even if the parent allows them, but not >> the other way around. > > Reads much better, done that way. Thanks. > >>> 0003: add clarification about NOT NULL constraint on partition columns in >>> alter_table.sgml >> >> This is about list-ifying a note, but I think we should try to >> de-note-ify it. It's a giant block of text that is not obviously more >> noteworthy than the surrounding text; I think should be saved >> for things that particularly need to be called out. > > OK. The patch is now just about de-note-ifying the block of text. Since > I don't see any other lists in the Parameters portion of the page, I also > take back my list-ifying proposal. > > Attached updated patches. Committed 0002, 0003. I think the section on BRIN in 0001 is just silly. BRIN is a very useful index type, possibly more useful than anything except btree, but I think documenting it as an alternative method of partitioning is over the top. + The following forms of partitioning can be implemented in + PostgreSQL: Any form of partitioning can be implemented, at least to some degree, using inheritance or UNION ALL views. I think what this should say is that PostgreSQL has native support for list and range partitioning, and then it can go on top say that if this built-in support is not suitable for a particular use case (either because you need some other partitioning scheme or due to some other limitation), inheritance or UNION ALL views can be used instead, adding flexibility but losing some of the performance benefits of built-in declarative partitioning. Partitions may have their own indexes, constraints and default values, -distinct from other partitions. Partitions do not inherit indexes from -the partitioned table. +distinct from other partitions. Partitions do not currently inherit +indexes from the partitioned table. + + + +See for more details creating partitioned +tables and partitions. I don't think we should add "currently"; that amounts to speculation about what will happen in future versions. Also, I favor collapsing these into one paragraph. A single-sentence paragraph tends to look OK when you're reading the SGML directly, but it looks funny in the rendered version. +sub-partitioning. It is not currently possible to +alter a regular table into a partitioned table or vice versa. However, +it is possible to add a regular or partitioned table containing data into +a partition of a partitioned table, or remove a partition; see I think we should say "as a partition" rather than "into a partition", assuming you're talking about ATTACH PARTITION here. - partitioned tables. For example, specifying ONLY - when querying data from a partitioned table would not make much sense, - because all the data is contained in partitions, so this raises an - error. Specifying ONLY when modifying schema is - not desirable in certain cases with partitioned tables where it may be - fine for regular inheritance parents (for example, dropping a column - from only the parent); an error will be thrown in that case. + partitioned tables. Specifying ONLY when modifying + schema is not desirable in certain cases with partitioned tables + whereas it may be fine for regular inheritance parents (for example, + dropping a column from only the parent); an error will be thrown in + that case. I don't see why this is an improvement. -data inserted into the partitioned table cannot be routed to foreign table -partitions. +data inserted into the partitioned table is currently not routed to foreign +table partitions. Again, let's not speculate about the future. + Note that it is currently not supported to propagate index definition + from the master partitioned table to its partitions; in fact, it is + not possible to define indexes on partitioned tables in the first + place. This might change in future releases. Same here. +There are currently the following limitations of using partitioned tables: And here. Better to write "The following limitations apply to partitioned
Re: [HACKERS] Documentation improvements for partitioning
On 2017/03/10 3:26, Robert Haas wrote: > I think you might have the titles for 0002 and 0003 backwards. Oops, you're right. > On Fri, Mar 3, 2017 at 2:51 AM, Amit Langote wrote: >> 0002: some cosmetic fixes to create_table.sgml > > I think this sentence may be unclear to some readers: > > + One might however want to set it for only some partitions, > + which is possible by doing SET NOT NULL on > individual > + partitions. > > I think you could replace this with something like: Even if there is > no NOT NULL constraint on the parent, such a constraint > can still be added to individual partitions, if desired; that is, the > children can disallow nulls even if the parent allows them, but not > the other way around. Reads much better, done that way. Thanks. >> 0003: add clarification about NOT NULL constraint on partition columns in >> alter_table.sgml > > This is about list-ifying a note, but I think we should try to > de-note-ify it. It's a giant block of text that is not obviously more > noteworthy than the surrounding text; I think should be saved > for things that particularly need to be called out. OK. The patch is now just about de-note-ifying the block of text. Since I don't see any other lists in the Parameters portion of the page, I also take back my list-ifying proposal. Attached updated patches. Thanks, Amit >From a159c9aa3ee7f2c51084f94243be16a30242d7a6 Mon Sep 17 00:00:00 2001 From: amitDate: Fri, 3 Mar 2017 16:39:24 +0900 Subject: [PATCH 1/3] Rewrite sections in ddl.sgml related to partitioning Merge sections Partitioned Tables and Partitioning into one section called Table Partitioning and Related Solutions. --- doc/src/sgml/ddl.sgml | 1359 + 1 file changed, 707 insertions(+), 652 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 09b5b3ff70..a2dd39df54 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2772,14 +2772,181 @@ VALUES ('Albany', NULL, NULL, 'NY'); - - Partitioned Tables + + Table Partitioning and Related Solutions + + +partitioning + + + +table +partitioning + partitioned table +PostgreSQL supports basic table +partitioning. This section describes why and how to implement +partitioning as part of your database design. + + + + Overview + + + Partitioning refers to splitting what is logically one large table into + smaller physical pieces. Partitioning can provide several benefits: + + + + Query performance can be improved dramatically in certain situations, + particularly when most of the heavily accessed rows of the table are in a + single partition or a small number of partitions. The partitioning + substitutes for leading columns of indexes, reducing index size and + making it more likely that the heavily-used parts of the indexes + fit in memory. + + + + + + When queries or updates access a large percentage of a single + partition, performance can be improved by taking advantage + of sequential scan of that partition instead of using an + index and random access reads scattered across the whole table. + + + + + + Bulk loads and deletes can be accomplished by adding or removing + partitions, if that requirement is planned into the partitioning design. + Doing ALTER TABLE DETACH PARTITION followed by + DROP TABLE is far faster than a bulk operation. These + commands also entirely avoid the VACUUM overhead + caused by a bulk DELETE. + + + + + + Seldom-used data can be migrated to cheaper and slower storage media. + + + + + The benefits will normally be worthwhile only when a table would + otherwise be very large. The exact point at which a table will + benefit from partitioning depends on the application, although a + rule of thumb is that the size of the table should exceed the physical + memory of the database server. + + + + The following forms of partitioning can be implemented in + PostgreSQL: + + + + Range Partitioning + + + + The table is partitioned into ranges defined + by a key column or set of columns, with no overlap between + the ranges of values assigned to different partitions. For + example one might partition by date ranges, or by ranges of + identifiers for particular business objects. + + + + + + List Partitioning + + + + The table is partitioned by explicitly listing which key values + appear in each partition. + + + + + + + + The following partitioning methods are currently supported: + + + +
Re: [HACKERS] Documentation improvements for partitioning
Robert Haaswrites: > This is about list-ifying a note, but I think we should try to > de-note-ify it. It's a giant block of text that is not obviously more > noteworthy than the surrounding text; I think should be saved > for things that particularly need to be called out. Yeah. A big problem with the markup we use, imo, is that segments are displayed in a way that makes them more prominent than the surrounding text, not less so. That doesn't really square with my intuitive view of what a ought to be used for; it forces it to be considered as something only slightly less dangerous than a or , not as a parenthetical remark. But that's what we have to deal with so we should mark up our text accordingly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
I think you might have the titles for 0002 and 0003 backwards. On Fri, Mar 3, 2017 at 2:51 AM, Amit Langotewrote: > 0002: some cosmetic fixes to create_table.sgml I think this sentence may be unclear to some readers: + One might however want to set it for only some partitions, + which is possible by doing SET NOT NULL on individual + partitions. I think you could replace this with something like: Even if there is no NOT NULL constraint on the parent, such a constraint can still be added to individual partitions, if desired; that is, the children can disallow nulls even if the parent allows them, but not the other way around. > 0003: add clarification about NOT NULL constraint on partition columns in > alter_table.sgml This is about list-ifying a note, but I think we should try to de-note-ify it. It's a giant block of text that is not obviously more noteworthy than the surrounding text; I think should be saved for things that particularly need to be called out. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/28 17:25, Simon Riggs wrote: > On 28 February 2017 at 08:14, Amit Langote >wrote: > >> OK. So, I will start writing the patch with above general skeleton and >> hopefully post it within this week and you can improve it as fit. > > Will do, thanks. Attached patch 0001 is what I managed so far. Please take a look and let me know if there is more I can do. I guess you might want to expand the parts related to UNION ALL views and BRIN indexes. Also for consideration, 0002: some cosmetic fixes to create_table.sgml 0003: add clarification about NOT NULL constraint on partition columns in alter_table.sgml Thoughts? Thanks, Amit >From 97c104054310c9361623c182497d708ed65bf65f Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 3 Mar 2017 16:39:24 +0900 Subject: [PATCH 1/3] Rewrite sections in ddl.sgml related to partitioning Merge sections Partitioned Tables and Partitioning into one section called Table Partitioning and Related Solutions. --- doc/src/sgml/ddl.sgml | 1359 + 1 file changed, 707 insertions(+), 652 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 09b5b3ff70..a2dd39df54 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2772,14 +2772,181 @@ VALUES ('Albany', NULL, NULL, 'NY'); - - Partitioned Tables + + Table Partitioning and Related Solutions + + +partitioning + + + +table +partitioning + partitioned table +PostgreSQL supports basic table +partitioning. This section describes why and how to implement +partitioning as part of your database design. + + + + Overview + + + Partitioning refers to splitting what is logically one large table into + smaller physical pieces. Partitioning can provide several benefits: + + + + Query performance can be improved dramatically in certain situations, + particularly when most of the heavily accessed rows of the table are in a + single partition or a small number of partitions. The partitioning + substitutes for leading columns of indexes, reducing index size and + making it more likely that the heavily-used parts of the indexes + fit in memory. + + + + + + When queries or updates access a large percentage of a single + partition, performance can be improved by taking advantage + of sequential scan of that partition instead of using an + index and random access reads scattered across the whole table. + + + + + + Bulk loads and deletes can be accomplished by adding or removing + partitions, if that requirement is planned into the partitioning design. + Doing ALTER TABLE DETACH PARTITION followed by + DROP TABLE is far faster than a bulk operation. These + commands also entirely avoid the VACUUM overhead + caused by a bulk DELETE. + + + + + + Seldom-used data can be migrated to cheaper and slower storage media. + + + + + The benefits will normally be worthwhile only when a table would + otherwise be very large. The exact point at which a table will + benefit from partitioning depends on the application, although a + rule of thumb is that the size of the table should exceed the physical + memory of the database server. + + + + The following forms of partitioning can be implemented in + PostgreSQL: + + + + Range Partitioning + + + + The table is partitioned into ranges defined + by a key column or set of columns, with no overlap between + the ranges of values assigned to different partitions. For + example one might partition by date ranges, or by ranges of + identifiers for particular business objects. + + + + + + List Partitioning + + + + The table is partitioned by explicitly listing which key values + appear in each partition. + + + + + + + + The following partitioning methods are currently supported: + + + + Declarative Partitioning + + + + One creates a partitioned table by specifying + the partitioning method and a set of columns as the partition key. + Partitions, which contain actual data inserted + into the table, are created by specifying what subset of the data it + accepts. + + + + + + Using inheritance + + + + Each partition must be created as a child table of a single parent + table. The parent table itself is normally empty; it exists just to + represent the entire data set. + + + + + + Using UNION ALL
Re: [HACKERS] Documentation improvements for partitioning
On Mon, Feb 27, 2017 at 5:14 PM, Simon Riggswrote: >> I like the idea of merging what are now two chapters into one and call it >> Partitioned Tables, retaining the text that describes concepts > > +1 > > ...but how? > > 5.10 Partitioned Tables and Related Solutions > 5.10.1 Declarative Partitioning (this new feature) > 5.10.2 Managing Partitions using Inheritance > 5.10.3 Managing Partitions using Union All Views > 5.10.4 Accessing tables using BRIN indexes > > So first and foremost we highlight the new feature and explain all its > strengths with examples. > > We then explain the other possible ways of implementing something > similar. This allows us to explain how to handle cases such as when > partitions have different set of columns etc.. > > I'm happy to put my name down to write the sections on Union All > Views, which is useful but only mentioned in passing, and the section > on BRIN indexes, all of which would have their own independent sets of > caveats. I like the proposed 5.10.1 and 5.10.2 organization. I am not sure whether 5.10.3 and 5.10.4 make sense because I can't quite imagine what content would go there. We don't document UNION ALL views as a method today, and I'm not sure we really need to start. Also I don't see what BRIN indexes have to do with partitioning. But I may be missing something. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 28 February 2017 at 08:14, Amit Langotewrote: > OK. So, I will start writing the patch with above general skeleton and > hopefully post it within this week and you can improve it as fit. Will do, thanks. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/27 20:44, Simon Riggs wrote: > On 27 February 2017 at 10:12, Amit Langote >wrote: > >> I agree that my patch failed to de-emphasize the old partitioning method >> enough. The examples in 5.11 Partitioning chapter also did not highlight >> the new partitioning feature as much as it should have been, so it indeed >> reads like a description of how to avoid using the new partitioning >> feature. > > Your patch was incredibly useful; I just wanted it earlier. Thanks. Anyway, we agree that there is room for improvement. > I think we're probably all agreed that we should highlight benefits of > the new approach more, though the list of caveats should stay > somewhere, just as we did for the original inheritance feature, and > other things such as replication. Yes. >> Should we completely remove details about the older partitioning >> methods? > > No, because there is much code out there using it for last 12 years > that needs to be explained and there are still some use cases where it > is useful that aren't on the roadmap for partitioning. Sure, it's a matter of where we place it in the rewritten section about partitioning. >> I like the idea of merging what are now two chapters into one and call it >> Partitioned Tables, retaining the text that describes concepts > > +1 > > ...but how? > > 5.10 Partitioned Tables and Related Solutions Presumably, this is where we put the "why" of using partitioning as part of database design, that is, some of the text at the beginning of the 5.11 Partitioning section. > 5.10.1 Declarative Partitioning (this new feature) > 5.10.2 Managing Partitions using Inheritance > 5.10.3 Managing Partitions using Union All Views > 5.10.4 Accessing tables using BRIN indexes Each of these sub-sections explain the "how", using that method. We point out here when it's better to use one method over another, limitations when using a particular method, etc. > So first and foremost we highlight the new feature and explain all its > strengths with examples. > > We then explain the other possible ways of implementing something > similar. This allows us to explain how to handle cases such as when > partitions have different set of columns etc.. Yeah, we don't allow that with declarative partitioned tables. A user reading this section should be able to choose the best solution for their needs. > I'm happy to put my name down to write the sections on Union All > Views, which is useful but only mentioned in passing, and the section > on BRIN indexes, all of which would have their own independent sets of > caveats. OK. So, I will start writing the patch with above general skeleton and hopefully post it within this week and you can improve it as fit. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 27/02/17 07:59, Amit Langote wrote: > On 2017/02/27 3:18, Petr Jelinek wrote: >> On 24/02/17 07:15, Robert Haas wrote: >>> On Fri, Feb 24, 2017 at 9:53 AM, Simon Riggswrote: The good news is that logical replication DOES work with partitioning, but only for a Publication with PUBLISH INSERT, pushing from a normal table to a partitioned one. Which is useful enough for first release. The work on having UPDATE work between partitions can be used to make updates and deletes work in logical replication. That might yet be made to work in this release, and I think we should look into it, but I think it can be left til next release if we try. >>> >>> Are you speaking of the case where you want to replicate from an >>> unpartitioned table to a partitioned table? I would imagine that if >>> you are replicating between two identical partitioning hierarchies, it >>> would just work. (If not, that's probably a bug, though I don't know >>> whose bug.) >>> >> >> Yes same hierarchies will work but mainly because one has to add >> partitions themselves to publication currently. I guess that's the >> limitation we might have to live with in 10 as adding the whole >> partitioned table should probably work for different hierarchies when we >> enable it and I am not quite sure that's doable before start of the CF >> at this point. > > If and when we add support to add partitioned tables to publications, I > think it will work by recursing to include the partitions to the same > publication (I see that OpenTableList() in publicationcmds.c calls > find_all_inheritors if recursion is requested by not specifying ONLY). > When a subscription replicates from this publication, it's going to match > changes for individual leaf partitions, not the root parent table. IOW, > none of the changes applied to a partitioned table are replicated as > changes to the table itself. So, it seems that adding a partitioned table > to a publication or subscription would simply be a *shorthand* for adding > all the (leaf) partitions that will actually emit and receive changes. This was my first thought as well. However we need to also take into account the use-case with different partitioning topology on publisher and subscriber (at least in a sense that the initial design will not paint us in the corner). Now I see two ways of achieving this. a) Either going with more or less what you wrote above and in the future have some smarts where we can specify that it should replicate with different name. We'll eventually want to be able to replicate table to differently named table anyway so it's not stretch by any means to do that. b) Or just replicate changes to leaf partitions as changes to partitioned table. That's also not that hard to do, we just need fast lookup of partitioned table from leaf table. I guess a) looks simpler given that we eventually need the rename anyway, but I'd like opinions of other people as well. > I'm not sure but should adding/removing partitions after the fact cause > their addition/removal from the publication (& subscription)? Maybe we'll > discuss these issues later. That's something I've been also wondering as there is many corner cases here. For example if table is in some publication and then is added as partition to partitioned table what should happen? What should happen when the partitioned table is then removed from same publication to which partition was added explicitly? Should we allow different publication configuration for different partitions within same partitioned table, etc? This somewhat brings bigger question about where we want to go in general with partitioning. And that is, should partition be a separate entity that is on the same level of table, or should it be part of the partitioned table without it's own "identity". -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 27 February 2017 at 10:12, Amit Langotewrote: > I agree that my patch failed to de-emphasize the old partitioning method > enough. The examples in 5.11 Partitioning chapter also did not highlight > the new partitioning feature as much as it should have been, so it indeed > reads like a description of how to avoid using the new partitioning > feature. Your patch was incredibly useful; I just wanted it earlier. I think we're probably all agreed that we should highlight benefits of the new approach more, though the list of caveats should stay somewhere, just as we did for the original inheritance feature, and other things such as replication. > Should we completely remove details about the older partitioning > methods? No, because there is much code out there using it for last 12 years that needs to be explained and there are still some use cases where it is useful that aren't on the roadmap for partitioning. > I like the idea of merging what are now two chapters into one and call it > Partitioned Tables, retaining the text that describes concepts +1 ...but how? 5.10 Partitioned Tables and Related Solutions 5.10.1 Declarative Partitioning (this new feature) 5.10.2 Managing Partitions using Inheritance 5.10.3 Managing Partitions using Union All Views 5.10.4 Accessing tables using BRIN indexes So first and foremost we highlight the new feature and explain all its strengths with examples. We then explain the other possible ways of implementing something similar. This allows us to explain how to handle cases such as when partitions have different set of columns etc.. I'm happy to put my name down to write the sections on Union All Views, which is useful but only mentioned in passing, and the section on BRIN indexes, all of which would have their own independent sets of caveats. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/15 12:00, Robert Haas wrote: > On Fri, Feb 10, 2017 at 3:00 AM, Simon Riggswrote: > >> Without claiming I'm happy about this, I think the best way to improve >> the number of eyeballs on this is to commit these docs as is. >> >> For me, the most important thing is understanding the feature, not >> (yet) discussing what the docs should look like. This is especially >> true if other patches reference the way partitioning works and nobody >> can comment on those patches because they don't understand >> >> Any issues with that? > > There are a number of things that I think are awkward about the patch > as committed: > > + > + > +See last section for some general information: > + > + > + > > I think we generally try to write the documentation in such a way as > to minimize backward references, and a backward reference to the > previous section seems particularly odd. We've now got section "5.10 > Partitioned Tables" followed immediately by section "5.11 > Partitioning", where the latter seems to think that you haven't read > the former. > > I think that section 5.11 needs a much heavier rewrite than what it > got as part of this patch. It's a hodgepodge of the old content > (which explained how to fake partitioning when we didn't have an > explicit concept of partitioning) and new content that talks about how > the new way is different from the old way. But now that we have the > new way, I'm guessing that most people are going to use that and not > care about the old way any more. I'm not that it's even appropriate > to keep the lengthy explanation of how to fake table partitioning > using table inheritance and non-overlapping CHECK constraints, but if > we still want that stuff it should be de-emphasized more than it is > here. Probably the section should be retitled: in previous releases > we called this "Partitioning" because we had no explicit notion of > partitioning, but now that we do, it's confusing to have a section > called "Partitioning" that explains how to avoid using the > partitioning feature, which is more or less what this does. Or maybe > the section title should stay the same (or this should be merged into > the previous section?) but rewritten to change the emphasis. I agree that my patch failed to de-emphasize the old partitioning method enough. The examples in 5.11 Partitioning chapter also did not highlight the new partitioning feature as much as it should have been, so it indeed reads like a description of how to avoid using the new partitioning feature. Should we completely remove details about the older partitioning methods? I like the idea of merging what are now two chapters into one and call it Partitioned Tables, retaining the text that describes concepts while getting rid of the texts detailing inheritance implementation. Perhaps with the following sub-sections: 5.10 Partitioned Tables 5.10.1 When To Use Partitioning? (what is now 5.11.1 Overview) 5.10.2 Example (what is now 5.11.2. Implementing Partitioning) 5.10.3 Managing Partitions (same title as 5.11.3) 5.10.4. Partitioning and Constraint Exclusion (same title as 5.11.4) About this, do we want to emphasize the fact that the new partitioned tables *currently* depend on constraint exclusion? I guess not. So the sub-section is retained most as is, with some tweaks. 5.10.5 Caveats (that still are) As mentioned above, the above sub-sections will retain the old text that talks about concepts and not the particular implementation details. For example, in 5.10.3 Managing Partitions, the following text will be retained: Managing Partitions Normally the set of partitions established when initially defining the table are not intended to remain static. It is common to want to remove old partitions of data and periodically add new partitions for new data. One of the most important advantages of partitioning is precisely that it allows this otherwise painful task to be executed nearly instantaneously by manipulating the partition structure, rather than physically moving large amounts of data around. I will go make a patch for this if there are no objections. Suggestions are welcome. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/27 3:18, Petr Jelinek wrote: > On 24/02/17 07:15, Robert Haas wrote: >> On Fri, Feb 24, 2017 at 9:53 AM, Simon Riggswrote: >>> The good news is that logical replication DOES work with partitioning, >>> but only for a Publication with PUBLISH INSERT, pushing from a normal >>> table to a partitioned one. Which is useful enough for first release. >>> >>> The work on having UPDATE work between partitions can be used to make >>> updates and deletes work in logical replication. That might yet be >>> made to work in this release, and I think we should look into it, but >>> I think it can be left til next release if we try. >> >> Are you speaking of the case where you want to replicate from an >> unpartitioned table to a partitioned table? I would imagine that if >> you are replicating between two identical partitioning hierarchies, it >> would just work. (If not, that's probably a bug, though I don't know >> whose bug.) >> > > Yes same hierarchies will work but mainly because one has to add > partitions themselves to publication currently. I guess that's the > limitation we might have to live with in 10 as adding the whole > partitioned table should probably work for different hierarchies when we > enable it and I am not quite sure that's doable before start of the CF > at this point. If and when we add support to add partitioned tables to publications, I think it will work by recursing to include the partitions to the same publication (I see that OpenTableList() in publicationcmds.c calls find_all_inheritors if recursion is requested by not specifying ONLY). When a subscription replicates from this publication, it's going to match changes for individual leaf partitions, not the root parent table. IOW, none of the changes applied to a partitioned table are replicated as changes to the table itself. So, it seems that adding a partitioned table to a publication or subscription would simply be a *shorthand* for adding all the (leaf) partitions that will actually emit and receive changes. I'm not sure but should adding/removing partitions after the fact cause their addition/removal from the publication (& subscription)? Maybe we'll discuss these issues later. By the way, we currently get the following error due to the relkind check in check_publication_add_relation(): create publication p_pub for table p; ERROR: "p" is not a table DETAIL: Only tables can be added to publications. Would it be better to produce an error message that explicitly states that adding partitioned tables to publications is not supported. Something like: create publication p_pub for table p; ERROR: table "p" cannot be replicated DETAIL: Adding partitioned tables to publications is not supported. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 24/02/17 07:15, Robert Haas wrote: > On Fri, Feb 24, 2017 at 9:53 AM, Simon Riggswrote: >> The good news is that logical replication DOES work with partitioning, >> but only for a Publication with PUBLISH INSERT, pushing from a normal >> table to a partitioned one. Which is useful enough for first release. >> >> The work on having UPDATE work between partitions can be used to make >> updates and deletes work in logical replication. That might yet be >> made to work in this release, and I think we should look into it, but >> I think it can be left til next release if we try. > > Are you speaking of the case where you want to replicate from an > unpartitioned table to a partitioned table? I would imagine that if > you are replicating between two identical partitioning hierarchies, it > would just work. (If not, that's probably a bug, though I don't know > whose bug.) > Yes same hierarchies will work but mainly because one has to add partitions themselves to publication currently. I guess that's the limitation we might have to live with in 10 as adding the whole partitioned table should probably work for different hierarchies when we enable it and I am not quite sure that's doable before start of the CF at this point. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Sun, Feb 26, 2017 at 4:31 AM, Bruce Momjianwrote: > I think you are right. I was only guessing on a possible cause of > Simon's reaction since I had the same reaction. When traveling, it is > hard to get excited about reading a 100+ post thread that has reached a > conclusion. I found Simon's summary of the 4 sub-features to be > helpful. OK, no problem. Basically, I think it's a bad plan to redesign this - or add any large amount of incremental change to what's already been done - at this point in the release cycle. Unless we're prepared to rip it all back out, we've got to ship more or less what we have and improve it later. I always viewed the mission of this patch as to set the stage for future improvements in this area, not to solve all of the problems by itself. I'm sorry if anyone was under a contrary impression, and I'm also sorry that the discussion seems to have left some people behind, but I did try my best. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Wed, Feb 22, 2017 at 07:44:41AM +0530, Robert Haas wrote: > On Tue, Feb 21, 2017 at 2:03 AM, Bruce Momjianwrote: > > I have to admit my reaction was similar to Simon's, meaning that the > > lack of docs is a problem, and that the limitations are kind of a > > surprise, and I wonder what other surprises there are. > > Did you read my message upthread pointing out that the initial commit > contained hundreds of lines of documentation? I agree that it would > be bad if table partitioning got committed with no documentation, but > that did not happen. > > > I am thinking this is a result of small teams, often from the same > > company, working on a features in isolation and then making them public. > > It is often not clear what decisions were made and why. > > That also did not happen, or at least certainly not with this patch. > All of the discussion was public and on the mailing list. I never > communicated with Amit Langote off-list about this work, except > shortly before I committed his patches I added him on Skype and gave > him a heads up that I was planning to do so real soon. At no time > have the two of us worked for the same company. Also, the patch had 7 > other reviewers credited in the commit messages spread across, I > think, 4 different companies. I think you are right. I was only guessing on a possible cause of Simon's reaction since I had the same reaction. When traveling, it is hard to get excited about reading a 100+ post thread that has reached a conclusion. I found Simon's summary of the 4 sub-features to be helpful. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Fri, Feb 24, 2017 at 9:53 AM, Simon Riggswrote: > The good news is that logical replication DOES work with partitioning, > but only for a Publication with PUBLISH INSERT, pushing from a normal > table to a partitioned one. Which is useful enough for first release. > > The work on having UPDATE work between partitions can be used to make > updates and deletes work in logical replication. That might yet be > made to work in this release, and I think we should look into it, but > I think it can be left til next release if we try. Are you speaking of the case where you want to replicate from an unpartitioned table to a partitioned table? I would imagine that if you are replicating between two identical partitioning hierarchies, it would just work. (If not, that's probably a bug, though I don't know whose bug.) >> You don't get to show up more than two >> months after the feature is committed and start complaining that it >> doesn't include all the things you want. Which things ought to be >> included in the initial patch was under active discussion about a year >> ago this time. > > I've provided lots of input across many years, for example my > explanation of how to do tuple routing is very clearly the basis of > the design of the current feature. You *knew* I would have feedback > and if it it hadn't appeared yet you knew it would come. Not really. I can't always predict who will take an interest in a patch. I'm not surprised that you have feedback on it, and if that feedback were phrased as "let's try to do X, Y, and Z in future releases" I'd have no problem with it. What I'm reacting against is really the idea that any of this should be done in v10 (and also the idea, which may be an overly defensive reading of your emails, that my original commit was somehow not up to the mark). > The "two months after the feature is committed" is a direct result of > the lack of docs. Note that within hours of reading the docs I'd given > feedback. How could I give feedback earlier? Well, I tried. Quite a few other people managed to give feedback earlier, so it evidently wasn't totally impossible. > I've flown > to Japan to ensure I could talk to Amit in person about this feature. > Your absence at two consecutive developer meetings hasn't helped > there. It is you that needs to show up more. I'm sure we both have > family and work reasons not to attend them. We used to have one developer meeting a year and I have attended it every year I was invited. Then it went up to two per year and I kept attending one of them per year. Now it seems to be three per year and I plan to keep attending one of them per year, unless one of the others happens to be scheduled together with an event I'm planning to attend anyway. As you say, for both family and work reasons, it's hard to commit to doing multiple such events per year. > The need for design feedback is exactly why the docs for logical > replication were published in June, six months before logical > replication was committed. Sure, I think that's great, but there's never been an obligation to write the documentation before the code in the PostgreSQL community. It's not a bad idea and I'm happy if people do it, but I'm not going to start refusing to commit the patches of people who don't do it. Probably 25% of patches have no documentation at all in the first version and that gets added later once some consensus is reached and the feature gets closer to commit; I think that's just fine. For larger features, the documentation often gets expanded after the initial commit; I think that's also fine. Unlike you, I actually don't think it's very hard to follow the discussion about the design of these features in most cases, and it speeds up development if every change in the proposed design doesn't require an adjustment to both the documentation and the code. I think the way logical replication was done was reasonable, and I think the way that partitioning was done was also reasonable. > With repect, you are making a few mistakes. The first is to imagine > that review comments are negative or complaints; with the right > viewpoint they could easily be seen as helping people to see what has > been missed, yet you repeatedly see them as personal attacks and throw > words back. Oh sure, I've done that myself in earlier days. Sure. > The second > is to imagine that discussing things on Hackers in multiple threads, > spanning many months with long, detailed emails and rapid replies is > something that anybody could have followed if they wished. I don't really see how that's a mistake. It might take more time than someone's willing to put in -- I have that problem too, on some threads -- but if someone has the time and is willing to spend it, then they can follow that discussion. > And the > third is to imagine I have no right to review; I will watch and see if > you deploy this same "You don't get to show up.." argument against Tom >
Re: [HACKERS] Documentation improvements for partitioning
On 24 February 2017 at 02:36, Robert Haaswrote: >> While its true that the patch had syntax documentation, there was no >> user design documentation which explained how it worked to allow >> objective review. Had I been able to provide input without reading >> every email message, I would have done so earlier. > > But I don't agree that it was impossible for you to provide input > earlier without reading every email message, nor do I agree that it is > unreasonable to expect to people who want to provide input to read the > relevant threads. >> The features I consider very important in the first release are >> 1. Declarative syntax (we have this!) >> 2. Tuple routing on INSERT/COPY (we have this!) >> 3. O(1) partition elimination for simple = queries >> 4. PRIMARY KEY defined using a) distinct set of UNIQUE constraints on >> each partition, b) partition key >> >> 2 and 3 are intimately connected because they would both use the same >> in-memory data for bsearch, so the code should be almost identical. >> >> 4 is important for Foreign Keys and Logical Replication >> >> As missing items, features 3 and 4 seem achievable in this release, >> potentially in restricted form. > > Simon, this is ridiculous. We're 4 or 5 days away from the start of > the last CommitFest. We have time to fix bugs and improve > documentation and maybe tweak things here and there, but 3 and 4 are > significant development projects. There isn't time to do that stuff > right now and get it right. Agreed, you beat me to it. I've spent the last few hours trying to see what I could personally pull out of the fire to assist. Those things aren't that big, but they are bigger than we have time for now. Had I known about that a few months back... well, we are where we are. The good news is that logical replication DOES work with partitioning, but only for a Publication with PUBLISH INSERT, pushing from a normal table to a partitioned one. Which is useful enough for first release. The work on having UPDATE work between partitions can be used to make updates and deletes work in logical replication. That might yet be made to work in this release, and I think we should look into it, but I think it can be left til next release if we try. > You don't get to show up more than two > months after the feature is committed and start complaining that it > doesn't include all the things you want. Which things ought to be > included in the initial patch was under active discussion about a year > ago this time. I've provided lots of input across many years, for example my explanation of how to do tuple routing is very clearly the basis of the design of the current feature. You *knew* I would have feedback and if it it hadn't appeared yet you knew it would come. The "two months after the feature is committed" is a direct result of the lack of docs. Note that within hours of reading the docs I'd given feedback. How could I give feedback earlier? Well, I tried. I've flown to Japan to ensure I could talk to Amit in person about this feature. Your absence at two consecutive developer meetings hasn't helped there. It is you that needs to show up more. I'm sure we both have family and work reasons not to attend them. The need for design feedback is exactly why the docs for logical replication were published in June, six months before logical replication was committed. With repect, you are making a few mistakes. The first is to imagine that review comments are negative or complaints; with the right viewpoint they could easily be seen as helping people to see what has been missed, yet you repeatedly see them as personal attacks and throw words back. Oh sure, I've done that myself in earlier days. The second is to imagine that discussing things on Hackers in multiple threads, spanning many months with long, detailed emails and rapid replies is something that anybody could have followed if they wished. And the third is to imagine I have no right to review; I will watch and see if you deploy this same "You don't get to show up.." argument against Tom or Noah when they point out holes in late reviews, though we already both know you won't. I see you using that yourself, objecting frequently against patches large and small if they do not meet your exacting standards, yet you have spoken multiple times against my right to do that. Perhaps we'll have time to scowl at each other in India. I'll look forward to that. ;-) -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Fri, Feb 24, 2017 at 8:06 AM, Robert Haaswrote: > Simon, this is ridiculous. We're 4 or 5 days away from the start of > the last CommitFest. We have time to fix bugs and improve > documentation and maybe tweak things here and there, but 3 and 4 are > significant development projects. There isn't time to do that stuff > right now and get it right. You don't get to show up more than two > months after the feature is committed and start complaining that it > doesn't include all the things you want. Which things ought to be > included in the initial patch was under active discussion about a year > ago this time. I take that back. You can complain as much as you like; everybody has a right to complain. But it's not reasonable to expect Amit (or anyone else) to go fix the things you're complaining about in time for v10, or really ever. He doesn't have to write any more partitioning patches ever, and if he does decide to do so, he doesn't have to write the ones you or I or anyone other than his employer wants him to write (and he only has to listen to his employer if he doesn't want to get fired). Also, I'm very much against against any major tinkering with this feature in this release. We've got enough work to do stabilizing what's already been committed in this area, and the last things we need is a bunch of patches that significant change it showing up at the eleventh hour without time for adequate reflection and discussion. Most if not all significant patches for this release should already have been submitted; again, the last CommitFest will be starting shortly, and we should have seen those patches in the previous CommitFest. We should be focusing on getting all the good patches that have already been written committed, not creating new ones at the last minute. Contrary to what you may think, neither changing the way partition pruning works nor inventing a system for indexes to roll down to partition children is a minor fix. Even if you restrict the scope to simple cases, there's still got to be a level of design agreement so that we know we're not boxing ourselves into a corner for the future, and the patch quality still has to be good. That's not going to happen in the next couple of days, barring a dramatic reversal of how the development process in this community has always worked before. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2/23/17 8:36 PM, Robert Haas wrote: We're 4 or 5 days away from the start of the last CommitFest. We have time to fix bugs and improve documentation and maybe tweak things here and there, but 3 and 4 are significant development projects. There isn't time to do that stuff right now and get it right. It might be possible to provide some temporary work-arounds for some of this, which would be nice. But I agree that there's definitely not enough time to implement *good* solutions to even just automatic index creation, let alone somehow handling uniqueness. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Thu, Feb 23, 2017 at 10:00 PM, Simon Riggswrote: > You seem a little defensive about some reasonable review comments. I am prone to that from time to time, and this may be an instance of it. > While its true that the patch had syntax documentation, there was no > user design documentation which explained how it worked to allow > objective review. Had I been able to provide input without reading > every email message, I would have done so earlier. But I don't agree that it was impossible for you to provide input earlier without reading every email message, nor do I agree that it is unreasonable to expect to people who want to provide input to read the relevant threads. > The features I consider very important in the first release are > 1. Declarative syntax (we have this!) > 2. Tuple routing on INSERT/COPY (we have this!) > 3. O(1) partition elimination for simple = queries > 4. PRIMARY KEY defined using a) distinct set of UNIQUE constraints on > each partition, b) partition key > > 2 and 3 are intimately connected because they would both use the same > in-memory data for bsearch, so the code should be almost identical. > > 4 is important for Foreign Keys and Logical Replication > > As missing items, features 3 and 4 seem achievable in this release, > potentially in restricted form. Simon, this is ridiculous. We're 4 or 5 days away from the start of the last CommitFest. We have time to fix bugs and improve documentation and maybe tweak things here and there, but 3 and 4 are significant development projects. There isn't time to do that stuff right now and get it right. You don't get to show up more than two months after the feature is committed and start complaining that it doesn't include all the things you want. Which things ought to be included in the initial patch was under active discussion about a year ago this time. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Thu, Feb 23, 2017 at 11:13 AM, Simon Riggswrote: > My argument was that CREATE INDEX is expected to just work on tables > at present, so should also just work on partitioned tables. Without > that, the reality is people will need to write scripts. Really? What about postgres_fdw? Even if that counter-example didn't exist, I'd still disagree. People may expect that CREATE INDEX should just work, but that expectation is not as general as you suggest. Otherwise, I doubt we'd be talking about it at all. > I don't see how that relates to the desire for multiple index options, > since one of them would need to be the default and we could provide > one in this release, one in the next etc.. You didn't say any of that until now. And besides, I think that global indexes make a lot more sense as a default. You seem to be saying that a simple CREATE INDEX could be interpreted as meaning one or the other of those two behaviors just as easily (global index vs. create an index on each partition). I don't think it's a good idea to try to meet any general "just works" expectation if what you actually get does not fit the intuition of almost all users. "Just don't show me an error" seems like a bad design goal, especially for a utility statement. > The current design has assumed many things, leading me to question > what else has been assumed. > > Challenging those assumptions is important and has been upheld. I agree. The irony is that in so doing, you yourself make your own assumptions, confusing everyone, and making it harder to act on your feedback. You did make some reasonable points, IMV. > I've seen many patches rejected because they do not contain the > desired feature set, yet. Obviously that general principle is not under discussion. My point, of course, was that it seems pretty clear to me that this is on the right side of that fence. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 23 February 2017 at 17:27, Peter Geogheganwrote: > On Thu, Feb 23, 2017 at 8:08 AM, Simon Riggs wrote: >> What claims are you talking about? Which things have been exaggerated, >> and by whom? > > * The specious argument that we should "just" have CREATE INDEX create > equivalent indexes across partitions, to save all those people from > writing all those scripts. > > The reality is that there are several ways that that would not comport > with the existing design of things. Most obviously: where does that > leave the design of global indexes that we eventually come up with? > Now, maybe there is a good way to make this easier for users, worth > doing sooner rather than later, but that will be subtle, and you > should at least acknowledge that. My argument was that CREATE INDEX is expected to just work on tables at present, so should also just work on partitioned tables. Without that, the reality is people will need to write scripts. I don't see how that relates to the desire for multiple index options, since one of them would need to be the default and we could provide one in this release, one in the next etc.. > * "It leaves me asking what else is missing"... "If we wanted them to > act identically we wouldn't have any need for a new feature at all, so > clearly that doesn't make sense as an argument." > > These remarks sound imperious to me. I think that this could be quite > demoralizing to someone in Amit's position, and you ought to give some > consideration to that. I think that several of your remarks on the > patch are facile and/or needlessly ambiguous, which is what makes this > lack of tact seem unfair to me. The current design has assumed many things, leading me to question what else has been assumed. Challenging those assumptions is important and has been upheld. I agree my review comments could well be demoralizing, which is why I said "Good work so far". It takes a while to realise that review comments are given to patch authors with the intent to help improve the product, not as personal attacks. I thought you would know that by now. Imperious? No, definitely a Jedi. > * "Good work so far, but there is still a very significant amount of > work to do." > > There is always more work to do, so why say so? I think that the > implication is that this isn't complete as a feature that goes into > the next release, which I disagree with. I've seen many patches rejected because they do not contain the desired feature set, yet. ISTM my opinion on when that is reached is as valid as yours or anyone else's, so I'm unclear as to your issue. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 02/23/2017 09:27 AM, Peter Geoghegan wrote: On Thu, Feb 23, 2017 at 8:08 AM, Simon Riggswrote: * "Good work so far, but there is still a very significant amount of work to do." There is always more work to do, so why say so? I think that the implication is that this isn't complete as a feature that goes into the next release, which I disagree with. There is nothing disappointing to me about this feature, and, as I said, I am unsurprised that it doesn't support certain things. I don't think we need to start going down the avenue of "you could be nicer". We can all be nicer and we all have our good and bad days. If we start worrying about egos to this degree, we will never get anything done. JD -- Command Prompt, Inc. http://the.postgres.company/ +1-503-667-4564 PostgreSQL Centered full stack support, consulting and development. Everyone appreciates your honesty, until you are honest with them. Unless otherwise stated, opinions are my own. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Thu, Feb 23, 2017 at 8:08 AM, Simon Riggswrote: > What claims are you talking about? Which things have been exaggerated, > and by whom? * The specious argument that we should "just" have CREATE INDEX create equivalent indexes across partitions, to save all those people from writing all those scripts. The reality is that there are several ways that that would not comport with the existing design of things. Most obviously: where does that leave the design of global indexes that we eventually come up with? Now, maybe there is a good way to make this easier for users, worth doing sooner rather than later, but that will be subtle, and you should at least acknowledge that. * "It leaves me asking what else is missing"... "If we wanted them to act identically we wouldn't have any need for a new feature at all, so clearly that doesn't make sense as an argument." These remarks sound imperious to me. I think that this could be quite demoralizing to someone in Amit's position, and you ought to give some consideration to that. I think that several of your remarks on the patch are facile and/or needlessly ambiguous, which is what makes this lack of tact seem unfair to me. * "Good work so far, but there is still a very significant amount of work to do." There is always more work to do, so why say so? I think that the implication is that this isn't complete as a feature that goes into the next release, which I disagree with. There is nothing disappointing to me about this feature, and, as I said, I am unsurprised that it doesn't support certain things. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 22 February 2017 at 02:14, Robert Haaswrote: > On Tue, Feb 21, 2017 at 2:03 AM, Bruce Momjian wrote: >> I have to admit my reaction was similar to Simon's, meaning that the >> lack of docs is a problem, and that the limitations are kind of a >> surprise, and I wonder what other surprises there are. > > Did you read my message upthread pointing out that the initial commit > contained hundreds of lines of documentation? I agree that it would > be bad if table partitioning got committed with no documentation, but > that did not happen. You seem a little defensive about some reasonable review comments. While its true that the patch had syntax documentation, there was no user design documentation which explained how it worked to allow objective review. Had I been able to provide input without reading every email message, I would have done so earlier. Amit, The features I consider very important in the first release are 1. Declarative syntax (we have this!) 2. Tuple routing on INSERT/COPY (we have this!) 3. O(1) partition elimination for simple = queries 4. PRIMARY KEY defined using a) distinct set of UNIQUE constraints on each partition, b) partition key 2 and 3 are intimately connected because they would both use the same in-memory data for bsearch, so the code should be almost identical. 4 is important for Foreign Keys and Logical Replication As missing items, features 3 and 4 seem achievable in this release, potentially in restricted form. I think we should probably avoid trying to UPDATE rows from one partition to another in this release, since that seems likely to be buggy and seems like would only be needed in relatively few cases. Let me know if I can help with these. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 22 February 2017 at 19:57, Peter Geogheganwrote: > FWIW, I agree that some of what has been claimed about what > contributors failed to do with this patch is exaggerated, and not in a > way that I'd understand as hyperbole that drives home a deeper point. What claims are you talking about? Which things have been exaggerated, and by whom? -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Tue, Feb 21, 2017 at 6:14 PM, Robert Haaswrote: > On Tue, Feb 21, 2017 at 2:03 AM, Bruce Momjian wrote: >> I have to admit my reaction was similar to Simon's, meaning that the >> lack of docs is a problem, and that the limitations are kind of a >> surprise, and I wonder what other surprises there are. > > Did you read my message upthread pointing out that the initial commit > contained hundreds of lines of documentation? I agree that it would > be bad if table partitioning got committed with no documentation, but > that did not happen. > >> I am thinking this is a result of small teams, often from the same >> company, working on a features in isolation and then making them public. >> It is often not clear what decisions were made and why. > > That also did not happen, or at least certainly not with this patch. > All of the discussion was public and on the mailing list. FWIW, I agree that some of what has been claimed about what contributors failed to do with this patch is exaggerated, and not in a way that I'd understand as hyperbole that drives home a deeper point. I'm not the slightest bit surprised at the limitations that this feature has, even if Bruce and Simon are. The documentation needs work, and perhaps the feature itself needs a small tweak here or there. Just not to a particularly notable degree, given the point we are in in the release cycle. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Tue, Feb 21, 2017 at 10:27 PM, Yugo Nagatawrote: > There is a very small typo that a comma is missing. > Attached is the patch to fix it. Thank you. I have committed your patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Tue, Feb 21, 2017 at 2:03 AM, Bruce Momjianwrote: > I have to admit my reaction was similar to Simon's, meaning that the > lack of docs is a problem, and that the limitations are kind of a > surprise, and I wonder what other surprises there are. Did you read my message upthread pointing out that the initial commit contained hundreds of lines of documentation? I agree that it would be bad if table partitioning got committed with no documentation, but that did not happen. > I am thinking this is a result of small teams, often from the same > company, working on a features in isolation and then making them public. > It is often not clear what decisions were made and why. That also did not happen, or at least certainly not with this patch. All of the discussion was public and on the mailing list. I never communicated with Amit Langote off-list about this work, except shortly before I committed his patches I added him on Skype and gave him a heads up that I was planning to do so real soon. At no time have the two of us worked for the same company. Also, the patch had 7 other reviewers credited in the commit messages spread across, I think, 4 different companies. I think the issue here might be that with this feature, as with some other features I've committed over the last few years, the email discussion got very long. On the one hand, that does make it hard for others to keep up, but not because anything is secret, only because reading hundreds of email messages takes a lot of time. However, the large number of emails on a public mailing list makes it absolutely clear that this wasn't developed in isolation and presented as a done deal. It was written and rewritten multiple times in response to feedback, not only from me but from other people who did take the time to keep up with the discussion. As Ashutosh Bapat and Amit Langote already pointed out, I even posted (on a separate thread with a clear subject line) some thoughts about the overall design of this feature, in response to concerns articulated on an unrelated thread by Tom and Alvaro. I did that in an attempt to give people a separate thread on which to discuss those issues - without having to dive into the main thread where things were being hashed out in detail - but it got no responses, either because the logic was unarguable or because nobody took the time to write back. I think there's certainly room to criticize cases where a feature is designed, developed, and committed almost entirely by people who work at a single company, or where a significant amount of discussion happens off-list, but it would be difficult to find a more clear-cut case of where that DIDN'T happen than this patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
Hi, There is a very small typo that a comma is missing. Attached is the patch to fix it. On Wed, 15 Feb 2017 07:57:53 -0500 Robert Haaswrote: > On Wed, Feb 15, 2017 at 4:26 AM, Ashutosh Bapat > wrote: > > Noticed some typos in the documentation. Here's patch to correct > > those. Sorry, if it has been already taken care of. > > Thanks. That is indeed nonstandard capitalization. Committed. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Yugo Nagata diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 5779eac..ef0f7cf 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2899,7 +2899,7 @@ VALUES ('Albany', NULL, NULL, 'NY'); - Since primary keys are not supported on partitioned tables + Since primary keys are not supported on partitioned tables, foreign keys referencing partitioned tables are not supported, nor are foreign key references from a partitioned table to some other table. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Tue, Feb 21, 2017 at 9:57 AM, Amit Langotewrote: > On 2017/02/21 12:10, Ashutosh Bapat wrote: >> I think, that's a limitation till we implement global indexes. But >> nothing in the current implementation stops us from implementing it. >> In fact, I remember, a reply from Robert to another thread on >> partitioning started in parallel to Amit's thread had explained some >> of these design decisions. I am unable to find link to that exact >> reply though. > > Are you perhaps thinking of the message titled "design for a partitioning > feature (was: inheritance)" a few months back: > > https://www.postgresql.org/message-id/ca+tgmozev0nryvw9cnb81xdojg4xtpjmkdif0zto-gdlcoc...@mail.gmail.com Thanks a lot, that's the one I was searching for. And now that I confirm it, there's NO reply to Robert's mail. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/21 12:10, Ashutosh Bapat wrote: > I think, that's a limitation till we implement global indexes. But > nothing in the current implementation stops us from implementing it. > In fact, I remember, a reply from Robert to another thread on > partitioning started in parallel to Amit's thread had explained some > of these design decisions. I am unable to find link to that exact > reply though. Are you perhaps thinking of the message titled "design for a partitioning feature (was: inheritance)" a few months back: https://www.postgresql.org/message-id/ca+tgmozev0nryvw9cnb81xdojg4xtpjmkdif0zto-gdlcoc...@mail.gmail.com Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Tue, Feb 21, 2017 at 2:03 AM, Bruce Momjianwrote: > On Mon, Feb 20, 2017 at 02:37:44PM +0530, Robert Haas wrote: >> On Mon, Feb 20, 2017 at 2:09 AM, Simon Riggs wrote: >> > On 15 February 2017 at 15:46, Robert Haas wrote: >> >>> It leaves me asking what else is missing. >> >> >> >> There is certainly a lot of room for improvement here but I don't >> >> understand your persistent negativity about what's been done thus far. >> >> I think it's pretty clearly a huge step forward, and I think Amit >> >> deserves a ton of credit for making it happen. The improvements in >> >> bulk loading performance alone are stupendous. You apparently have >> >> the idea that somebody could have written an even larger patch that >> >> solved even more problems at once, but this was already a really big >> >> patch, and IMHO quite a good one. >> > >> > Please explain these personal comments against me. >> >> Several of your emails, including your first post to this thread, >> seemed to me to be quite negative about the state of this feature. I >> don't think that's warranted, though perhaps I am misreading your >> tone, as I have been known to do. I also don't think that expressing >> the opinion that the feature is better than you're giving it credit >> for is a personal comment against you. Where exactly do you see a >> personal comment against you in what I wrote? > > I have to admit my reaction was similar to Simon's, meaning that the > lack of docs is a problem, and that the limitations are kind of a > surprise, and I wonder what other surprises there are. > > I am thinking this is a result of small teams, often from the same > company, working on a features in isolation and then making them public. I agree that this is result of small teams. The partitioning feature encompasses features like global indexes, which is large in themselves. Usually, in a company many teams would be working on different sub-features in the same release schedule. But that's not the case here. We have to add sub-features in multiple releases. That might be the reason behind some of the current limitations. > It is often not clear what decisions were made and why. Amit Langote submitted the patch sometime in August 2015, which certainly didn't look like a well-thought design, certainly not a product of 'long cooking' within his company. It was more experimental. (Obviously he had background of many earlier discussions on partitioning, that all happened on hackers.) Since then all the discussion is on hackers; all decisions were made during the discussion. While what you are saying may be true with other patches, I am not sure whether it's true with this work. > The idea that > unique indexes on a parent table can't guarantee uniqueness across child > tables is both a surprise, and obvious once stated. I think, that's a limitation till we implement global indexes. But nothing in the current implementation stops us from implementing it. In fact, I remember, a reply from Robert to another thread on partitioning started in parallel to Amit's thread had explained some of these design decisions. I am unable to find link to that exact reply though. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/16 10:45, Amit Langote wrote: > Also attaching 0002 (unchanged) for tab-completion support for the new > partitioning syntax. Robert already spawned a new thread titled "tab completion for partitioning" for this [0]. > 0003 changes how ExecFindPartition() shows the row for which > get_partition_for_tuple() failed to find a partition. As Simon commented > upthread, we should show just the partition key, not the whole row in the > error DETAIL. So the DETAIL now looks like what's shown by > _bt_check_unique() upon uniqueness violation: > > DETAIL: Partition key of the failing row contains (key1, key2, ...)=(val1, > val2, ...) > > The rules about which columns to show or whether to show the DETAIL at all > are similar to those in BuildIndexValueDescription(): > > - if user has SELECT privilege on the whole table, simply go ahead > > - if user doesn't have SELECT privilege on the table, check that they >can see all the columns in the key (no point in showing partial key); >however abort on finding an expression for which we don't try finding >out privilege situation of whatever columns may be in the expression I posted this patch in a new thread titled "error detail when partition not found" [1]. Thanks, Amit [0] https://www.postgresql.org/message-id/CA+TgmobYOj=a8gesies_v2wq46-_w0+7mowpinwc+iuzj-u...@mail.gmail.com [1] https://www.postgresql.org/message-id/9f9dc7ae-14f0-4a25-5485-964d9bfc19bd%40lab.ntt.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Mon, Feb 20, 2017 at 02:37:44PM +0530, Robert Haas wrote: > On Mon, Feb 20, 2017 at 2:09 AM, Simon Riggswrote: > > On 15 February 2017 at 15:46, Robert Haas wrote: > >>> It leaves me asking what else is missing. > >> > >> There is certainly a lot of room for improvement here but I don't > >> understand your persistent negativity about what's been done thus far. > >> I think it's pretty clearly a huge step forward, and I think Amit > >> deserves a ton of credit for making it happen. The improvements in > >> bulk loading performance alone are stupendous. You apparently have > >> the idea that somebody could have written an even larger patch that > >> solved even more problems at once, but this was already a really big > >> patch, and IMHO quite a good one. > > > > Please explain these personal comments against me. > > Several of your emails, including your first post to this thread, > seemed to me to be quite negative about the state of this feature. I > don't think that's warranted, though perhaps I am misreading your > tone, as I have been known to do. I also don't think that expressing > the opinion that the feature is better than you're giving it credit > for is a personal comment against you. Where exactly do you see a > personal comment against you in what I wrote? I have to admit my reaction was similar to Simon's, meaning that the lack of docs is a problem, and that the limitations are kind of a surprise, and I wonder what other surprises there are. I am thinking this is a result of small teams, often from the same company, working on a features in isolation and then making them public. It is often not clear what decisions were made and why. The idea that unique indexes on a parent table can't guarantee uniqueness across child tables is both a surprise, and obvious once stated. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Wed, Feb 15, 2017 at 12:08:19PM -0500, Robert Haas wrote: > On Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrera >wrote: > > I think new-style partitioning is supposed to consider each partition as > > an implementation detail of the table; the fact that you can manipulate > > partitions separately does not really mean that they are their own > > independent object. You don't stop to think "do I really want to drop > > the TOAST table attached to this main table?" and attach a CASCADE > > clause if so. You just drop the main table, and the toast one is > > dropped automatically. I think new-style partitions should behave > > equivalently. > > That's a reasonable point of view. I'd like to get some more opinions > on this topic. I'm happy to have us do whatever most people want, but > I'm worried that having table inheritance and table partitioning work > differently will be create confusion. I'm also suspicious that there > may be some implementation difficulties. On the hand, it does seem a > little silly to say that DROP TABLE partitioned_table should always > fail except in the degenerate case where there are no partitions, so > maybe changing it is for the best. I think we have a precedent for this, and that is the SERIAL data type, which is really just a macro on top of CREATE SEQUENCE and DEFAULT nextval() using the sequence. You can manipulate the sequence and the DEFAULT separately, but if you drop the table the sequence is dropped to automatically. This seems like an instructive example of how we have bundled behavior together in the past in a logical and easy-to-understand way. Of course, their might be some technical limitations that prevent us from using this approach, and I would be interested in hearing those details. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Mon, Feb 20, 2017 at 2:09 AM, Simon Riggswrote: > On 15 February 2017 at 15:46, Robert Haas wrote: >>> It leaves me asking what else is missing. >> >> There is certainly a lot of room for improvement here but I don't >> understand your persistent negativity about what's been done thus far. >> I think it's pretty clearly a huge step forward, and I think Amit >> deserves a ton of credit for making it happen. The improvements in >> bulk loading performance alone are stupendous. You apparently have >> the idea that somebody could have written an even larger patch that >> solved even more problems at once, but this was already a really big >> patch, and IMHO quite a good one. > > Please explain these personal comments against me. Several of your emails, including your first post to this thread, seemed to me to be quite negative about the state of this feature. I don't think that's warranted, though perhaps I am misreading your tone, as I have been known to do. I also don't think that expressing the opinion that the feature is better than you're giving it credit for is a personal comment against you. Where exactly do you see a personal comment against you in what I wrote? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/20 1:04, Robert Haas wrote: > On Thu, Feb 16, 2017 at 12:43 PM, Amit Langote wrote: >> So I count more than a few votes saying that we should be able to DROP >> partitioned tables without specifying CASCADE. >> >> I tried to implement that using the attached patch by having >> StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between >> parent and child if the child is a partition, instead of DEPENDENCY_NORMAL >> that would otherwise be created. Now it seems that that is one way of >> making sure that partitions are dropped when the root partitioned table is >> dropped, not sure if the best; why create the pg_depend entries at all one >> might ask. I chose it for now because that's the one with fewest lines of >> change. Adjusted regression tests as well, since we recently tweaked >> tests [1] to work around the irregularities of test output when using >> CASCADE. > > Could you possibly post this on a new thread with a reference back to > this one? The number of patches on this one is getting a bit hard to > track, and some people may be under the misimpression that this one is > just about documentation. Sorry about that. Sent the above message and the patch in a new thread titled "dropping partitioned tables without CASCADE" [1]. Thanks, Amit [1] https://postgr.es/m/6c420206-45d7-3f56-8325-4bd7b76483ba%40lab.ntt.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 15 February 2017 at 15:46, Robert Haaswrote: >> It leaves me asking what else is missing. > > There is certainly a lot of room for improvement here but I don't > understand your persistent negativity about what's been done thus far. > I think it's pretty clearly a huge step forward, and I think Amit > deserves a ton of credit for making it happen. The improvements in > bulk loading performance alone are stupendous. You apparently have > the idea that somebody could have written an even larger patch that > solved even more problems at once, but this was already a really big > patch, and IMHO quite a good one. Please explain these personal comments against me. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Thu, Feb 16, 2017 at 12:43 PM, Amit Langotewrote: > On 2017/02/16 2:08, Robert Haas wrote: >> On Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrera >> wrote: >>> I think new-style partitioning is supposed to consider each partition as >>> an implementation detail of the table; the fact that you can manipulate >>> partitions separately does not really mean that they are their own >>> independent object. You don't stop to think "do I really want to drop >>> the TOAST table attached to this main table?" and attach a CASCADE >>> clause if so. You just drop the main table, and the toast one is >>> dropped automatically. I think new-style partitions should behave >>> equivalently. >> >> That's a reasonable point of view. I'd like to get some more opinions >> on this topic. I'm happy to have us do whatever most people want, but >> I'm worried that having table inheritance and table partitioning work >> differently will be create confusion. I'm also suspicious that there >> may be some implementation difficulties. On the hand, it does seem a >> little silly to say that DROP TABLE partitioned_table should always >> fail except in the degenerate case where there are no partitions, so >> maybe changing it is for the best. > > So I count more than a few votes saying that we should be able to DROP > partitioned tables without specifying CASCADE. > > I tried to implement that using the attached patch by having > StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between > parent and child if the child is a partition, instead of DEPENDENCY_NORMAL > that would otherwise be created. Now it seems that that is one way of > making sure that partitions are dropped when the root partitioned table is > dropped, not sure if the best; why create the pg_depend entries at all one > might ask. I chose it for now because that's the one with fewest lines of > change. Adjusted regression tests as well, since we recently tweaked > tests [1] to work around the irregularities of test output when using CASCADE. Could you possibly post this on a new thread with a reference back to this one? The number of patches on this one is getting a bit hard to track, and some people may be under the misimpression that this one is just about documentation. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Thu, Feb 16, 2017 at 7:15 AM, Amit Langotewrote: >> I think 0001 is better than the status quo, but I'm wondering whether >> we should try to do something slightly different. Maybe it should >> always work for the child table to specify neither WITH OIDS nor >> WITHOUT OIDS, but if you do specify one of them then it has to be the >> one that matches the parent partitioned table? With this patch, IIUC, >> WITH OIDS is allowed only if the parent has the same, but WITHOUT OIDS >> is allowed (but ignored) regardless of the parent setting. > > With the patch, one can always specify (or not) WITH/WITHOUT OIDS when > creating partitions. If WITH OIDS is specified and the parent doesn't > have OIDs, then an error is raised. Then just like with normal > inheritance, WITHOUT OIDS specification for a partition will be > *overridden* if the parent has OIDs. By the way, CREATE TABLE page says > this about inheritance and OIDS: > > (If the new table inherits from any tables that have OIDs, then > OIDS=TRUE is forced even if the command says > OIDS=FALSE. > > Hopefully it's clear to someone reading "If the table inherits from any > tables ..." that it also refers to creating partition of a partitioned table. I rewrote this to be a bit more explicit and committed it. I noticed that there was some duplication here: the old text said both this: A partition cannot have columns other than those inherited from the parent. And also this just a little later in the same page: A partition must have the same column names and types as the table of which it is a partition. The second wording seemed better, so I moved that statement a little higher up and rejiggered the wording to be super-clear about OIDs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Fri, Feb 17, 2017 at 11:25 AM, Ashutosh Bapatwrote: > Forgot to attach the patch. Thanks Rajkumar for notifying me. I think this is overexplaining what is anyway obvious. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
Forgot to attach the patch. Thanks Rajkumar for notifying me. On Fri, Feb 17, 2017 at 11:18 AM, Ashutosh Bapatwrote: > In the documentation of ALTER TABLE ... ATTACH PARTITION its implicit > that partition name specified should be the name of the existing table > being attached. Same is the case with DETACH PARTITION where its > implicit that the detached partition becomes a stand-alone table with > same name as the partition being detached. I think this needs to be > more explicit. PFA patch on those lines. > > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company alter_table_part_doc.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
In the documentation of ALTER TABLE ... ATTACH PARTITION its implicit that partition name specified should be the name of the existing table being attached. Same is the case with DETACH PARTITION where its implicit that the detached partition becomes a stand-alone table with same name as the partition being detached. I think this needs to be more explicit. PFA patch on those lines. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/16 2:08, Robert Haas wrote: > On Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrera >wrote: >> I think new-style partitioning is supposed to consider each partition as >> an implementation detail of the table; the fact that you can manipulate >> partitions separately does not really mean that they are their own >> independent object. You don't stop to think "do I really want to drop >> the TOAST table attached to this main table?" and attach a CASCADE >> clause if so. You just drop the main table, and the toast one is >> dropped automatically. I think new-style partitions should behave >> equivalently. > > That's a reasonable point of view. I'd like to get some more opinions > on this topic. I'm happy to have us do whatever most people want, but > I'm worried that having table inheritance and table partitioning work > differently will be create confusion. I'm also suspicious that there > may be some implementation difficulties. On the hand, it does seem a > little silly to say that DROP TABLE partitioned_table should always > fail except in the degenerate case where there are no partitions, so > maybe changing it is for the best. So I count more than a few votes saying that we should be able to DROP partitioned tables without specifying CASCADE. I tried to implement that using the attached patch by having StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between parent and child if the child is a partition, instead of DEPENDENCY_NORMAL that would otherwise be created. Now it seems that that is one way of making sure that partitions are dropped when the root partitioned table is dropped, not sure if the best; why create the pg_depend entries at all one might ask. I chose it for now because that's the one with fewest lines of change. Adjusted regression tests as well, since we recently tweaked tests [1] to work around the irregularities of test output when using CASCADE. Thanks, Amit [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c397814 >From 3b75f82b4f47e840074d0209323d8ab1f39ed676 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 16 Feb 2017 15:56:44 +0900 Subject: [PATCH] Allow dropping partitioned table without CASCADE Currently, a normal dependency is created between a inheritance parent and child when creating the child. That means one must specify CASCADE to drop the parent table if a child table exists. When creating partitions as inheritance children, create auto dependency instead, so that partitions are dropped automatically when the parent is dropped i.e., without specifying CASCADE. --- src/backend/commands/tablecmds.c | 26 ++ src/test/regress/expected/alter_table.out | 10 -- src/test/regress/expected/create_table.out | 9 ++--- src/test/regress/expected/inherit.out | 18 -- src/test/regress/expected/insert.out | 7 ++- src/test/regress/expected/update.out | 5 - src/test/regress/sql/alter_table.sql | 10 -- src/test/regress/sql/create_table.sql | 9 ++--- src/test/regress/sql/insert.sql| 7 ++- 9 files changed, 34 insertions(+), 67 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f33aa70da6..27b6556a71 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -289,9 +289,11 @@ static List *MergeAttributes(List *schema, List *supers, char relpersistence, static bool MergeCheckConstraint(List *constraints, char *name, Node *expr); static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel); static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel); -static void StoreCatalogInheritance(Oid relationId, List *supers); +static void StoreCatalogInheritance(Oid relationId, List *supers, + bool child_is_partition); static void StoreCatalogInheritance1(Oid relationId, Oid parentOid, - int16 seqNumber, Relation inhRelation); + int16 seqNumber, Relation inhRelation, + bool child_is_partition); static int findAttrByName(const char *attributeName, List *schema); static void AlterIndexNamespaces(Relation classRel, Relation rel, Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved); @@ -730,7 +732,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, typaddress); /* Store inheritance information for new rel. */ - StoreCatalogInheritance(relationId, inheritOids); + StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL); /* * We must bump the command counter to make the newly-created relation @@ -2245,7 +2247,8 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr) * supers is a list of the OIDs of the new relation's direct ancestors. */ static void -StoreCatalogInheritance(Oid relationId, List *supers) +StoreCatalogInheritance(Oid
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/15 13:31, Robert Haas wrote: > On Mon, Feb 13, 2017 at 5:57 AM, Amit Langote >wrote: >> On 2017/02/13 14:21, Amit Langote wrote: >>> On 2017/02/10 19:19, Simon Riggs wrote: * The OID inheritance needs work - you shouldn't need to specify a partition needs OIDS if the parent has it already. >>> >>> That sounds right. It's better to keep the behavior same as for regular >>> inheritance. Will post a patch to get rid of the unnecessary check. >>> >>> FWIW, the check was added to prevent the command from succeeding in the >>> case where WITHOUT OIDS has been specified for a partition and the parent >>> partitioned table has the OID column. Regular inheritance simply >>> *overrides* the WITHOUT OIDS specification, which might be seen as >>> surprising. >> >> 0001 of the attached patches takes care of this. > > I think 0001 needs to remove this hunk of documentation: > > > > If the partitioned table specified WITH OIDS then > each partition must also specify WITH OIDS. Oids > are not automatically inherited by partitions. > > Attached updated 0001 which does that. > I think 0001 is better than the status quo, but I'm wondering whether > we should try to do something slightly different. Maybe it should > always work for the child table to specify neither WITH OIDS nor > WITHOUT OIDS, but if you do specify one of them then it has to be the > one that matches the parent partitioned table? With this patch, IIUC, > WITH OIDS is allowed only if the parent has the same, but WITHOUT OIDS > is allowed (but ignored) regardless of the parent setting. With the patch, one can always specify (or not) WITH/WITHOUT OIDS when creating partitions. If WITH OIDS is specified and the parent doesn't have OIDs, then an error is raised. Then just like with normal inheritance, WITHOUT OIDS specification for a partition will be *overridden* if the parent has OIDs. By the way, CREATE TABLE page says this about inheritance and OIDS: (If the new table inherits from any tables that have OIDs, then OIDS=TRUE is forced even if the command says OIDS=FALSE. Hopefully it's clear to someone reading "If the table inherits from any tables ..." that it also refers to creating partition of a partitioned table. Also attaching 0002 (unchanged) for tab-completion support for the new partitioning syntax. 0003 changes how ExecFindPartition() shows the row for which get_partition_for_tuple() failed to find a partition. As Simon commented upthread, we should show just the partition key, not the whole row in the error DETAIL. So the DETAIL now looks like what's shown by _bt_check_unique() upon uniqueness violation: DETAIL: Partition key of the failing row contains (key1, key2, ...)=(val1, val2, ...) The rules about which columns to show or whether to show the DETAIL at all are similar to those in BuildIndexValueDescription(): - if user has SELECT privilege on the whole table, simply go ahead - if user doesn't have SELECT privilege on the table, check that they can see all the columns in the key (no point in showing partial key); however abort on finding an expression for which we don't try finding out privilege situation of whatever columns may be in the expression Thanks, Amit >From 1f249c0a395d18532c470ebe4912e33aa61409fd Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 13 Feb 2017 13:32:18 +0900 Subject: [PATCH 1/3] Inherit OID system column automatically for partitions Currently, WITH OIDS must be explicitly specified when creating a partition if the parent table has the OID system column. Instead, inherit it automatically, possibly overriding any explicit WITHOUT OIDS specification. Per review comment from Simon Riggs --- doc/src/sgml/ddl.sgml | 8 doc/src/sgml/ref/create_table.sgml | 12 +--- src/backend/commands/tablecmds.c | 21 - src/test/regress/expected/create_table.out | 18 +- src/test/regress/sql/create_table.sql | 10 ++ 5 files changed, 32 insertions(+), 37 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index f909242e4c..5779eac43d 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2863,14 +2863,6 @@ VALUES ('Albany', NULL, NULL, 'NY'); - If the partitioned table specified WITH OIDS then - each partition must also specify WITH OIDS. Oids - are not automatically inherited by partitions. - - - - - One cannot drop a NOT NULL constraint on a partition's column, if the constraint is present in the parent table. diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index e0f7cd9b93..87a3443ee2 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -301,13 +301,11 @@
Re: [HACKERS] Documentation improvements for partitioning
Alvaro Herrerawrites: > Robert Haas wrote: >> True. I think the question here is: do we want to view the dependency >> between a partitioned table and a partition of that table as >> DEPENDENCY_NORMAL or as DEPENDENCY_AUTO? With table inheritance, it's >> always been "normal" and I'm not sure there's any good reason for >> partitioning to make the opposite decision. > I think new-style partitioning is supposed to consider each partition as > an implementation detail of the table; the fact that you can manipulate > partitions separately does not really mean that they are their own > independent object. You don't stop to think "do I really want to drop > the TOAST table attached to this main table?" and attach a CASCADE > clause if so. You just drop the main table, and the toast one is > dropped automatically. I think new-style partitions should behave > equivalently. I agree with Alvaro's position. If you need CASCADE to get rid of the individual partitions, that's going to be a serious usability fail. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrerawrote: > I think new-style partitioning is supposed to consider each partition as > an implementation detail of the table; the fact that you can manipulate > partitions separately does not really mean that they are their own > independent object. You don't stop to think "do I really want to drop > the TOAST table attached to this main table?" and attach a CASCADE > clause if so. You just drop the main table, and the toast one is > dropped automatically. I think new-style partitions should behave > equivalently. That's a reasonable point of view. I'd like to get some more opinions on this topic. I'm happy to have us do whatever most people want, but I'm worried that having table inheritance and table partitioning work differently will be create confusion. I'm also suspicious that there may be some implementation difficulties. On the hand, it does seem a little silly to say that DROP TABLE partitioned_table should always fail except in the degenerate case where there are no partitions, so maybe changing it is for the best. > Now that partitions are declarative, the underlying implementation could > change away from inheritance. It's now just an implementation artifact. I don't really agree with that. It's true that, for example, we could decide to store the inheritance information for partitions someplace other than pg_inherits, but from a user perspective these are always going to be closely-related features. Also, there are quite a number of ways in which it's very noticeable that the objects are separate. They are dumped separately. They have separate indexes, and even if we provide some facility to create a common indexing scheme across all partitions automatically, you'll still be able to REINDEX or CLUSTER one of those tables individually. They can have different storage properties, like one can be UNLOGGED while another is not. They show up in EXPLAIN plans. The partitioning structure affects what you can and can't do with foreign keys. All of those are user-visible things that make this look and feel like a collection of tables, not just a single table that happens to have several relfilenodes under the hood. I think that's actually a really important feature, not a design defect. As you may or may not know, EDB has had a proprietary implementation of table partitioning since Advanced Server 9.1, and one of the things we've learned from that implementation is that users really like to be able to fiddle with the individual partitions. They want to things like make them individually unlogged, rename them, vacuum them, add contraints, add triggers, put them in different tablespaces, vary indexing strategies, all the stuff that you normally do with standalone tables. Any time one of those things didn't work quite like it would for a standalone table, we got complaints. One of the things that has become really clear over the five years that feature has been out of the field is that users value the ability to do different things with different child tables. Now that of course does not mean that they don't want to be able to operate on the hierarchy as a whole; we have numerous feature requests in that direction, too. But, at least among the base of customers that have talked to us about the proprietary partitioning feature in Advanced Server, wanting to treat a partition as a table and do some arbitrary thing to it that can be done to a table is extremely common. Of course, I can't promise (and am not trying to argue) that the reaction among PostgreSQL users generally will necessarily be similar to the experience we've had with our Advanced Server customers, but this experience has definitely caused me to lean in the direction of wanting partitions to be first-class objects. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
Robert Haas wrote: > On Wed, Feb 15, 2017 at 9:37 AM, Alvaro Herrera >wrote: > > Joshua D. Drake wrote: > >> Because partitions may have data. > > > > So would the table, were it not partitioned. > > True. I think the question here is: do we want to view the dependency > between a partitioned table and a partition of that table as > DEPENDENCY_NORMAL or as DEPENDENCY_AUTO? With table inheritance, it's > always been "normal" and I'm not sure there's any good reason for > partitioning to make the opposite decision. I think new-style partitioning is supposed to consider each partition as an implementation detail of the table; the fact that you can manipulate partitions separately does not really mean that they are their own independent object. You don't stop to think "do I really want to drop the TOAST table attached to this main table?" and attach a CASCADE clause if so. You just drop the main table, and the toast one is dropped automatically. I think new-style partitions should behave equivalently. You can make the partition an independent entity, but if you don't explicitly take that step beforehand, I don't see why we should see it that way implicitly. > The new partitioning > implementation provides a user experience that is overall smoother > than doing the same thing with inheritance, but it's not as if you can > ignore the fact that your partitioned tables have sub-objects that are > also tables. Now that partitions are declarative, the underlying implementation could change away from inheritance. It's now just an implementation artifact. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Wed, Feb 15, 2017 at 9:10 AM, Simon Riggswrote: >>> * "ERROR: cannot create index on partitioned table >>> "measurement_year_month"" >>> is misleading because you can create indexes on partitions >> >> Do you mean that this should not cause an error at all, but create the >> specified index on partitions as part of running the command? Should the >> code to handle that be part of this release? > > Sounds fairly basic to me. If you don't support this, then presumably > every ORM, pgAdmin etc will all be broken. I don't see why that should be the case. > And 1000 people will need to write a script that does what we could do > easily in a loop internally. Now that is probably true. > At present you haven't even documented how you'd do this. It's not that hard to figure it out, though. A HINT wouldn't be a bad idea, certainly. There are some really thorny problems with making index creation cascade to all of the partitions. I think it's worth doing, but there's a lot of stuff to think about before you go and start writing code. Most obviously, if you can use a single CREATE INDEX statement to create indexes on all of the partitions, then probably you ought to also be able to use DROP INDEX to get rid of all of those indexes. In other words, it should probably work a lot like what already happens with constraints: constraints cascade from the parent down to the children, but we still know which child object goes with which parent object, so if the parent object is dropped we can get rid of all of the children. I think we need something similar here, although if we restrict it to the partitioning case and don't make it work with table inheritance then it can be simpler since table partitioning doesn't allow multiple inheritance. Presumably we'd want other index commands like REINDEX to cascade similarly. Also, it's not entirely clear what the semantics should be. If the partitioning key is (a) and you ask for an index on (a, b), you could conceivably omit a from the indexes created on partitions that only cover a single value of a. (That case is easy to detect when list partitioning is in use.) Should we try do that elimination, or just do what the user asked for? Will users be unhappy if we try to do this sort of column elimination but it only works in simple cases? Think about the possibility that there are partitioning expressions rather than partitioning columns before concluding we can make it work in all cases. On the other hand, if you ask for a UNIQUE index on (b), should we go ahead and create such an index on each partition, ensuring uniqueness within each partition, or should we refuse to proceed on the grounds that we can't be sure that such an index will ensure global uniqueness? If you do the former, someone might find the behavior surprising, but if you do the latter, you might annoy people who know what they're asking for and want that thing but can't get it. I suspect we want to eventually allow a user to ask for either one, because eventually we'll probably have global indexes, and then you really need a way to say whether you want a global index or a partitioned non-global index. But that requires agreeing on syntax, which is complicated and will probably involve a lot of bikeshedding (as well it should - these are big decisions). I think it would be a bad idea to try to fix this problem for v10. One of the earlier versions of the patch allowed indexes on the parent table as if it were just a regular empty table, which did not seem useful. I asked him to disallow that so as to keep our options open for the future. I see no reason why v11 or v12 can't fill in the functionality in this area. Right now we're 2 weeks away from the start of the last CommitFest, and that's not the time to go start writing a complex patch for a feature that isn't even particularly well-defined. If somebody really cared about this make-an-index-for-everything-in-the-hierarchy problem, they could've written a patch for that at any time in the last 5 years; it's not strictly dependent on the new partitioning stuff. Nobody's done that, and trying to throw together something now in the last couple of weeks could easily end with us getting it wrong and then having to face the unpleasant prospect of either leaving it broken or breaking backward compatibility to fix it. > It leaves me asking what else is missing. There is certainly a lot of room for improvement here but I don't understand your persistent negativity about what's been done thus far. I think it's pretty clearly a huge step forward, and I think Amit deserves a ton of credit for making it happen. The improvements in bulk loading performance alone are stupendous. You apparently have the idea that somebody could have written an even larger patch that solved even more problems at once, but this was already a really big patch, and IMHO quite a good one. -- Robert Haas EnterpriseDB:
Re: [HACKERS] Documentation improvements for partitioning
On Wed, Feb 15, 2017 at 9:37 AM, Alvaro Herrerawrote: > Joshua D. Drake wrote: >> On 02/15/2017 06:10 AM, Simon Riggs wrote: >> > On 13 February 2017 at 05:21, Amit Langote >> > wrote: >> >> > If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it >> > has indexes, sequences etc on it. So why should it just because it has >> > partitions? >> >> Because partitions may have data. > > So would the table, were it not partitioned. True. I think the question here is: do we want to view the dependency between a partitioned table and a partition of that table as DEPENDENCY_NORMAL or as DEPENDENCY_AUTO? With table inheritance, it's always been "normal" and I'm not sure there's any good reason for partitioning to make the opposite decision. The new partitioning implementation provides a user experience that is overall smoother than doing the same thing with inheritance, but it's not as if you can ignore the fact that your partitioned tables have sub-objects that are also tables. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
Joshua D. Drake wrote: > On 02/15/2017 06:10 AM, Simon Riggs wrote: > > On 13 February 2017 at 05:21, Amit Langote > >wrote: > > > If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it > > has indexes, sequences etc on it. So why should it just because it has > > partitions? > > Because partitions may have data. So would the table, were it not partitioned. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 02/15/2017 06:10 AM, Simon Riggs wrote: On 13 February 2017 at 05:21, Amit Langotewrote: If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it has indexes, sequences etc on it. So why should it just because it has partitions? Because partitions may have data. JD -- Command Prompt, Inc. http://the.postgres.company/ +1-503-667-4564 PostgreSQL Centered full stack support, consulting and development. Everyone appreciates your honesty, until you are honest with them. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 13 February 2017 at 05:21, Amit Langotewrote: >> Few points from tests >> >> * "ERROR: cannot create index on partitioned table "measurement_year_month"" >> is misleading because you can create indexes on partitions > > Do you mean that this should not cause an error at all, but create the > specified index on partitions as part of running the command? Should the > code to handle that be part of this release? Sounds fairly basic to me. If you don't support this, then presumably every ORM, pgAdmin etc will all be broken. And 1000 people will need to write a script that does what we could do easily in a loop internally. At present you haven't even documented how you'd do this. I see you might want to create an index on a subset of partitions, but I expect that you've thought of that and have a proposal for syntax that allows it. Though the default should be that CREATE INDEX just works. > Or do you simply mean that we should rewrite the error message and/or add > a HINT asking to create the index on partitions instead? You could >> * It's very weird you can't DROP a partitioned table. I think you need >> to add dependencies. > > Do you mean it should be possible to DROP a partitioned table without > needing to specify CASCADE? Currently, same thing happens for a > partitioned table as will for a inheritance parent. If we wanted them to act identically we wouldn't have any need for a new feature at all, so clearly that doesn't make sense as an argument. If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it has indexes, sequences etc on it. So why should it just because it has partitions? Most especially if they were created automatically for me in the first place. I might just understand that running ATTACH TABLE might change that viewpoint. I'm pretty sure DROP TABLE and CREATE INDEX are fairly basic expectations from users about how tables should work, partitioned or otherwise. It leaves me asking what else is missing. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Wed, Feb 15, 2017 at 4:26 AM, Ashutosh Bapatwrote: > Noticed some typos in the documentation. Here's patch to correct > those. Sorry, if it has been already taken care of. Thanks. That is indeed nonstandard capitalization. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
Noticed some typos in the documentation. Here's patch to correct those. Sorry, if it has been already taken care of. On Wed, Feb 15, 2017 at 10:01 AM, Robert Haaswrote: > On Mon, Feb 13, 2017 at 5:57 AM, Amit Langote > wrote: >> On 2017/02/13 14:21, Amit Langote wrote: >>> On 2017/02/10 19:19, Simon Riggs wrote: * The OID inheritance needs work - you shouldn't need to specify a partition needs OIDS if the parent has it already. >>> >>> That sounds right. It's better to keep the behavior same as for regular >>> inheritance. Will post a patch to get rid of the unnecessary check. >>> >>> FWIW, the check was added to prevent the command from succeeding in the >>> case where WITHOUT OIDS has been specified for a partition and the parent >>> partitioned table has the OID column. Regular inheritance simply >>> *overrides* the WITHOUT OIDS specification, which might be seen as >>> surprising. >> >> 0001 of the attached patches takes care of this. > > I think 0001 needs to remove this hunk of documentation: > > > > If the partitioned table specified WITH OIDS then > each partition must also specify WITH OIDS. Oids > are not automatically inherited by partitions. > > > > I think 0001 is better than the status quo, but I'm wondering whether > we should try to do something slightly different. Maybe it should > always work for the child table to specify neither WITH OIDS nor > WITHOUT OIDS, but if you do specify one of them then it has to be the > one that matches the parent partitioned table? With this patch, IIUC, > WITH OIDS is allowed only if the parent has the same, but WITHOUT OIDS > is allowed (but ignored) regardless of the parent setting. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company pg_part_doc.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Mon, Feb 13, 2017 at 5:57 AM, Amit Langotewrote: > On 2017/02/13 14:21, Amit Langote wrote: >> On 2017/02/10 19:19, Simon Riggs wrote: >>> * The OID inheritance needs work - you shouldn't need to specify a >>> partition needs OIDS if the parent has it already. >> >> That sounds right. It's better to keep the behavior same as for regular >> inheritance. Will post a patch to get rid of the unnecessary check. >> >> FWIW, the check was added to prevent the command from succeeding in the >> case where WITHOUT OIDS has been specified for a partition and the parent >> partitioned table has the OID column. Regular inheritance simply >> *overrides* the WITHOUT OIDS specification, which might be seen as >> surprising. > > 0001 of the attached patches takes care of this. I think 0001 needs to remove this hunk of documentation: If the partitioned table specified WITH OIDS then each partition must also specify WITH OIDS. Oids are not automatically inherited by partitions. I think 0001 is better than the status quo, but I'm wondering whether we should try to do something slightly different. Maybe it should always work for the child table to specify neither WITH OIDS nor WITHOUT OIDS, but if you do specify one of them then it has to be the one that matches the parent partitioned table? With this patch, IIUC, WITH OIDS is allowed only if the parent has the same, but WITHOUT OIDS is allowed (but ignored) regardless of the parent setting. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On Fri, Feb 10, 2017 at 3:00 AM, Simon Riggswrote: > Given that we already have partitioning feature committed, we really > need to have the docs committed as well. Just for the record, it's not like there were no documentation changes in the originally committed patch. In fact there were over 400 lines of documentation: doc/src/sgml/catalogs.sgml | 129 +++- doc/src/sgml/ref/alter_table.sgml | 117 +- doc/src/sgml/ref/create_foreign_table.sgml | 26 + doc/src/sgml/ref/create_table.sgml | 154 + The patch you committed approximately doubles the amount of documentation for this feature, which is fine as far as it goes, but the key points were all explained in the original commit. I have been known to leave out documentation from commits from time to time and fill it in after-the-fact, but that's not really what happened here. > Without claiming I'm happy about this, I think the best way to improve > the number of eyeballs on this is to commit these docs as is. > > For me, the most important thing is understanding the feature, not > (yet) discussing what the docs should look like. This is especially > true if other patches reference the way partitioning works and nobody > can comment on those patches because they don't understand > > Any issues with that? There are a number of things that I think are awkward about the patch as committed: + + +See last section for some general information: + + + I think we generally try to write the documentation in such a way as to minimize backward references, and a backward reference to the previous section seems particularly odd. We've now got section "5.10 Partitioned Tables" followed immediately by section "5.11 Partitioning", where the latter seems to think that you haven't read the former. I think that section 5.11 needs a much heavier rewrite than what it got as part of this patch. It's a hodgepodge of the old content (which explained how to fake partitioning when we didn't have an explicit concept of partitioning) and new content that talks about how the new way is different from the old way. But now that we have the new way, I'm guessing that most people are going to use that and not care about the old way any more. I'm not that it's even appropriate to keep the lengthy explanation of how to fake table partitioning using table inheritance and non-overlapping CHECK constraints, but if we still want that stuff it should be de-emphasized more than it is here. Probably the section should be retitled: in previous releases we called this "Partitioning" because we had no explicit notion of partitioning, but now that we do, it's confusing to have a section called "Partitioning" that explains how to avoid using the partitioning feature, which is more or less what this does. Or maybe the section title should stay the same (or this should be merged into the previous section?) but rewritten to change the emphasis. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/13 14:21, Amit Langote wrote: > On 2017/02/10 19:19, Simon Riggs wrote: >> On 10 February 2017 at 08:18, Amit Langote >>wrote: >> >>> I agree that getting the proposed documentation changes committed would be >>> a step ahead. >> >> Committed. I tested how it works and added documentation that matched >> my experiences, correcting what you'd said and adding more information >> for clarity as I went. > > Thanks for making the improvements to and committing the patch! Since I had added this to CF 2017-03, I have marked it as committed. https://commitfest.postgresql.org/13/983/ Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/13 14:21, Amit Langote wrote: > On 2017/02/10 19:19, Simon Riggs wrote: >> * The OID inheritance needs work - you shouldn't need to specify a >> partition needs OIDS if the parent has it already. > > That sounds right. It's better to keep the behavior same as for regular > inheritance. Will post a patch to get rid of the unnecessary check. > > FWIW, the check was added to prevent the command from succeeding in the > case where WITHOUT OIDS has been specified for a partition and the parent > partitioned table has the OID column. Regular inheritance simply > *overrides* the WITHOUT OIDS specification, which might be seen as surprising. 0001 of the attached patches takes care of this. >> * There's no tab completion, which prevents people from testing this >> (maybe better now with some docs) > > Will post a patch as well. ...and 0002 for this. >> * ERROR: no partition of relation "measurement_year_month" found for row >> DETAIL: Failing row contains (2016-12-02, 1, 1). >> should not return the whole row, just the partition keys > > I think that makes sense. Something along the lines of > BuildIndexValueDescription() for partition keys will be necessary. Will > post a patch. Let me spend a bit more time on this one. Thanks, Amit >From 42682394d3d40aeaa5b2565a4f0ca23828a0dda0 Mon Sep 17 00:00:00 2001 From: amitDate: Mon, 13 Feb 2017 13:32:18 +0900 Subject: [PATCH 1/3] Inherit OID system column automatically for partitions Currently, WITH OIDS must be explicitly specified when creating a partition if the parent table has the OID system column. Instead, inherit it automatically, possibly overriding any explicit WITHOUT OIDS specification. Per review comment from Simon Riggs --- src/backend/commands/tablecmds.c | 21 - src/test/regress/expected/create_table.out | 18 +- src/test/regress/sql/create_table.sql | 10 ++ 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 37a4c4a3d6..96650e69df 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -634,19 +634,14 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, relkind == RELKIND_PARTITIONED_TABLE)); descriptor->tdhasoid = (localHasOids || parentOidCount > 0); - if (stmt->partbound) - { - /* If the parent has OIDs, partitions must have them too. */ - if (parentOidCount > 0 && !localHasOids) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot create table without OIDs as partition of table with OIDs"))); - /* If the parent doesn't, partitions must not have them. */ - if (parentOidCount == 0 && localHasOids) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot create table with OIDs as partition of table without OIDs"))); - } + /* + * If a partitioned table doesn't have the system OID column, then none + * of its partitions should have it. + */ + if (stmt->partbound && parentOidCount == 0 && localHasOids) + ereport(ERROR, +(errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot create table with OIDs as partition of table without OIDs"))); /* * Find columns with default values and prepare for insertion of the diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index fc92cd92dd..20eb3d35f9 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -524,16 +524,24 @@ DROP TABLE temp_parted; CREATE TABLE no_oids_parted ( a int ) PARTITION BY RANGE (a) WITHOUT OIDS; -CREATE TABLE fail_part PARTITION OF no_oids_parted FOR VALUES FROM (1) TO (10 )WITH OIDS; +CREATE TABLE fail_part PARTITION OF no_oids_parted FOR VALUES FROM (1) TO (10) WITH OIDS; ERROR: cannot create table with OIDs as partition of table without OIDs DROP TABLE no_oids_parted; --- likewise, the reverse if also true +-- If the partitioned table has oids, then the partition must have them. +-- If the WITHOUT OIDS option is specified for partition, it is overridden. CREATE TABLE oids_parted ( a int ) PARTITION BY RANGE (a) WITH OIDS; -CREATE TABLE fail_part PARTITION OF oids_parted FOR VALUES FROM (1) TO (10 ) WITHOUT OIDS; -ERROR: cannot create table without OIDs as partition of table with OIDs -DROP TABLE oids_parted; +CREATE TABLE part_forced_oids PARTITION OF oids_parted FOR VALUES FROM (1) TO (10) WITHOUT OIDS; +\d+ part_forced_oids + Table "public.part_forced_oids" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description ++-+---+--+-+-+--+- + a | integer | | not null | | plain | | +Partition of: oids_parted FOR VALUES FROM (1) TO (10) +Has OIDs: yes + +DROP TABLE oids_parted,
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/10 19:19, Simon Riggs wrote: > On 10 February 2017 at 08:18, Amit Langote >wrote: > >> I agree that getting the proposed documentation changes committed would be >> a step ahead. > > Committed. I tested how it works and added documentation that matched > my experiences, correcting what you'd said and adding more information > for clarity as I went. Thanks for making the improvements to and committing the patch! > Few points from tests > > * "ERROR: cannot create index on partitioned table "measurement_year_month"" > is misleading because you can create indexes on partitions Do you mean that this should not cause an error at all, but create the specified index on partitions as part of running the command? Should the code to handle that be part of this release? Or do you simply mean that we should rewrite the error message and/or add a HINT asking to create the index on partitions instead? > * You can create additional CHECK (column is NOT NULL) as a check > constraint, even if you can't create a not null constraint Sorry but I am not exactly sure which "cannot create a not null constraint" you are referring to here. There are no restrictions on *creating* constraints on partitions, but there are on dropping. Regular inheritance rules prevent dropping inherited constraints (just the same for partitioned tables), of which there are only named CHECK constraints at the moment. A new rule introduced for partitions prevents dropping a column's NOT NULL constraint if it's been "inherited" (i.e., the same constraint is present in the parent partitioned table), although it's not in the same sense as for CHECK constraints, because NOT NULL constraints are not tracked with pg_constraints. > * The OID inheritance needs work - you shouldn't need to specify a > partition needs OIDS if the parent has it already. That sounds right. It's better to keep the behavior same as for regular inheritance. Will post a patch to get rid of the unnecessary check. FWIW, the check was added to prevent the command from succeeding in the case where WITHOUT OIDS has been specified for a partition and the parent partitioned table has the OID column. Regular inheritance simply *overrides* the WITHOUT OIDS specification, which might be seen as surprising. > * There's no tab completion, which prevents people from testing this > (maybe better now with some docs) Will post a patch as well. > * ERROR: no partition of relation "measurement_year_month" found for row > DETAIL: Failing row contains (2016-12-02, 1, 1). > should not return the whole row, just the partition keys I think that makes sense. Something along the lines of BuildIndexValueDescription() for partition keys will be necessary. Will post a patch. > * It's very weird you can't DROP a partitioned table. I think you need > to add dependencies. Do you mean it should be possible to DROP a partitioned table without needing to specify CASCADE? Currently, same thing happens for a partitioned table as will for a inheritance parent. > * ERROR: TO must specify exactly one value per partitioning column > should say something more like "you specified one column value, > whereas the partitioning key requires two columns" Should that be a DETAIL or HINT message? > Good work so far, but there is still a very significant amount of work > to do. And as this feature evolves it must now contain full > documentation at every step, otherwise it won't be able to receive > full and fair review. So please make sure each new patch contains docs > changes for that patch. Agreed that comprehensive documentation of any new feature is crucial both during development and after the feature is released. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 10 February 2017 at 08:18, Amit Langotewrote: > I agree that getting the proposed documentation changes committed would be > a step ahead. Committed. I tested how it works and added documentation that matched my experiences, correcting what you'd said and adding more information for clarity as I went. Few points from tests * "ERROR: cannot create index on partitioned table "measurement_year_month"" is misleading because you can create indexes on partitions * You can create additional CHECK (column is NOT NULL) as a check constraint, even if you can't create a not null constraint * The OID inheritance needs work - you shouldn't need to specify a partition needs OIDS if the parent has it already. * There's no tab completion, which prevents people from testing this (maybe better now with some docs) * ERROR: no partition of relation "measurement_year_month" found for row DETAIL: Failing row contains (2016-12-02, 1, 1). should not return the whole row, just the partition keys * It's very weird you can't DROP a partitioned table. I think you need to add dependencies. * ERROR: TO must specify exactly one value per partitioning column should say something more like "you specified one column value, whereas the partitioning key requires two columns" Good work so far, but there is still a very significant amount of work to do. And as this feature evolves it must now contain full documentation at every step, otherwise it won't be able to receive full and fair review. So please make sure each new patch contains docs changes for that patch. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 2017/02/10 17:00, Simon Riggs wrote: > On 10 February 2017 at 07:35, Amit Langote >wrote: > >>> A final note, because I'm really familiar with partitioning on Postgres and >>> other databases, documentation which is clear to me might not be to someone >>> less familiar with partitioning. Maybe we want another reviewer for that? >> >> More eyeballs will only help make this better. > > Given that we already have partitioning feature committed, we really > need to have the docs committed as well. > > Without claiming I'm happy about this, I think the best way to improve > the number of eyeballs on this is to commit these docs as is. > > For me, the most important thing is understanding the feature, not > (yet) discussing what the docs should look like. This is especially > true if other patches reference the way partitioning works and nobody > can comment on those patches because they don't understand > > Any issues with that? I agree that getting the proposed documentation changes committed would be a step ahead. I saw in the logical replication thread that dealing with partitioned tables without the docs explaining what they are has been difficult. Hopefully the proposed documentation improvements help make progress in that regard. Partitioned tables, at this point, have certain limitations which affect its interaction with other features (old or new); documenting those limitations will be helpful not only to the users deciding whether to start using the new partitioned tables right away, but also to the developers of other features who want to understand what partitioned tables are. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
On 10 February 2017 at 07:35, Amit Langotewrote: >> A final note, because I'm really familiar with partitioning on Postgres and >> other databases, documentation which is clear to me might not be to someone >> less familiar with partitioning. Maybe we want another reviewer for that? > > More eyeballs will only help make this better. Given that we already have partitioning feature committed, we really need to have the docs committed as well. Without claiming I'm happy about this, I think the best way to improve the number of eyeballs on this is to commit these docs as is. For me, the most important thing is understanding the feature, not (yet) discussing what the docs should look like. This is especially true if other patches reference the way partitioning works and nobody can comment on those patches because they don't understand Any issues with that? -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation improvements for partitioning
Hi Corey, On 2017/02/09 6:14, Corey Huinker wrote: > On Fri, Feb 3, 2017 at 4:15 AM, Amit Langote> wrote: > >> Here are some patches to improve the documentation about partitioned >> tables: > > Patch applies. > > Overall this looks really good. It goes a long way towards stating some of > the things I had to learn through experimentation. Thanks a lot for taking a look at it. > I had to read a really long way into the patch before finding a blurb that > I felt wasn't completely clear: > > + > + > + INSERT statements with ON CONFLICT > + clause are currently not allowed on partitioned tables, that is, > + cause error when specified. > + > > > Here's some other tries at saying the same thing, none of which are > completely satisfying: > > ...ON CONFLICT clause are currently not allowed on partitioned tables and > will cause an error? > ...ON CONFLICT clause are currently not allowed on partitioned tables and > will instead cause an error? > ...ON CONFLICT clause will currently cause an error if used on a > partitioned table? The last one sounds better. > As far as additional issues to cover, this bit: > > + > + > + One cannot drop a NOT NULL constraint on a > + partition's column, if the constraint is present in the parent > table. > + > + > > Maybe we should add something about how one would go about dropping a NOT > NULL constraint (parent first then partitions?) Dropping it on the parent will cause it to be dropped on the partitions as well. About your point whether we should add a note about how to go about dropping it in the partition, it seems to me it would be out of place here; it's just saying that dropping NOT NULL constraint has a different behavior with partitioned tables than regular inheritance. That note most likely belongs in the ALTER TABLE reference page in the DROP NOT NULL description, so created a patch for that (patch 0004 of the attached patches). > In reviewing this patch, do all our target formats make word spacing > irrelevant? i.e. is there any point in looking at the number of spaces > after a period, etc? It seems to be a convention in the sources to include 2 spaces after a period, which I just try to follow (both in the code comments and SGML). I don't see that spaces are relevant as far as how the targets such as HTML are rendered. > A final note, because I'm really familiar with partitioning on Postgres and > other databases, documentation which is clear to me might not be to someone > less familiar with partitioning. Maybe we want another reviewer for that? More eyeballs will only help make this better. Thanks, Amit >From 3074d1986afe0f3ce4710f38517f2e1929ff4c48 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 26 Jan 2017 18:57:55 +0900 Subject: [PATCH 1/4] Improve CREATE TABLE documentation of partitioning --- doc/src/sgml/ref/create_table.sgml | 103 ++--- 1 file changed, 96 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 58f8bf6d6a..5596250aef 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -85,8 +85,8 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI and partition_bound_spec is: -{ IN ( expression [, ...] ) | - FROM ( { expression | UNBOUNDED } [, ...] ) TO ( { expression | UNBOUNDED } [, ...] ) } +{ IN ( { bound_literal | NULL } [, ...] ) | + FROM ( { bound_literal | UNBOUNDED } [, ...] ) TO ( { bound_literal | UNBOUNDED } [, ...] ) } index_parameters in UNIQUE, PRIMARY KEY, and EXCLUDE constraints are: @@ -261,6 +261,44 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI any existing partition of that parent. + + + Each of the values specified in the partition bound specification is + a literal, NULL, or UNBOUNDED. + A literal is either a numeric constant or a string constant that can be + automatically coerced to the corresponding partition key column's type. + + + + When creating a range partition, the lower bound specified with + FROM is an inclusive bound, whereas the upper bound + specified with TO is an exclusive bound. That is, + the values specified in the FROM list are accepted + values of the corresponding partition key columns in a given partition, + whereas those in the TO list are not. To be precise, + this applies only to the first of the partition key columns for which + the corresponding values in the FROM and + TO lists are not equal. All rows in a given + partition contain the same values for all preceding columns, equal to + those specified in FROM and TO + lists. On the other hand, any subsequent columns are insignificant + as far as implicit
Re: [HACKERS] Documentation improvements for partitioning
On Fri, Feb 3, 2017 at 4:15 AM, Amit Langotewrote: > Here are some patches to improve the documentation about partitioned > tables: > > 0001: Adds some details about partition_bound_spec to the CREATE TABLE > page, especially: > > - a note about inclusivity of range partition bounds, > - a note about the UNBOUNDED literal in case of range partitioning, > - a note about the NULL literal in case of list partitioning, > > I wonder if the above "note" should be added under the Notes section or > are they fine to be added as part of the description of PARTITION OF > clause. Also: > > - in syntax synopsis, it appears now that any expression is OK to be used >for individual bound datum, but it's not true. Only literals are >allowed. So fixed that. > - added an example showing how to create partitions of a range >partitioned table with multiple columns in the partition key > - added PARTITION BY and PARTITION OF (FOR VALUES) as PostgreSQL >extensions in the compatibility section > > > 0002: Adds details about partitioned tables to the DDL chapter (ddl.sgml) > > - a new section named "Partitioned Tables" right next to the >Inheritance and Partitioning sections is created. > - examples are added to the existing Partitioning section using the new >partitioned tables. Old text about implementing table partitioning >using inheritance is kept, sort of as a still supported older >alternative. > > 0003: Add partitioning keywords to keywords.sgml > > This is all I have for now. Any feedback is greatly appreciated. Adding > this to the next CF. > > Thanks, > Amit > Patch applies. Overall this looks really good. It goes a long way towards stating some of the things I had to learn through experimentation. I had to read a really long way into the patch before finding a blurb that I felt wasn't completely clear: + + + INSERT statements with ON CONFLICT + clause are currently not allowed on partitioned tables, that is, + cause error when specified. + Here's some other tries at saying the same thing, none of which are completely satisfying: ...ON CONFLICT clause are currently not allowed on partitioned tables and will cause an error? ...ON CONFLICT clause are currently not allowed on partitioned tables and will instead cause an error? ...ON CONFLICT clause will currently cause an error if used on a partitioned table? As far as additional issues to cover, this bit: + + + One cannot drop a NOT NULL constraint on a + partition's column, if the constraint is present in the parent table. + + Maybe we should add something about how one would go about dropping a NOT NULL constraint (parent first then partitions?) In reviewing this patch, do all our target formats make word spacing irrelevant? i.e. is there any point in looking at the number of spaces after a period, etc? A final note, because I'm really familiar with partitioning on Postgres and other databases, documentation which is clear to me might not be to someone less familiar with partitioning. Maybe we want another reviewer for that?
[HACKERS] Documentation improvements for partitioning
Here are some patches to improve the documentation about partitioned tables: 0001: Adds some details about partition_bound_spec to the CREATE TABLE page, especially: - a note about inclusivity of range partition bounds, - a note about the UNBOUNDED literal in case of range partitioning, - a note about the NULL literal in case of list partitioning, I wonder if the above "note" should be added under the Notes section or are they fine to be added as part of the description of PARTITION OF clause. Also: - in syntax synopsis, it appears now that any expression is OK to be used for individual bound datum, but it's not true. Only literals are allowed. So fixed that. - added an example showing how to create partitions of a range partitioned table with multiple columns in the partition key - added PARTITION BY and PARTITION OF (FOR VALUES) as PostgreSQL extensions in the compatibility section 0002: Adds details about partitioned tables to the DDL chapter (ddl.sgml) - a new section named "Partitioned Tables" right next to the Inheritance and Partitioning sections is created. - examples are added to the existing Partitioning section using the new partitioned tables. Old text about implementing table partitioning using inheritance is kept, sort of as a still supported older alternative. 0003: Add partitioning keywords to keywords.sgml This is all I have for now. Any feedback is greatly appreciated. Adding this to the next CF. Thanks, Amit >From 47c9edf3e46080d4bfb7d2a2908016c9039ee20f Mon Sep 17 00:00:00 2001 From: amitDate: Thu, 26 Jan 2017 18:57:55 +0900 Subject: [PATCH 1/3] Improve CREATE TABLE documentation of partitioning --- doc/src/sgml/ref/create_table.sgml | 103 ++--- 1 file changed, 96 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 58f8bf6d6a..5596250aef 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -85,8 +85,8 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI and partition_bound_spec is: -{ IN ( expression [, ...] ) | - FROM ( { expression | UNBOUNDED } [, ...] ) TO ( { expression | UNBOUNDED } [, ...] ) } +{ IN ( { bound_literal | NULL } [, ...] ) | + FROM ( { bound_literal | UNBOUNDED } [, ...] ) TO ( { bound_literal | UNBOUNDED } [, ...] ) } index_parameters in UNIQUE, PRIMARY KEY, and EXCLUDE constraints are: @@ -261,6 +261,44 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI any existing partition of that parent. + + + Each of the values specified in the partition bound specification is + a literal, NULL, or UNBOUNDED. + A literal is either a numeric constant or a string constant that can be + automatically coerced to the corresponding partition key column's type. + + + + When creating a range partition, the lower bound specified with + FROM is an inclusive bound, whereas the upper bound + specified with TO is an exclusive bound. That is, + the values specified in the FROM list are accepted + values of the corresponding partition key columns in a given partition, + whereas those in the TO list are not. To be precise, + this applies only to the first of the partition key columns for which + the corresponding values in the FROM and + TO lists are not equal. All rows in a given + partition contain the same values for all preceding columns, equal to + those specified in FROM and TO + lists. On the other hand, any subsequent columns are insignificant + as far as implicit partition constraint is concerned. + + Specifying UNBOUNDED in FROM + signifies -infinity as the lower bound of the + corresponding column, whereas it signifies +infinity + as the upper bound when specified in TO. + + + + When creating a list partition, NULL can be specified + to signify that the partition allows the partition key column to be null. + However, there cannot be more than one such list partitions for a given + parent table. NULL cannot specified for range + partitions. + + + A partition cannot have columns other than those inherited from the parent. That includes the oid column, which can be @@ -386,11 +424,12 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI partitioned table. The parenthesized list of columns or expressions forms the partition key for the table. When using range partitioning, the partition key can - include multiple columns or expressions, but for list partitioning, the - partition key must consist of a single column or expression. If no - btree operator class is specified when creating a