On Mon, May 11, 2020 at 05:13:38PM -0700, David G. Johnston wrote:
> Skimming through the code in event_trigger.c and noticed that while most of
> the stanzas that reference IsUnderPostmaster refer back to the code comment
> beginning on line 675 the block for table rewrite copied it in
> verbatim starting at line 842.  The currentEventTriggerState comment at
> lines 730 and 861 seem to be the same too.

An even more interesting part here is that EventTriggerDDLCommandEnd()
and Drop() have basically the same comments, but they tell to refer
back toEventTriggerDDLCommandStart().  So let's just do the same for
all the exact duplicate in EventTriggerTableRewrite().

The second point about the check with (!currentEventTriggerState) in
EventTriggerTableRewrite() and EventTriggerDDLCommandEnd() shows that
both comments share the same first sentence, but there is enough
different context to just keep them as separate IMO.

> I did also notice a difference with the test on line 861 compared to line
> 785 though I unable to evaluate whether the absence of a "rewriteList" is
> expected (there is a "dropList" at 785 ...).

An event table rewrite happens only for one relation at a time.

In short, something like the attached sounds enough to me.  What do
you think?
--
Michael
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 91800d1fac..5d90281350 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -839,20 +839,8 @@ EventTriggerTableRewrite(Node *parsetree, Oid tableOid, int reason)
 	EventTriggerData trigdata;
 
 	/*
-	 * Event Triggers are completely disabled in standalone mode.  There are
-	 * (at least) two reasons for this:
-	 *
-	 * 1. A sufficiently broken event trigger might not only render the
-	 * database unusable, but prevent disabling itself to fix the situation.
-	 * In this scenario, restarting in standalone mode provides an escape
-	 * hatch.
-	 *
-	 * 2. BuildEventTriggerCache relies on systable_beginscan_ordered, and
-	 * therefore will malfunction if pg_event_trigger's indexes are damaged.
-	 * To allow recovery from a damaged index, we need some operating mode
-	 * wherein event triggers are disabled.  (Or we could implement
-	 * heapscan-and-sort logic for that case, but having disaster recovery
-	 * scenarios depend on code that's otherwise untested isn't appetizing.)
+	 * See EventTriggerDDLCommandStart for a discussion about why event
+	 * triggers are disabled in single user mode.
 	 */
 	if (!IsUnderPostmaster)
 		return;

Attachment: signature.asc
Description: PGP signature

Reply via email to