Re: [HACKERS] Adding column_constraint description in ALTER TABLE synopsis

[Lætitia Avrot]

Hi,

Thank you all for this so warm reception for my very first patch.
Thanks Stephen to have thought to add that patch to the commit fest. As it
was committed Friday, I was able to tell the whole story in my talk and
that's great.
And thanks again to everyone who helped me with that patch.

Regards

Lætitia

Le 5 févr. 2018 01:36, "Amit Langote"  a
écrit :

> On 2018/02/02 19:41, Stephen Frost wrote:
> > * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
> >> On 2018/01/23 8:57, Thomas Munro wrote:
> >>> Here's an update to move the new stuff to the correct side of the
> >>> closing "".  Builds for me, and the typesetting looks OK.
> >>> I'm not sure why the preexisting bogo-productions have inconsistent
> >>> indentation levels (looking at table_constraint_using_index) but
> >>> that's not this patch's fault.
> >>
> >> Thanks for fixing that.
> >>
> >> I noticed that partition_bound_spec in the patch is missing hash
> partition
> >> bound syntax that was added after the original patch was written.  Fixed
> >> in the attached.
> >
> > I've pushed this with just a bit of re-ordering to match the order in
> > which things show up in the synopsis.
>
> Thank you Stephen.
>
> Regards,
> Amit
>
>

Re: [HACKERS] Adding column_constraint description in ALTER TABLE synopsis

[Amit Langote]

On 2018/02/02 19:41, Stephen Frost wrote:
> * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
>> On 2018/01/23 8:57, Thomas Munro wrote:
>>> Here's an update to move the new stuff to the correct side of the
>>> closing "".  Builds for me, and the typesetting looks OK.
>>> I'm not sure why the preexisting bogo-productions have inconsistent
>>> indentation levels (looking at table_constraint_using_index) but
>>> that's not this patch's fault.
>>
>> Thanks for fixing that.
>>
>> I noticed that partition_bound_spec in the patch is missing hash partition
>> bound syntax that was added after the original patch was written.  Fixed
>> in the attached.
> 
> I've pushed this with just a bit of re-ordering to match the order in
> which things show up in the synopsis.

Thank you Stephen.

Regards,
Amit

Re: [HACKERS] Adding column_constraint description in ALTER TABLE synopsis

[Stephen Frost]

Greetings,

* Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
> On 2018/01/23 8:57, Thomas Munro wrote:
> > On Tue, Jan 23, 2018 at 12:41 PM, Thomas Munro
> >  wrote:
> >> On Mon, Jan 15, 2018 at 2:32 PM, Stephen Frost  wrote:
> >>> If someone else would like to review it, that'd be great, otherwise I'll
> >>> probably get it committed soon.
> >>
> >> FYI the v2 doesn't build:
> >>
> >> ref/alter_table.sgml:135: parser error : Opening and ending tag
> >> mismatch: refentry line 6 and synopsis
> >> 
> > 
> > Here's an update to move the new stuff to the correct side of the
> > closing "".  Builds for me, and the typesetting looks OK.
> > I'm not sure why the preexisting bogo-productions have inconsistent
> > indentation levels (looking at table_constraint_using_index) but
> > that's not this patch's fault.
> 
> Thanks for fixing that.
> 
> I noticed that partition_bound_spec in the patch is missing hash partition
> bound syntax that was added after the original patch was written.  Fixed
> in the attached.

I've pushed this with just a bit of re-ordering to match the order in
which things show up in the synopsis.

Thanks!

Stephen


signature.asc
Description: PGP signature

Re: [HACKERS] Adding column_constraint description in ALTER TABLE synopsis

[Amit Langote]

On 2018/01/23 8:57, Thomas Munro wrote:
> On Tue, Jan 23, 2018 at 12:41 PM, Thomas Munro
>  wrote:
>> On Mon, Jan 15, 2018 at 2:32 PM, Stephen Frost  wrote:
>>> If someone else would like to review it, that'd be great, otherwise I'll
>>> probably get it committed soon.
>>
>> FYI the v2 doesn't build:
>>
>> ref/alter_table.sgml:135: parser error : Opening and ending tag
>> mismatch: refentry line 6 and synopsis
>> 
> 
> Here's an update to move the new stuff to the correct side of the
> closing "".  Builds for me, and the typesetting looks OK.
> I'm not sure why the preexisting bogo-productions have inconsistent
> indentation levels (looking at table_constraint_using_index) but
> that's not this patch's fault.

