[HACKERS] [PATCH] WIP Add ALWAYS DEFERRED option for constraints

[Nico Williams]

[make check-world passes.  Tests and docs included.  Should be ready for
code review.]

Attached are patches to add an ALWAYS DEFERRED option to CONSTRAINTs and
CONSTRAINT TRIGGERs, meaning: SET CONSTRAINTS .. IMMEDIATE will not make
immediate any constraint/trigger that is declared as ALWAYS DEFERRED.

I.e., the opposite of NOT DEFERRED.

Motivation:

 - Security.

   One may have triggers they need to always be deferred and they
   cannot give direct PG access because of SET CONSTRAINTS .. IMMEDIATE.

   I have such triggers that must run at the end of the transaction
   (after the last statement prior to COMMIT sent by the client/user),
   which I make DEFERRABLE, INITIALLY DEFERRED CONSTRAINT TRIGGERs.

   I have written SQL code to detect that constraint triggers have fired
   too soon, but I'd rather not need it as it does slow things down (it
   uses DDL event triggers and per-table triggers).

   Making it easier to write secure code DEFERRED CONSTRAINT TRIGGERs
   seems like a good idea to me.

 - Symmetry.

   Not using NOT DEFERRABLE is not the inverse of NOT DEFERRABLE.  There
   is no inverse at this time.

   If we can have NOT DEFERRABLE constraints, why not also the inverse,
   a constraint that cannot be made IMMEDIATE with SET CONSTRAINTs?


I've *not* cleaned up C style issues in surrounding -- I'm not sure
if that's desired.  Not cleaning up makes it easier to see what I
changed.

Some questions for experienced PostgreSQL developers:

Q0: Is this sort of patch welcomed?

Q1: Should new columns for pg_catalog tables go at the end, or may they
be added in the middle?

FYI, I'm adding them in the middle, so they are next to related
columns.

Q2: Can I add new columns to information_schema tables, or are there
standards-compliance issues with that?

This is done in the second patch, and it can be dropped safely.

Q3: Perhaps I should make this NOT IMMEDIATE rather than ALWAYS DEFERRED?
Making it NOT IMMEDIATE has the benefit of not having to change the
precedence of ALWAYS to avoid a shift/reduce conflict...  It may
also be more in keeping with NOT DEFERRED.  Thoughts?

Nico
-- 
>From 1d04483511f99cd3417df571ecc0498e928ace35 Mon Sep 17 00:00:00 2001
From: Nicolas Williams 
Date: Tue, 3 Oct 2017 00:33:09 -0500
Subject: [PATCH 1/2] 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/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 +++-
 32 files changed, 416 insertions(+), 91 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 

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

[Andreas Joseph Krogh]

På onsdag 04. oktober 2017 kl. 00:24:19, skrev Vik Fearing <
vik.fear...@2ndquadrant.com >:
On 10/03/2017 10:10 PM, Andreas Joseph Krogh wrote:
 > While we're in deferrable constraints land...;
 > I even more often need deferrable /conditional /unique-indexes.
 > In PG you now may have:
 >
 > ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, 
folder_type, name) DEFERRABLE INITIALLY DEFERRED;
 >
 > 
 > But this isn't supported:
 >
 > CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) 
WHERE parent_id IS NULL DEFERRABLE INITIALLY DEFERRED;
 >
 > Are there any plans to support this?

 I don't want to hijack the thread, but you can do that with exclusion
 constraints.
 
 
True.
 
--
 Andreas Joseph Krogh
 

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

[Vik Fearing]

On 10/03/2017 10:10 PM, Andreas Joseph Krogh wrote:
> While we're in deferrable constraints land...; 
> I even more often need deferrable /conditional /unique-indexes.
> In PG you now may have:
> 
> ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, 
> folder_type, name) DEFERRABLE INITIALLY DEFERRED;
> 
>  
> But this isn't supported:
> 
> CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) 
> WHERE parent_id IS NULL DEFERRABLE INITIALLY DEFERRED;
> 
> Are there any plans to support this?

I don't want to hijack the thread, but you can do that with exclusion
constraints.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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

[Nico Williams]

On Tue, Oct 03, 2017 at 02:51:30PM -0500, Nico Williams wrote:
> Anyways, this patch is NOT passing tests at the moment, and I'm not sure
> why.  I'm sure I can figure it out, but first I need to understand the
> failures.  E.g., I see this sort of difference:
> 
>\d testschema.test_index1
>Index "testschema.test_index1"
> Column |  Type  | Definition
>++
> id | bigint | id
>   -btree, for table "testschema.test_default_tab"
>   +f, for table "testschema.btree", predicate (test_default_tab)
> 
> which means, I think, that I've screwed up in src/bin/psql/describe.c,
> don't it's not obvious to me yet how.

