Обсуждение: 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

Amit,

* Amit Langote (Langote_Amit_f8@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

Hi Stephen,

On Thu, Dec 1, 2016 at 12:24 AM, Stephen Frost <sfrost@snowman.net> wrote:
> Amit,
>
> * Amit Langote (Langote_Amit_f8@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

Stephen Frost <sfrost@snowman.net> 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 alist of
multiplealterations 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

On 2016/12/01 1:17, Tom Lane wrote:
> Stephen Frost <sfrost@snowman.net> 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:

   <para>
    The main reason for providing the option to specify multiple changes
    in a single <command>ALTER TABLE</> is that multiple table scans or
    rewrites can thereby be combined into a single pass over the table.
   </para>

Thanks,
Amit

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

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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Tom Lane (tgl@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

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@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

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Tom Lane (tgl@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

Stephen Frost wrote:
> Tom,
> 
> * Tom Lane (tgl@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 Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services