Re: [HACKERS] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-11-07 Thread Nico Williams
On Mon, Nov 06, 2017 at 05:50:21PM +1300, Thomas Munro wrote:
> On Fri, Oct 20, 2017 at 9:05 AM, Nico Williams  wrote:
> > Rebased (there were conflicts in the SGML files).
> 
> Hi Nico
> 
> FYI that version has some stray absolute paths in constraints.source:
> 
> -COPY COPY_TBL FROM '@abs_srcdir@/data/constro.data';
> +COPY COPY_TBL FROM 
> '/home/nico/ws/postgres/src/test/regress/data/constro.data';
> 
> -COPY COPY_TBL FROM '@abs_srcdir@/data/constrf.data';
> +COPY COPY_TBL FROM 
> '/home/nico/ws/postgres/src/test/regress/data/constrf.data';

Oops!  Thanks for catching that!


-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-11-05 Thread Thomas Munro
On Fri, Oct 20, 2017 at 9:05 AM, Nico Williams  wrote:
> Rebased (there were conflicts in the SGML files).

Hi Nico

FYI that version has some stray absolute paths in constraints.source:

-COPY COPY_TBL FROM '@abs_srcdir@/data/constro.data';
+COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constro.data';

-COPY COPY_TBL FROM '@abs_srcdir@/data/constrf.data';
+COPY COPY_TBL FROM '/home/nico/ws/postgres/src/test/regress/data/constrf.data';

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


-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-11-03 Thread Nico Williams
On Fri, Nov 03, 2017 at 01:41:45PM -0400, Peter Eisentraut wrote:
> On 11/2/17 16:54, Nico Williams wrote:
> > Replacing condeferred and condeferrable with a char columns also
> > occurred to me, and though I assume backwards-incompatible changes to
> > pg_catalog tables are fair game, I assumed everyone would prefer
> > avoiding such changes where possible.
> 
> I don't think there is an overriding mandate to avoid such catalog
> changes.  Consider old clients that don't know about your new column.
> They might look at the catalog entries and derive information about a
> constraint, not being aware that there is additional information in
> another column that overrides that.  So in such cases it's arguably
> better to make a break.

Makes sense.

> (In any case, it might be worth waiting for a review of the rest of the
> patch before taking on a significant rewrite of the catalog structures.)

I'll wait then :)

When you're done with that I'll make this change (replacing those three
bool columns with a single char column).

> > Hmmm, must I do anything special about _downgrades_?  Does PostgreSQL
> > support downgrades?
> 
> no

Oh good.  Thanks for clarifying that.

Nico
-- 


-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-11-03 Thread Peter Eisentraut
On 11/2/17 16:54, Nico Williams wrote:
> Replacing condeferred and condeferrable with a char columns also
> occurred to me, and though I assume backwards-incompatible changes to
> pg_catalog tables are fair game, I assumed everyone would prefer
> avoiding such changes where possible.

I don't think there is an overriding mandate to avoid such catalog
changes.  Consider old clients that don't know about your new column.
They might look at the catalog entries and derive information about a
constraint, not being aware that there is additional information in
another column that overrides that.  So in such cases it's arguably
better to make a break.

(In any case, it might be worth waiting for a review of the rest of the
patch before taking on a significant rewrite of the catalog structures.)

> Hmmm, must I do anything special about _downgrades_?  Does PostgreSQL
> support downgrades?

no

-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-11-02 Thread Nico Williams
On Thu, Nov 02, 2017 at 04:20:19PM -0400, Peter Eisentraut wrote:
> I haven't really thought about this feature too hard; I just want to
> give you a couple of code comments.

Thanks!

> I think the catalog structure, and relatedly also the parser structures,
> could be made more compact.  We currently have condeferrable and
> condeferred to represent three valid states (NOT DEFERRABLE, DEFERRABLE
> INITIALLY IMMEDIATE, DEFERRABLE INITIALLY DEFERRED).  You are adding
> conalwaysdeferred, but you are adding only additional state (ALWAYS
> DEFERRED).  So we end up with three bool fields to represent four
> states.  I think this should all be rolled into one char field with four
> states.

I thought about this.  I couldn't see a way to make the two existing
boolean columns have a combination meaning "ALWAYS DEFERRED" that might
not break something else.

