Hi, Il 30/12/17 08:42, Craig Ringer ha scritto: > On 30 December 2017 at 03:32, Petr Jelinek <petr.jeli...@2ndquadrant.com > <mailto:petr.jeli...@2ndquadrant.com>> wrote: > > On 29/12/17 16:53, Marco Nenciarini wrote: > > Il 29/12/17 15:14, Petr Jelinek ha scritto: > >> > >> May be worth documenting that the session_replication_role also affects > >> TRUNCATE's interaction with FKs in config.sgml. > >> > > > > The current documentation of session_replication_role GUC is: > > > > Controls firing of replication-related triggers and rules for the > > current session. Setting this variable requires superuser privilege > > and results in discarding any previously cached query plans. > > Possible values are origin (the default), replica and local. > > See ALTER TABLE for more information. > > > > It doesn't speak about foreign keys or referential integrity, but only > > about triggers and rules. I don't think that it's worth to add a special > > case for truncate, unless we want to expand/rewrite the documentation to > > specify all the effects in the details. > > > > The effects on foreign keys is implied by the fact that for DML it's > implemented using triggers, but that's not true for TRUNCATE. In any > case it does not hurt to mention the FKs explicitly rather than > implicitly here. > > > Yeah. I'd argue that's an oversight in the current docs that "can cause > FK violations" isn't mentioned. That's kind of important, and FKs being > triggers is implementation detail we shouldn't be relying on users to know. >
I've tried to amend the documentation to be more clear. Feel free to suggest further editing. Patch v2 attached. Regards, Marco P.S: I'm struggling to understand why we have two possible values of session_replication_role settings that behave identically (origin and local). I'm unable to see any difference according to the code or the documentation, so I'm wondering if we should document that they are the same. -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e4a01699e4..aff56891e6 100644
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 6502,6508 **** COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
<listitem>
<para>
Controls firing of replication-related triggers and rules for the
! current session. Setting this variable requires
superuser privilege and results in discarding any previously cached
query plans. Possible values are <literal>origin</literal> (the
default),
<literal>replica</literal> and <literal>local</literal>.
--- 6502,6510 ----
<listitem>
<para>
Controls firing of replication-related triggers and rules for the
! current session. When set to <literal>replica</literal> it also
! disables all the foreign key checks, which can leave the data in an
! inconsistent state if improperly used. Setting this variable requires
superuser privilege and results in discarding any previously cached
query plans. Possible values are <literal>origin</literal> (the
default),
<literal>replica</literal> and <literal>local</literal>.
diff --git a/src/backend/commanindex d979ce266d..296807849f 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 1341,1356 **** ExecuteTruncate(TruncateStmt *stmt)
}
/*
! * Check foreign key references. In CASCADE mode, this should be
! * unnecessary since we just pulled in all the references; but as a
! * cross-check, do it anyway if in an Assert-enabled build.
*/
#ifdef USE_ASSERT_CHECKING
- heap_truncate_check_FKs(rels, false);
- #else
- if (stmt->behavior == DROP_RESTRICT)
heap_truncate_check_FKs(rels, false);
#endif
/*
* If we are asked to restart sequences, find all the sequences, lock
them
--- 1341,1364 ----
}
/*
! * Suppress foreign key references check if session replication role is
! * set to REPLICA.
*/
+ if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA)
+ {
+
+ /*
+ * Check foreign key references. In CASCADE mode, this should
be
+ * unnecessary since we just pulled in all the references; but
as a
+ * cross-check, do it anyway if in an Assert-enabled build.
+ */
#ifdef USE_ASSERT_CHECKING
heap_truncate_check_FKs(rels, false);
+ #else
+ if (stmt->behavior == DROP_RESTRICT)
+ heap_truncate_check_FKs(rels, false);
#endif
+ }
/*
* If we are asked to restart sequences, find all the sequences, lock
them
signature.asc
Description: OpenPGP digital signature
