Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c76d0ee

Browse files
committed
Fix race conditions when an event trigger is added concurrently with DDL.
EventTriggerTableRewrite crashed if there were table_rewrite triggers present, but there had not been when the calling command started. EventTriggerDDLCommandEnd called ddl_command_end triggers if present, even if there had been no such triggers when the calling command started, which would lead to a failure in pg_event_trigger_ddl_commands. In both cases, fix by doing nothing; it's better to wait till the next command when things will be properly initialized. In passing, remove an elog(DEBUG1) call that might have seemed interesting four years ago but surely isn't today. We found this because of intermittent failures in the buildfarm. Thanks to Alvaro Herrera and Andrew Gierth for analysis. Back-patch to 9.5; some of this code exists before that, but the specific hazards we need to guard against don't. Discussion: https://postgr.es/m/5767.1523995174@sss.pgh.pa.us
1 parent 64ad858 commit c76d0ee

File tree

1 file changed

+25
-3
lines changed

1 file changed

+25
-3
lines changed

src/backend/commands/event_trigger.c

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,19 @@ EventTriggerDDLCommandEnd(Node *parsetree)
834834
if (!IsUnderPostmaster)
835835
return;
836836

837+
/*
838+
* Also do nothing if our state isn't set up, which it won't be if there
839+
* weren't any relevant event triggers at the start of the current DDL
840+
* command. This test might therefore seem optional, but it's important
841+
* because EventTriggerCommonSetup might find triggers that didn't exist
842+
* at the time the command started. Although this function itself
843+
* wouldn't crash, the event trigger functions would presumably call
844+
* pg_event_trigger_ddl_commands which would fail. Better to do nothing
845+
* until the next command.
846+
*/
847+
if (!currentEventTriggerState)
848+
return;
849+
837850
runlist = EventTriggerCommonSetup(parsetree,
838851
EVT_DDLCommandEnd, "ddl_command_end",
839852
&trigdata);
@@ -885,9 +898,10 @@ EventTriggerSQLDrop(Node *parsetree)
885898
&trigdata);
886899

887900
/*
888-
* Nothing to do if run list is empty. Note this shouldn't happen,
901+
* Nothing to do if run list is empty. Note this typically can't happen,
889902
* because if there are no sql_drop events, then objects-to-drop wouldn't
890903
* have been collected in the first place and we would have quit above.
904+
* But it could occur if event triggers were dropped partway through.
891905
*/
892906
if (runlist == NIL)
893907
return;
@@ -934,8 +948,6 @@ EventTriggerTableRewrite(Node *parsetree, Oid tableOid, int reason)
934948
List *runlist;
935949
EventTriggerData trigdata;
936950

937-
elog(DEBUG1, "EventTriggerTableRewrite(%u)", tableOid);
938-
939951
/*
940952
* Event Triggers are completely disabled in standalone mode. There are
941953
* (at least) two reasons for this:
@@ -955,6 +967,16 @@ EventTriggerTableRewrite(Node *parsetree, Oid tableOid, int reason)
955967
if (!IsUnderPostmaster)
956968
return;
957969

970+
/*
971+
* Also do nothing if our state isn't set up, which it won't be if there
972+
* weren't any relevant event triggers at the start of the current DDL
973+
* command. This test might therefore seem optional, but it's
974+
* *necessary*, because EventTriggerCommonSetup might find triggers that
975+
* didn't exist at the time the command started.
976+
*/
977+
if (!currentEventTriggerState)
978+
return;
979+
958980
runlist = EventTriggerCommonSetup(parsetree,
959981
EVT_TableRewrite,
960982
"table_rewrite",

0 commit comments

Comments
 (0)