Since (condeferred AND NOT condeferrable) is an impossible combination
today, I could use it to mean ALWAYS DEFERRED.  I'm not sure how safe
that would be.  And it does seem like a weird way to express ALWAYS
DEFERRED, though it would work.

Replacing condeferred and condeferrable with a char columns also
occurred to me, and though I assume backwards-incompatible changes to
pg_catalog tables are fair game, I assumed everyone would prefer
avoiding such changes where possible.

Also, a backwards-incompatible change to the table would significantly
enlarge the patch, as more version checks would be needed, particularly
regarding upgrades (which are otherwise trivial).

I felt adding a new column was probably safest.  I'll make a backwards-
incompatible change if requested, naturally, but I guess I'd want to
get wider consensus on that, as I fear others may not agree.  That fear
may just be due to my ignorance of the community's preference as to
pg_catalog backwards-compatibility vs. cleanliness.

Hmmm, must I do anything special about _downgrades_?  Does PostgreSQL
support downgrades?

> In psql and pg_dump, if you are query new catalog fields, you need to
> have a version check to have a different query for >=PG11.  (This would
> likely apply whether you adopt my suggestion above or not.)

Ah, yes, of course.  I will add such a check.

> Maybe a test case in pg_dump would be useful.

Will do.

> Other than that, this looks like a pretty complete patch.

Thanks for the review!  It's a small-ish patch, and my very first for
PG.  It was fun writing it.  I greatly appreciate that PG source is easy
to read.

Nico
-- 


-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-11-02 Thread Peter Eisentraut
I haven't really thought about this feature too hard; I just want to
give you a couple of code comments.

I think the catalog structure, and relatedly also the parser structures,
could be made more compact.  We currently have condeferrable and
condeferred to represent three valid states (NOT DEFERRABLE, DEFERRABLE
INITIALLY IMMEDIATE, DEFERRABLE INITIALLY DEFERRED).  You are adding
conalwaysdeferred, but you are adding only additional state (ALWAYS
DEFERRED).  So we end up with three bool fields to represent four
states.  I think this should all be rolled into one char field with four
states.

In psql and pg_dump, if you are query new catalog fields, you need to
have a version check to have a different query for >=PG11.  (This would
likely apply whether you adopt my suggestion above or not.)

Maybe a test case in pg_dump would be useful.

Other than that, this looks like a pretty complete patch.

-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-10-19 Thread Nico Williams
Rebased (there were conflicts in the SGML files).

Nico
-- 
>From 80d284ecefa22945d507d2822f1f1a195e2af751 Mon Sep 17 00:00:00 2001
From: Nicolas Williams 
Date: Tue, 3 Oct 2017 00:33:09 -0500
Subject: [PATCH] Add ALWAYS DEFERRED option for CONSTRAINTs

and CONSTRAINT TRIGGERs.

This is important so that one can have triggers and constraints that
must run after all of the user/client's statements in a transaction
(i.e., at COMMIT time), so that the user/client may make no further
changes (triggers, of course, still can).
---
 doc/src/sgml/catalogs.sgml | 17 -
 doc/src/sgml/ref/alter_table.sgml  |  4 +-
 doc/src/sgml/ref/create_table.sgml | 10 ++-
 doc/src/sgml/ref/create_trigger.sgml   |  2 +-
 doc/src/sgml/trigger.sgml  |  1 +
 src/backend/bootstrap/bootparse.y  |  2 +
 src/backend/catalog/heap.c |  1 +
 src/backend/catalog/index.c|  8 +++
 src/backend/catalog/information_schema.sql |  8 +++
 src/backend/catalog/pg_constraint.c|  2 +
 src/backend/catalog/toasting.c |  2 +-
 src/backend/commands/indexcmds.c   |  2 +-
 src/backend/commands/tablecmds.c   | 20 +-
 src/backend/commands/trigger.c | 28 +++--
 src/backend/commands/typecmds.c|  3 +
 src/backend/nodes/copyfuncs.c  |  3 +
 src/backend/nodes/outfuncs.c   |  4 ++
 src/backend/parser/gram.y  | 99 ++
 src/backend/parser/parse_utilcmd.c | 46 +-
 src/backend/utils/adt/ruleutils.c  |  4 ++
 src/bin/pg_dump/pg_dump.c  | 31 --
 src/bin/pg_dump/pg_dump.h  |  2 +
 src/bin/psql/describe.c| 34 +++---
 src/bin/psql/tab-complete.c|  4 +-
 src/include/catalog/index.h|  2 +
 src/include/catalog/pg_constraint.h| 42 +++--
 src/include/catalog/pg_constraint_fn.h |  1 +
 src/include/catalog/pg_trigger.h   | 16 ++---
 src/include/commands/trigger.h |  1 +
 src/include/nodes/parsenodes.h |  6 +-
 src/include/utils/reltrigger.h |  1 +
 src/test/regress/input/constraints.source  | 51 +++
 src/test/regress/output/constraints.source | 54 +++-
 33 files changed, 418 insertions(+), 93 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index ef60a58..1bc35dc 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -,6 +,13 @@ SCRAM-SHA-256$iteration count:
  
 
  