Thanks for fixing that.

I noticed that partition_bound_spec in the patch is missing hash partition
bound syntax that was added after the original patch was written.  Fixed
in the attached.

Thanks,
Amit
diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 286c7a8589..5cc0519c8c 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -85,6 +85,20 @@ ALTER TABLE [ IF EXISTS ] name
 OWNER TO { new_owner | 
CURRENT_USER | SESSION_USER }
 REPLICA IDENTITY { DEFAULT | USING INDEX index_name | FULL | NOTHING }
 
+and column_constraint 
is:
+
+[ CONSTRAINT constraint_name ]
+{ NOT NULL |
+  NULL |
+  CHECK ( expression ) [ NO 
INHERIT ] |
+  DEFAULT default_expr |
+  GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY [ ( 
sequence_options ) ] |
+  UNIQUE index_parameters |
+  PRIMARY KEY index_parameters |
+  REFERENCES reftable [ ( 
refcolumn ) ] [ MATCH FULL | MATCH 
PARTIAL | MATCH SIMPLE ]
+[ ON DELETE action ] [ ON 
UPDATE action ] }
+[ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
+
 and table_constraint 
is:
 
 [ CONSTRAINT constraint_name ]
@@ -96,11 +110,27 @@ ALTER TABLE [ IF EXISTS ] name
 [ MATCH FULL | MATCH PARTIAL | MATCH SIMPLE ] [ ON DELETE action ] [ ON UPDATE action ] }
 [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
 
+index_parameters in 
UNIQUE, PRIMARY KEY, and 
EXCLUDE constraints are:
+
+[ WITH ( storage_parameter [= 
value] [, ... ] ) ]
+[ USING INDEX TABLESPACE tablespace_name ]
+
+exclude_element in an 
EXCLUDE constraint is:
+
+{ column_name | ( expression ) } [ opclass ] [ ASC | DESC ] [ NULLS { FIRST | LAST 
} ]
+
 and table_constraint_using_index is:
 
 [ CONSTRAINT constraint_name ]
 { UNIQUE | PRIMARY KEY } USING INDEX index_name
 [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE 
]
+
+and partition_bound_spec 
is:
+
+IN ( { numeric_literal | 
string_literal | NULL } [, ...] ) |
+FROM ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] )
+  TO ( { numeric_literal | 
string_literal | MINVALUE | 
MAXVALUE } [, ...] ) |
+WITH ( MODULUS numeric_literal, 
REMAINDER numeric_literal )
 
  
 

Re: [HACKERS] Adding column_constraint description in ALTER TABLE synopsis

[Thomas Munro]

On Tue, Jan 23, 2018 at 12:41 PM, Thomas Munro
 wrote:
> On Mon, Jan 15, 2018 at 2:32 PM, Stephen Frost  wrote:
>> If someone else would like to review it, that'd be great, otherwise I'll
>> probably get it committed soon.
>
> FYI the v2 doesn't build:
>
> ref/alter_table.sgml:135: parser error : Opening and ending tag
> mismatch: refentry line 6 and synopsis
> 

Here's an update to move the new stuff to the correct side of the
closing "".  Builds for me, and the typesetting looks OK.
I'm not sure why the preexisting bogo-productions have inconsistent
indentation levels (looking at table_constraint_using_index) but
that's not this patch's fault.

-- 
Thomas Munro
http://www.enterprisedb.com


adding_column_constraint_description_in_ALTER_TABLE_synopsis_v3.patch
Description: Binary data

Re: [HACKERS] Adding column_constraint description in ALTER TABLE synopsis

[Stephen Frost]

Greetings Lætitia, Amit,

* Lætitia Avrot (laetitia.av...@gmail.com) wrote:
> Thanks Stephen for the suggestion. I wan't thinking globally enough. I was
> planning to look at it today but Amit was faster. So thanks Amit too!

This seems to have gotten lost in the shuffle, but Amit's patch still
applies cleanly and it looks like a good patch to me, so I've added it
to the current CF and marked it as Needs Review.  There's been some
further discussion of this change over in this thread:

https://www.postgresql.org/message-id/flat/20171220090816.25744.72592%40wrigleys.postgresql.org#20171220090816.25744.72...@wrigleys.postgresql.org

which seemed to reach the conclusion that the proposed patch makes
sense.

If someone else would like to review it, that'd be great, otherwise I'll
probably get it committed soon.

Thanks!

Stephen


signature.asc
Description: PGP signature