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

[Lætitia Avrot]

Hi all,

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!

Have a nice day (UGT),

Lætitia

2017-11-01 1:35 GMT+01:00 Amit Langote :

> On 2017/10/31 21:31, Stephen Frost wrote:
> > * Lætitia Avrot (laetitia.av...@gmail.com) wrote:
> >> As Amit Langot pointed out, the column_constraint definition is missing
> >> whereas it is used in ALTER TABLE synopsis. It can be easily found in
> the
> >> CREATE TABLE synopsis, but it's not very user friendly.
> >
> > Thanks, this looks pretty reasonable, but did you happen to look for any
> > other keywords in the ALTER TABLE that should really be in ALTER TABLE
> > also?
> >
> > I'm specifically looking at, at least, partition_bound_spec.  Maybe you
> > could propose an updated patch which addresses that also, and any other
> > cases you find?
>
> Ah, yes.  I remember having left out partition_bound_spec simply because I
> thought it was kind of how it was supposed to be done, seeing that neither
> column_constraint and table_constraint were expanded in the ALTER TABLE's
> synopsis.
>
> It seems that there are indeed a couple of other things that need to be
> brought over to ALTER TABLE synopsis including partition_bound_spec.
> 9f295c08f877 [1] added table_constraint, but missed to add the description
> of index_parameters and exclude_element which are referenced therein.
>
> Attached find updated version of the Lætitia's patch.
>
> Thanks,
> Amit
>
> [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9f295c
>

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

[Amit Langote]

On 2017/10/31 21:31, Stephen Frost wrote:
> * Lætitia Avrot (laetitia.av...@gmail.com) wrote:
>> As Amit Langot pointed out, the column_constraint definition is missing
>> whereas it is used in ALTER TABLE synopsis. It can be easily found in the
>> CREATE TABLE synopsis, but it's not very user friendly.
>
> Thanks, this looks pretty reasonable, but did you happen to look for any
> other keywords in the ALTER TABLE that should really be in ALTER TABLE
> also?
> 
> I'm specifically looking at, at least, partition_bound_spec.  Maybe you
> could propose an updated patch which addresses that also, and any other
> cases you find?

Ah, yes.  I remember having left out partition_bound_spec simply because I
thought it was kind of how it was supposed to be done, seeing that neither
column_constraint and table_constraint were expanded in the ALTER TABLE's
synopsis.

It seems that there are indeed a couple of other things that need to be
brought over to ALTER TABLE synopsis including partition_bound_spec.
9f295c08f877 [1] added table_constraint, but missed to add the description
of index_parameters and exclude_element which are referenced therein.

Attached find updated version of the Lætitia's patch.

Thanks,
Amit

[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9f295c
diff --git a/doc/src/sgml/ref/alter_table.sgml 
b/doc/src/sgml/ref/alter_table.sgml
index 41acda003f..e059f87875 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,6 +110,15 @@ 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 ]
@@ -104,6 +127,13 @@ ALTER TABLE [ IF EXISTS ] name
 
  
 
+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 } [, ...] )
+
+
  
   Description
 

-- 
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] Adding column_constraint description in ALTER TABLE synopsis

[Stephen Frost]

Greetings,

* Lætitia Avrot (laetitia.av...@gmail.com) wrote:
> As Amit Langot pointed out, the column_constraint definition is missing
> whereas it is used in ALTER TABLE synopsis. It can be easily found in the
> CREATE TABLE synopsis, but it's not very user friendly.

Agreed.

> You will find enclosed my patch.

Thanks, this looks pretty reasonable, but did you happen to look for any
other keywords in the ALTER TABLE that should really be in ALTER TABLE
also?

I'm specifically looking at, at least, partition_bound_spec.  Maybe you
could propose an updated patch which addresses that also, and any other
cases you find?

Thanks again!

Stephen


signature.asc
Description: Digital signature

[HACKERS] Adding column_constraint description in ALTER TABLE synopsis

[Lætitia Avrot]

Hello,

As Amit Langot pointed out, the column_constraint definition is missing
whereas it is used in ALTER TABLE synopsis. It can be easily found in the
CREATE TABLE synopsis, but it's not very user friendly.

I simply copied/paste the column_constraint definition from the CREATE
TABLE synopsis to the ALTER TABLE synopsis. I also had to change the first
word "where" to "and" as it's not the first definition in that synopsis.

I choose to add it above table_constraint as column_constraint is used
before table_constraint.

The patch should apply to MASTER (or I messed up with git). I built and
tested it successfully on my laptop.

You will find enclosed my patch.

Regards,

Lætitia
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 41acda0..c02dc38 100644
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***
*** 85,90  ALTER TABLE [ IF EXISTS ] name
--- 85,104 
  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 ]

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