Ah, I needed to adjust references to results columns.  I suspect that
something similar relates to other remaining failures.


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

[Nico Williams]

On Tue, Oct 03, 2017 at 10:10:59PM +0200, Andreas Joseph Krogh wrote:
> +1.
> 
> While we're in deferrable constraints land...;  I even more often need 
> deferrable conditional unique-indexes.
> In PG you now may have:
> ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, 
> folder_type, 
> name) DEFERRABLE INITIALLY DEFERRED; 
> 
> But this isn't supported:
> CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) 
> WHERE 
> parent_id IS NULL DEFERRABLE INITIALLY DEFERRED;
> 
> Are there any plans to support this?

Not by me, but I can take a look and, if it is trivial, do it.  At a
quick glance it does look like it should be easy enough to do it, at
least to get started on a patch.  

If I can get some help with my current patch, I'll take a look :)

But yes, I'd like to have full consistency between CREATE and ALTER.
Everything that one can do with CREATE should be possible to do with
ALTER, including IF NOT EXISTS.

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

[Andreas Joseph Krogh]

På tirsdag 03. oktober 2017 kl. 21:51:30, skrev Nico Williams <
n...@cryptonector.com >:
Attached are patches to add an ALWAYS DEFERRED option to CONSTRAINTs and
 CONSTRAINT TRIGGERs, meaning: SET CONSTRAINTS .. IMMEDIATE will not make
 immediate any constraint/trigger that is declared as ALWAYS DEFERRED.

 I.e., the opposite of NOT DEFERRED.  Perhaps I should make this NOT
 IMMEDIATE?  Making it NOT IMMEDIATE has the benefit of not having to
 change the precedence of ALWAYS to avoid a shift/reduce conflict...  It
 may also be more in keeping with NOT DEFERRED.

 Motivation:

  - I have trigger procedures that must run at the end of the transaction
    (after the last statement prior to COMMIT sent by the client/user),
    which I make DEFERRABLE, INITIALLY DEFERRED CONSTRAINT TRIGGERs out
    of, but SET CONSTRAINTS can be used to foil my triggers.  I have
    written SQL code to detect that constraint triggers have fired too
    soon, but I'd rather not need it.

  - Symmetry.  If we can have NOT DEFERRABLE constraints, why not also
    NOT IMMEDIABLE?  :)  Naturally "immediable" is not a word, but you
    get the point.

  - To learn my way around PostgreSQL source code in preparation for
    other contributions.

 Anyways, this patch is NOT passing tests at the moment, and I'm not sure
 why.  I'm sure I can figure it out, but first I need to understand the
 failures.  E.g., I see this sort of difference:

    \d testschema.test_index1
    Index "testschema.test_index1"
     Column |  Type  | Definition
    ++
     id     | bigint | id
   -btree, for table "testschema.test_default_tab"
   +f, for table "testschema.btree", predicate (test_default_tab)

 which means, I think, that I've screwed up in src/bin/psql/describe.c,
 don't it's not obvious to me yet how.

 Some questions for experienced PostgreSQL developers:

 Q0: Is this sort of patch welcomed?

 Q1: Should new columns for pg_catalog.pg_constraint go at the end, or may
     they be added in the middle?

 Q2: Can I add new columns to information_schema tables, or are there
     standards-compliance issues with that?

 Q3: Is the C-style for PG documented somewhere?  (sorry if I missed this)

 Q4: Any ideas what I'm doing wrong in this patch series?

 Nico
 
 
+1.
 
While we're in deferrable constraints land...;  I even more often need 
deferrable conditional unique-indexes.
In PG you now may have:
ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, folder_type, 
name) DEFERRABLE INITIALLY DEFERRED; 
 
But this isn't supported:
CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) WHERE 
parent_idIS NULL DEFERRABLE INITIALLY DEFERRED;  

Are there any plans to support this?
 
Thanks.



 
--
 Andreas Joseph Krogh
 

[HACKERS] [PATCH] WIP Add ALWAYS DEFERRED option for constraints

[Nico Williams]

Attached are patches to add an ALWAYS DEFERRED option to CONSTRAINTs and
CONSTRAINT TRIGGERs, meaning: SET CONSTRAINTS .. IMMEDIATE will not make
immediate any constraint/trigger that is declared as ALWAYS DEFERRED.

