Re: [HACKERS] Minor correction in alter_table.sgml
Amit, * Amit Langote (amitlangot...@gmail.com) wrote: > On Fri, Dec 23, 2016 at 12:07 AM, Stephen Frost wrote: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> (Of course, maybe the question we ought to be asking here is why > >> ATTACH/DETACH PARTITION failed to go with the flow and be a > >> combinable action.) > > > > I did wonder that myself but havne't looked at the code. I'm guessing > > there's a reason it's that way. > > I thought the possibility of something like the following happening > should be avoided: > > alter table p attach partition p1 for values in (1, 2, 3), add b int; > ERROR: child table is missing column "b" Sure, but what about something like: alter table p attach partition p1 for values in (1, 2, 3), alter column b set default 1; ? > Although, the same can be said about ALTER TABLE child INHERIT parent, I > guess. Certainly seems like that's an indication that there are use-cases for allowing it then. We do tend to avoid arbitrary restrictions and if there isn't really anything code-level for ATTACH/DETACH partition to be this way then we change it to be allowed. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Minor correction in alter_table.sgml
On Fri, Dec 23, 2016 at 12:07 AM, Stephen Frost wrote: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> (Of course, maybe the question we ought to be asking here is why >> ATTACH/DETACH PARTITION failed to go with the flow and be a >> combinable action.) > > I did wonder that myself but havne't looked at the code. I'm guessing > there's a reason it's that way. I thought the possibility of something like the following happening should be avoided: alter table p attach partition p1 for values in (1, 2, 3), add b int; ERROR: child table is missing column "b" Although, the same can be said about ALTER TABLE child INHERIT parent, I guess. 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] Minor correction in alter_table.sgml
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Alvaro Herrera writes: > >> I had considered removing those but thought that some users might think > >> that they're only "altering" one table and therefore felt it made sense > >> to keep those explicitly listed. > > > I agree with Stephen; it's not crystal clear from the user's POV that > > the command is altering two tables. It's worth mentioning this > > explicitly; otherwise this is just a documented gotcha. > > Well, it already is shown explicitly in the syntax summary. The text > is simply trying to restate that in an easily remembered fashion, and > the more exceptions, the harder it is to remember. You might as well > forget trying to provide a rule at all and just say something like > "Most forms of ALTER TABLE can be combined, except as shown in the > syntax diagram." I do wonder if perhaps we should change 'action' to something like 'combinable_action' or something more explicit which we could easily refer to later in a statement along the lines of: Multiple combinable_actions specified in a single ALTER TABLE statement will be applied together in a single pass over the table. > (Of course, maybe the question we ought to be asking here is why > ATTACH/DETACH PARTITION failed to go with the flow and be a > combinable action.) I did wonder that myself but havne't looked at the code. I'm guessing there's a reason it's that way. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Minor correction in alter_table.sgml
Alvaro Herrera writes: >> I had considered removing those but thought that some users might think >> that they're only "altering" one table and therefore felt it made sense >> to keep those explicitly listed. > I agree with Stephen; it's not crystal clear from the user's POV that > the command is altering two tables. It's worth mentioning this > explicitly; otherwise this is just a documented gotcha. Well, it already is shown explicitly in the syntax summary. The text is simply trying to restate that in an easily remembered fashion, and the more exceptions, the harder it is to remember. You might as well forget trying to provide a rule at all and just say something like "Most forms of ALTER TABLE can be combined, except as shown in the syntax diagram." (Of course, maybe the question we ought to be asking here is why ATTACH/DETACH PARTITION failed to go with the flow and be a combinable action.) 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] Minor correction in alter_table.sgml
Stephen Frost wrote: > Tom, > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > > BTW, so far as the HEAD version of this patch goes, I notice that > > ATTACH PARTITION and DETACH PARTITION were recently added to the > > list of exceptions. But they're not exceptions according to this > > wording: they act on more than one table (the parent and the > > partition), no? So we could simplify the sentence some more by > > removing them again. > > I had considered removing those but thought that some users might think > that they're only "altering" one table and therefore felt it made sense > to keep those explicitly listed. I agree with Stephen; it's not crystal clear from the user's POV that the command is altering two tables. It's worth mentioning this explicitly; otherwise this is just a documented gotcha. -- Á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] Minor correction in alter_table.sgml
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> So maybe something like > >> > >> All the forms of ALTER TABLE that act on a single table, > >> except RENAME and SET SCHEMA, can be combined into a > >> list of multiple alterations to be applied together. > > > Committed and back-patch'd that way. > > BTW, so far as the HEAD version of this patch goes, I notice that > ATTACH PARTITION and DETACH PARTITION were recently added to the > list of exceptions. But they're not exceptions according to this > wording: they act on more than one table (the parent and the > partition), no? So we could simplify the sentence some more by > removing them again. I had considered removing those but thought that some users might think that they're only "altering" one table and therefore felt it made sense to keep those explicitly listed. I don't hold that position very strongly, but wanted to explain my thinking there. If you still feel it'd be better to keep it simple rather than be explicit for those cases then I'll change it. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Minor correction in alter_table.sgml
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> So maybe something like >> >> All the forms of ALTER TABLE that act on a single table, >> except RENAME and SET SCHEMA, can be combined into a >> list of multiple alterations to be applied together. > Committed and back-patch'd that way. BTW, so far as the HEAD version of this patch goes, I notice that ATTACH PARTITION and DETACH PARTITION were recently added to the list of exceptions. But they're not exceptions according to this wording: they act on more than one table (the parent and the partition), no? So we could simplify the sentence some more by removing them again. 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] Minor correction in alter_table.sgml
* Tom Lane (t...@sss.pgh.pa.us) wrote: > So maybe something like > > All the forms of ALTER TABLE that act on a single table, > except RENAME and SET SCHEMA, can be combined into a > list of multiple alterations to be applied together. Committed and back-patch'd that way. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Minor correction in alter_table.sgml
On 2016/12/20 4:08, Peter Eisentraut wrote: > On 11/30/16 8:47 PM, Amit Langote wrote: >>> So maybe something like >>> >>> All the forms of ALTER TABLE that act on a single table, >>> except RENAME and SET SCHEMA, can be combined into a >>> list of multiple alterations to be applied together. >> >> Updated patch attached. > > Could you please rebase this patch? Some new stuff about partitions has > been added in the paragraph you are changing. Sure, here it is. Thanks, Amit diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 8ea6624147..cce853be8a 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -767,13 +767,12 @@ ALTER TABLE [ IF EXISTS ] name - All the actions except RENAME, - SET TABLESPACE, SET SCHEMA, - ATTACH PARTITION, and - DETACH PARTITION can be combined into - a list of multiple alterations to apply in parallel. For example, it - is possible to add several columns and/or alter the type of several - columns in a single command. This is particularly useful with large + All the forms of ALTER TABLE that act on a single table, + except RENAME, SET SCHEMA, + ATTACH PARTITION, and DETACH PARTITION + can be combined into a list of multiple alterations to be applied together. + For example, it is possible to add several columns and/or alter the type of + several columns in a single command. This is particularly useful with large tables, since only one pass over the table need be made. -- 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] Minor correction in alter_table.sgml
On 11/30/16 8:47 PM, Amit Langote wrote: >> So maybe something like >> >> All the forms of ALTER TABLE that act on a single table, >> except RENAME and SET SCHEMA, can be combined into a >> list of multiple alterations to be applied together. > > Updated patch attached. Could you please rebase this patch? Some new stuff about partitions has been added in the paragraph you are changing. -- Peter Eisentraut http://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] Minor correction in alter_table.sgml
On 2016/12/01 1:17, Tom Lane wrote: > Stephen Frost writes: >> Seems like this would be a bit better: > >> -- >> All the actions, when acting on a single table and not using the ALL IN >> TABLESPACE form, except RENAME and SET SCHEMA, can be combined into a >> list of multiple alterations to be applied. >> -- > >> I note that we say 'in parallel', but given that we have actual parallel >> operations now, we should probably shy away from using that except in >> cases where we actually mean operations utilizing multiple parallel >> processes. > > I follow your beef with use of the word "parallel", but your proposed > rewording loses the entire point of multiple actions per ALTER TABLE; > namely that they're accomplished without repeated scans of the table. > > Also the above seems a bit clunky; doesn't ALL IN TABLESPACE fall outside > the restriction "acting on a single table"? > > So maybe something like > > All the forms of ALTER TABLE that act on a single table, > except RENAME and SET SCHEMA, can be combined into a > list of multiple alterations to be applied together. Updated patch attached. > We would have to enlarge on what "together" means, but I think there may > already be text explaining that further down. There is this explanation: The main reason for providing the option to specify multiple changes in a single ALTER TABLE is that multiple table scans or rewrites can thereby be combined into a single pass over the table. Thanks, Amit diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index e48ccf2..9e79e08 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -708,12 +708,11 @@ ALTER TABLE ALL IN TABLESPACE name - All the actions except RENAME, - SET TABLESPACE and SET SCHEMA - can be combined into - a list of multiple alterations to apply in parallel. For example, it - is possible to add several columns and/or alter the type of several - columns in a single command. This is particularly useful with large + All the forms of ALTER TABLE that act on a single table, + except RENAME and SET SCHEMA, can be + combined into a list of multiple alterations to be applied together. + For example, it is possible to add several columns and/or alter the type of + several columns in a single command. This is particularly useful with large tables, since only one pass over the table need be made. -- 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] Minor correction in alter_table.sgml
Stephen Frost writes: > Seems like this would be a bit better: > -- > All the actions, when acting on a single table and not using the ALL IN > TABLESPACE form, except RENAME and SET SCHEMA, can be combined into a > list of multiple alterations to be applied. > -- > I note that we say 'in parallel', but given that we have actual parallel > operations now, we should probably shy away from using that except in > cases where we actually mean operations utilizing multiple parallel > processes. I follow your beef with use of the word "parallel", but your proposed rewording loses the entire point of multiple actions per ALTER TABLE; namely that they're accomplished without repeated scans of the table. Also the above seems a bit clunky; doesn't ALL IN TABLESPACE fall outside the restriction "acting on a single table"? So maybe something like All the forms of ALTER TABLE that act on a single table, except RENAME and SET SCHEMA, can be combined into a list of multiple alterations to be applied together. We would have to enlarge on what "together" means, but I think there may already be text explaining that further down. 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] Minor correction in alter_table.sgml
Hi Stephen, On Thu, Dec 1, 2016 at 12:24 AM, Stephen Frost wrote: > Amit, > > * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote: >> Perhaps, it should say something like: >> >> All the actions except RENAME, SET TABLESPACE (when using the ALL IN >> TABLESPACE form) and SET SCHEMA can be combined into a list of multiple >> alterations to apply in parallel. > > Seems like this would be a bit better: > > -- > All the actions, when acting on a single table and not using the ALL IN > TABLESPACE form, except RENAME and SET SCHEMA, can be combined into a > list of multiple alterations to be applied. > -- > > I note that we say 'in parallel', but given that we have actual parallel > operations now, we should probably shy away from using that except in > cases where we actually mean operations utilizing multiple parallel > processes. > > Thoughts? Sounds good to me. 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] Minor correction in alter_table.sgml
Amit, * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote: > Perhaps, it should say something like: > > All the actions except RENAME, SET TABLESPACE (when using the ALL IN > TABLESPACE form) and SET SCHEMA can be combined into a list of multiple > alterations to apply in parallel. Seems like this would be a bit better: -- All the actions, when acting on a single table and not using the ALL IN TABLESPACE form, except RENAME and SET SCHEMA, can be combined into a list of multiple alterations to be applied. -- I note that we say 'in parallel', but given that we have actual parallel operations now, we should probably shy away from using that except in cases where we actually mean operations utilizing multiple parallel processes. Thoughts? Thanks! Stephen signature.asc Description: Digital signature
[HACKERS] Minor correction in alter_table.sgml
The following sentence in the ALTER TABLE documentation is not entirely accurate: "All the actions except RENAME, SET TABLESPACE and SET SCHEMA can be combined into a list of multiple alterations to apply in parallel." SET TABLESPACE (in the ALTER TABLE form) can be combined with other sub-commands; following works: alter table foo set tablespace mytbls, add b int; Perhaps, it should say something like: All the actions except RENAME, SET TABLESPACE (when using the ALL IN TABLESPACE form) and SET SCHEMA can be combined into a list of multiple alterations to apply in parallel. Attached is a patch. Thanks, Amit diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index e48ccf2..150a3b8 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -709,8 +709,8 @@ ALTER TABLE ALL IN TABLESPACE name All the actions except RENAME, - SET TABLESPACE and SET SCHEMA - can be combined into + SET TABLESPACE (when using the ALL IN TABLESPACE + form) and SET SCHEMA can be combined into a list of multiple alterations to apply in parallel. For example, it is possible to add several columns and/or alter the type of several columns in a single command. This is particularly useful with large -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Minor correction in alter_table.sgml
The following sentence in the ALTER TABLE documentation is not entirely accurate: "All the actions except RENAME, SET TABLESPACE and SET SCHEMA can be combined into a list of multiple alterations to apply in parallel." SET TABLESPACE (in the ALTER TABLE form) can be combined with other subcommands; for example, following works: alter table foo set tablespace mytbls, add b int; Perhaps, it should say something like: All the actions except RENAME, SET TABLESPACE (when using the ALL IN TABLESPACE form) and SET SCHEMA can be combined into a list of multiple alterations to apply in parallel. Attached is a patch. Thanks, Amit diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index e48ccf2..150a3b8 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -709,8 +709,8 @@ ALTER TABLE ALL IN TABLESPACE name All the actions except RENAME, - SET TABLESPACE and SET SCHEMA - can be combined into + SET TABLESPACE (when using the ALL IN TABLESPACE + form) and SET SCHEMA can be combined into a list of multiple alterations to apply in parallel. For example, it is possible to add several columns and/or alter the type of several columns in a single command. This is particularly useful with large -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
