Re: So, why shouldn't SET CONSTRAINTS set a transaction snapshot?

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tom Lane wrote:
>> 1. Always set a snapshot for SET CONSTRAINTS. This is a minus-one-liner
>> --- just remove it from the exclusion list in PortalRunUtility.
>>
>> 2. Have it set a snapshot only if it finds pending trigger events to
>> fire. This would only require another half dozen lines of code, but
>> it's certainly more complicated than choice #1.

> It seems to me there is room for a backwards compatibility argument
> here. How about doing #2 for 8.3 and back, and #1 for 8.4?

Well, if you think there's a real backwards compatibility issue, we
should just do #2 and be done with it. It's not like it's enough code
to really matter in the big scheme of things.

Index: src/backend/commands/trigger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/trigger.c,v
retrieving revision 1.241
diff -c -r1.241 trigger.c
*** src/backend/commands/trigger.c 21 Nov 2008 20:14:27 -0000 1.241
--- src/backend/commands/trigger.c 12 Dec 2008 23:08:41 -0000
***************
*** 3716,3727 ****
--- 3716,3743 ----
if (!stmt->deferred)
{
AfterTriggerEventList *events = &afterTriggers->events;
+ bool snapshot_set = false;

while (afterTriggerMarkEvents(events, NULL, true))
{
CommandId firing_id = afterTriggers->firing_counter++;

/*
+ * Make sure a snapshot has been established in case trigger
+ * functions need one. Note that we avoid setting a snapshot if
+ * we don't find at least one trigger that has to be fired now.
+ * This is so that BEGIN; SET CONSTRAINTS ...; SET TRANSACTION
+ * ISOLATION LEVEL SERIALIZABLE; ... works properly. (If we are
+ * at the start of a transaction it's not possible for any trigger
+ * events to be queued yet.)
+ */
+ if (!snapshot_set)
+ {
+ PushActiveSnapshot(GetTransactionSnapshot());
+ snapshot_set = true;
+ }
+
+ /*
* We can delete fired events if we are at top transaction level,
* but we'd better not if inside a subtransaction, since the
* subtransaction could later get rolled back.
***************
*** 3730,3735 ****
--- 3746,3754 ----
!IsSubTransaction()))
break; /* all fired */
}
+
+ if (snapshot_set)
+ PopActiveSnapshot();
}
}

regards, tom lane