I.e., the opposite of NOT DEFERRED.  Perhaps I should make this NOT
IMMEDIATE?  Making it NOT IMMEDIATE has the benefit of not having to
change the precedence of ALWAYS to avoid a shift/reduce conflict...  It
may also be more in keeping with NOT DEFERRED.

Motivation:

 - I have trigger procedures that must run at the end of the transaction
   (after the last statement prior to COMMIT sent by the client/user),
   which I make DEFERRABLE, INITIALLY DEFERRED CONSTRAINT TRIGGERs out
   of, but SET CONSTRAINTS can be used to foil my triggers.  I have
   written SQL code to detect that constraint triggers have fired too
   soon, but I'd rather not need it.

 - Symmetry.  If we can have NOT DEFERRABLE constraints, why not also
   NOT IMMEDIABLE?  :)  Naturally "immediable" is not a word, but you
   get the point.

 - To learn my way around PostgreSQL source code in preparation for
   other contributions.

Anyways, this patch is NOT passing tests at the moment, and I'm not sure
why.  I'm sure I can figure it out, but first I need to understand the
failures.  E.g., I see this sort of difference:

   \d testschema.test_index1
   Index "testschema.test_index1"
Column |  Type  | Definition
   ++
id | bigint | id
  -btree, for table "testschema.test_default_tab"
  +f, for table "testschema.btree", predicate (test_default_tab)

which means, I think, that I've screwed up in src/bin/psql/describe.c,
don't it's not obvious to me yet how.

Some questions for experienced PostgreSQL developers:

Q0: Is this sort of patch welcomed?

Q1: Should new columns for pg_catalog.pg_constraint go at the end, or may
they be added in the middle?

Q2: Can I add new columns to information_schema tables, or are there
standards-compliance issues with that?

Q3: Is the C-style for PG documented somewhere?  (sorry if I missed this)

Q4: Any ideas what I'm doing wrong in this patch series?

Nico
-- 
>From 02f2765bde7e7d4fd357853c33dac55e4bdc2732 Mon Sep 17 00:00:00 2001
From: Nicolas Williams 
Date: Tue, 3 Oct 2017 00:33:09 -0500
Subject: [PATCH 1/4] WIP: 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).
---
 WIP| 25 +
 doc/src/sgml/catalogs.sgml |  7 +++
 doc/src/sgml/ref/alter_table.sgml  |  4 +-
 doc/src/sgml/ref/create_table.sgml | 10 ++--
 doc/src/sgml/ref/create_trigger.sgml   |  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 | 25 +++--
 src/backend/commands/typecmds.c|  3 ++
 src/backend/parser/gram.y  | 81 --
 src/backend/parser/parse_utilcmd.c |  1 +
 src/backend/utils/adt/ruleutils.c  |  2 +
 src/bin/pg_dump/pg_dump.c  | 23 +++--
 src/bin/pg_dump/pg_dump.h  |  2 +
 src/bin/psql/describe.c| 15 --
 src/include/catalog/index.h|  2 +
 src/include/catalog/pg_constraint.h|  4 +-
 src/include/catalog/pg_constraint_fn.h |  1 +
 src/include/catalog/pg_trigger.h   |  4 +-
 src/include/commands/trigger.h |  1 +
 src/include/nodes/parsenodes.h |  6 ++-
 src/include/utils/reltrigger.h |  1 +
 27 files changed, 221 insertions(+), 41 deletions(-)
 create mode 100644 WIP

diff --git a/WIP b/WIP
new file mode 100644
index 000..806df83
--- /dev/null
+++ b/WIP
@@ -0,0 +1,25 @@
+WIP notes for adding ALWAYS DEFERRED option for CONSTRAINTs and CONSTRAINT TRIGGERs
+===
+
+ - add ALWAYS DEFERRED syntax in src/backend/parser/gram.y (DONE)
+
+   (the existing NOT DEFERRABLE == ALWAYS IMMEDIATE, so we don't add that)
+
+ - add alwaysdeferred field to several structs -- wherever initdeferred
+   is defined, basically (DONE)
+
+ - add conalwaysdeferred column to pg_constraints
+
+ - in src/backend/commands/trigger.c modify places where all_isdeferred
+   and all_isset are used (DONE)
+
+ - add AFTER_TRIGGER_ALWAYSDEFERRED for AfterTriggerSharedData's
+   ats_event (struct) and fill it