+  conalwaysdeferred
+  bool
+  
+  Is the constraint always deferred?
+ 
+
+ 
   convalidated
   bool
   
@@ -6968,6 +6975,13 @@ SCRAM-SHA-256$iteration count:
  
 
  
+  tgalwaysdeferred
+  bool
+  
+  True if constraint trigger is always deferred
+ 
+
+ 
   tgnargs
   int2
   
@@ -7029,7 +7043,8 @@ SCRAM-SHA-256$iteration count:

 When tgconstraint is nonzero,
 tgconstrrelid, tgconstrindid,
-tgdeferrable, and tginitdeferred are
+tgdeferrable, tginitdeferred, and
+tgalwaysdeferred are
 largely redundant with the referenced pg_constraint entry.
 However, it is possible for a non-deferrable trigger to be associated
 with a deferrable constraint: foreign key constraints can have some
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index b4b8dab..fe24521 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -55,7 +55,7 @@ ALTER TABLE [ IF EXISTS ] name
 ALTER [ COLUMN ] column_name SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
 ADD table_constraint [ NOT VALID ]
 ADD table_constraint_using_index
-ALTER CONSTRAINT constraint_name [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
+ALTER CONSTRAINT constraint_name [ DEFERRABLE | NOT DEFERRABLE | ALWAYS DEFERRED ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
 VALIDATE CONSTRAINT constraint_name
 DROP CONSTRAINT [ IF EXISTS ]  constraint_name [ RESTRICT | CASCADE ]
 DISABLE TRIGGER [ trigger_name | ALL | USER ]
@@ -89,7 +89,7 @@ ALTER TABLE [ IF EXISTS ] name
 
 [ CONSTRAINT constraint_name ]
 { UNIQUE | PRIMARY KEY } USING INDEX index_name
-[ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
+[ DEFERRABLE | NOT DEFERRABLE | ALWAYS DEFERRED ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
 
  
 
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 2db2e9f..cf1ba1c 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -67,7 +67,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
   PRIMARY KEY index_parameters |
   REFERENCES reftable [ ( refcolumn ) ] [ MATCH 

Re: [HACKERS] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-10-11 Thread Nico Williams
FYI, I've added my patch to the commitfest.

https://commitfest.postgresql.org/15/1319/


-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-10-05 Thread Nico Williams
I accidentally typoed when saving a file.  Here's the new patch with
that typo corrected, changes to information_schema dropped, and with the
addition of tab completion of ALWAYS DEFERRED in psql.

Nico
-- 
>From 97d3db0be9307eff5919821db7fc437da52ef7e3 Mon Sep 17 00:00:00 2001
From: Nicolas Williams 
Date: Tue, 3 Oct 2017 00:33:09 -0500
Subject: [PATCH] Add ALWAYS DEFERRED option for CONSTRAINTs

and CONSTRAINT TRIGGERs.

This is important so that one can have triggers and constraints that
must run after all of the user/client's statements in a transaction
(i.e., at COMMIT time), so that the user/client may make no further
changes (triggers, of course, still can).
---
 doc/src/sgml/catalogs.sgml | 17 -
 doc/src/sgml/ref/alter_table.sgml  |  4 +-
 doc/src/sgml/ref/create_table.sgml | 10 ++-
 doc/src/sgml/ref/create_trigger.sgml   |  2 +-
 doc/src/sgml/trigger.sgml  |  1 +
 src/backend/bootstrap/bootparse.y  |  2 +
 src/backend/catalog/heap.c |  1 +
 src/backend/catalog/index.c|  8 +++
 src/backend/catalog/information_schema.sql |  8 +++
 src/backend/catalog/pg_constraint.c|  2 +
 src/backend/catalog/toasting.c |  2 +-
 src/backend/commands/indexcmds.c   |  2 +-
 src/backend/commands/tablecmds.c   | 20 +-
 src/backend/commands/trigger.c | 28 +++--
 src/backend/commands/typecmds.c|  3 +
 src/backend/nodes/copyfuncs.c  |  3 +
 src/backend/nodes/outfuncs.c   |  4 ++
 src/backend/parser/gram.y  | 99 ++
 src/backend/parser/parse_utilcmd.c | 46 +-
 src/backend/utils/adt/ruleutils.c  |  4 ++
 src/bin/pg_dump/pg_dump.c  | 31 --
 src/bin/pg_dump/pg_dump.h  |  2 +
 src/bin/psql/describe.c| 34 +++---
 src/bin/psql/tab-complete.c|  4 +-
 src/include/catalog/index.h|  2 +
 src/include/catalog/pg_constraint.h| 42 +++--
 src/include/catalog/pg_constraint_fn.h |  1 +
 src/include/catalog/pg_trigger.h   | 16 ++---
 src/include/commands/trigger.h |  1 +
 src/include/nodes/parsenodes.h |  6 +-
 src/include/utils/reltrigger.h |  1 +
 src/test/regress/input/constraints.source  | 51 +++
 src/test/regress/output/constraints.source | 54 +++-
 33 files changed, 418 insertions(+), 93 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9af77c1..2c3ed23 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2202,6 +2202,13 @@ SCRAM-SHA-256$iteration count:salt<
  
 
  
+  conalwaysdeferred
+  bool
+  
+  Is the constraint always deferred?
+ 
+
+ 
   convalidated
   bool
   
@@ -6948,6 +6955,13 @@ SCRAM-SHA-256$iteration count:salt<
  
 
  
+  tgalwaysdeferred
+  bool
+  
+  True if constraint trigger is always deferred
+ 
+
+ 
   tgnargs
   int2
   
@@ -7009,7 +7023,8 @@ SCRAM-SHA-256$iteration count:salt<

 When tgconstraint is nonzero,
 tgconstrrelid, tgconstrindid,
-tgdeferrable, and tginitdeferred are
+tgdeferrable, tginitdeferred, and
+tgalwaysdeferred are
 largely redundant with the referenced pg_constraint entry.
 However, it is possible for a non-deferrable trigger to be associated
 with a deferrable constraint: foreign key constraints can have some
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 0fb385e..e81d1fa 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -55,7 +55,7 @@ ALTER TABLE [ IF EXISTS ] name
 ALTER [ COLUMN ] column_name SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
 ADD table_constraint [ NOT VALID ]
 ADD table_constraint_using_index
-ALTER CONSTRAINT constraint_name [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
+ALTER CONSTRAINT constraint_name [ DEFERRABLE | NOT DEFERRABLE | ALWAYS DEFERRED ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
 VALIDATE CONSTRAINT constraint_name
 DROP CONSTRAINT [ IF EXISTS ]  constraint_name [ RESTRICT | CASCADE ]
 DISABLE TRIGGER [ trigger_name | ALL | USER ]
@@ -89,7 +89,7 @@ ALTER TABLE [ IF EXISTS ] name
 
 [ CONSTRAINT constraint_name ]
 { UNIQUE | PRIMARY KEY } USING INDEX index_name
-[ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
+[ DEFERRABLE | NOT DEFERRABLE | ALWAYS DEFERRED ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
 
  
 
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 1477288..38c88b8 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -67,7 +67,7 @@ 

Re: [HACKERS] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-10-04 Thread Nico Williams
Ah, David Fetter points out that I should also update tabe completion
for psql.  I'll do that at some point.  I notice there's no table
completion for column constraint attributes...  If it's obvious enough
I'll try to fix that too.


-- 
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] [PATCH] Add ALWAYS DEFERRED option for constraints

2017-10-04 Thread Nico Williams
Ay, NOT WIP -- I left that in the Subject: by accident.


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