On 12/29/17 07:01, Marco Nenciarini wrote:
> The current behavior of session_replication_role = replica with TRUNCATE
> is not the same of with the other commands.
> It does not check FKs for INSERT/UPDATE/DELETE but it does for TRUNCATE,
> so one cannot execute TRUNCATE on a table when it is possible to DELETE
> from table without WHERE clause.
> 
> I'm attaching a simple patch to make TRUNCATE match behavior of DELETE
> for session_replication_role = replica.

I'm wondering whether this shouldn't be fixed the other way around,
namely have foreign keys always enforced on replicas.

I'm not aware of an explanation why it currently works the way it does,
other than that FKs happen to be implemented by triggers and triggers
happen to work that way.  But I think it's pretty bogus that logical
replication subscriptions can insert data that violates constraints.
It's also weird that you can violate deferred unique constraints but not
immediate ones (I think, not tested).

So I'm proposing the attached alternative patch, which creates
constraint triggers to be TRIGGER_FIRES_ALWAYS by default.

Thoughts?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From aa22bdd6428ae3df1dbf0f0e31f9431dd374d8c5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 18 Jan 2018 12:06:45 -0500
Subject: [PATCH] Create constraint triggers to fire ALWAYS

This ensures that all constraints are also enforced on logical replicas.
---
 src/backend/commands/trigger.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 1c488c338a..e67f981029 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -143,6 +143,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
                          bool isInternal)
 {
        int16           tgtype;
+       char            tgenabled;
        int                     ncolumns;
        int16      *columns;
        int2vector *tgattr;
@@ -327,6 +328,12 @@ CreateTrigger(CreateTrigStmt *stmt, const char 
*queryString,
                                         errmsg("INSTEAD OF triggers cannot 
have column lists")));
        }
 
+       /*
+        * Constraint triggers fire always, normal triggers only on origin by
+        * default.
+        */
+       tgenabled = stmt->isconstraint ? TRIGGER_FIRES_ALWAYS : 
TRIGGER_FIRES_ON_ORIGIN;
+
        /*
         * We don't yet support naming ROW transition variables, but the parser
         * recognizes the syntax so we can give a nicer message here.
@@ -741,7 +748,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
                                                                                
                                         CStringGetDatum(trigname));
        values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid);
        values[Anum_pg_trigger_tgtype - 1] = Int16GetDatum(tgtype);
-       values[Anum_pg_trigger_tgenabled - 1] = 
CharGetDatum(TRIGGER_FIRES_ON_ORIGIN);
+       values[Anum_pg_trigger_tgenabled - 1] = CharGetDatum(tgenabled);
        values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal);
        values[Anum_pg_trigger_tgconstrrelid - 1] = 
ObjectIdGetDatum(constrrelid);
        values[Anum_pg_trigger_tgconstrindid - 1] = ObjectIdGetDatum(indexOid);
-- 
2.15.1

Reply via email to