]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix failure to ensure that a snapshot is available to datatype input functions
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 13 Dec 2008 02:00:53 +0000 (02:00 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 13 Dec 2008 02:00:53 +0000 (02:00 +0000)
when they are invoked by the parser.  We had been setting up a snapshot at
plan time but really it needs to be done earlier, before parse analysis.
Per report from Dmitry Koterov.

Also fix two related problems discovered while poking at this one:
exec_bind_message called datatype input functions without establishing a
snapshot, and SET CONSTRAINTS IMMEDIATE could call trigger functions without
establishing a snapshot.

Backpatch to 8.2.  The underlying problem goes much further back, but it is
masked in 8.1 and before because we didn't attempt to invoke domain check
constraints within datatype input.  It would only be exposed if a C-language
datatype input function used the snapshot; which evidently none do, or we'd
have heard complaints sooner.  Since this code has changed a lot over time,
a back-patch is hardly risk-free, and so I'm disinclined to patch further
than absolutely necessary.

src/backend/commands/trigger.c
src/backend/parser/analyze.c
src/backend/tcop/postgres.c
src/include/parser/analyze.h

index 0bc6f10cc792aa83ab09f71bdfe7f3f563ff6a1d..627be4d5813875509fa0e599d5ce2be026db6fc3 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.210.2.6 2008/10/25 03:32:51 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.210.2.7 2008/12/13 02:00:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3181,19 +3181,51 @@ AfterTriggerSetState(ConstraintsSetStmt *stmt)
        if (!stmt->deferred)
        {
                AfterTriggerEventList *events = &afterTriggers->events;
+               Snapshot        saveActiveSnapshot = ActiveSnapshot;
 
-               while (afterTriggerMarkEvents(events, NULL, true))
+               /* PG_TRY to ensure previous ActiveSnapshot is restored on error */
+               PG_TRY();
                {
-                       CommandId       firing_id = afterTriggers->firing_counter++;
+                       Snapshot        mySnapshot = NULL;
 
-                       /*
-                        * 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.
-                        */
-                       afterTriggerInvokeEvents(-1, firing_id, NULL,
-                                                                        !IsSubTransaction());
+                       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 (mySnapshot == NULL)
+                               {
+                                       mySnapshot = CopySnapshot(GetTransactionSnapshot());
+                                       ActiveSnapshot = mySnapshot;
+                               }
+
+                               /*
+                                * 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.
+                                */
+                               afterTriggerInvokeEvents(-1, firing_id, NULL,
+                                                                                !IsSubTransaction());
+                       }
+
+                       if (mySnapshot)
+                               FreeSnapshot(mySnapshot);
+               }
+               PG_CATCH();
+               {
+                       ActiveSnapshot = saveActiveSnapshot;
+                       PG_RE_THROW();
                }
+               PG_END_TRY();
+               ActiveSnapshot = saveActiveSnapshot;
        }
 }
 
index e1be8a89467290d1cf8cc9cfe8845d37379f2c88..226f7c763398a0e6a9d1134bb8ecf15762f0e43b 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *     $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.353.2.1 2007/06/20 18:21:08 tgl Exp $
+ *     $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.353.2.2 2008/12/13 02:00:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -432,6 +432,52 @@ transformStmt(ParseState *pstate, Node *parseTree,
        return result;
 }
 
+/*
+ * analyze_requires_snapshot
+ *             Returns true if a snapshot must be set before doing parse analysis
+ *             on the given raw parse tree.
+ *
+ * Classification here should match transformStmt().
+ */
+bool
+analyze_requires_snapshot(Node *parseTree)
+{
+       bool            result;
+
+       switch (nodeTag(parseTree))
+       {
+                       /*
+                        * Optimizable statements
+                        */
+               case T_InsertStmt:
+               case T_DeleteStmt:
+               case T_UpdateStmt:
+               case T_SelectStmt:
+                       result = true;
+                       break;
+
+                       /*
+                        * Special cases
+                        */
+               case T_DeclareCursorStmt:
+                       /* yes, because it's analyzed just like SELECT */
+                       result = true;
+                       break;
+
+               case T_ExplainStmt:
+                       /* yes, because it's analyzed just like SELECT */
+                       result = true;
+                       break;
+
+               default:
+                       /* other utility statements don't have any active parse analysis */
+                       result = false;
+                       break;
+       }
+
+       return result;
+}
+
 static Query *
 transformViewStmt(ParseState *pstate, ViewStmt *stmt,
                                  List **extras_before, List **extras_after)
index bcb116a50e4daf7c09c114d17368f107b84c7046..1ebcaa482b6dfb39ffda70dec23ff9d5186ff4fd 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.518.2.1 2007/01/04 00:58:01 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.518.2.2 2008/12/13 02:00:52 tgl Exp $
  *
  * NOTES
  *       this is the "main" module of the postgres backend and
@@ -655,6 +655,9 @@ pg_plan_query(Query *querytree, ParamListInfo boundParams)
        if (querytree->commandType == CMD_UTILITY)
                return NULL;
 
+       /* Planner must have a snapshot in case it calls user-defined functions. */
+       Assert(ActiveSnapshot != NULL);
+
        if (log_planner_stats)
                ResetUsage();
 
@@ -823,6 +826,7 @@ exec_simple_query(const char *query_string)
        foreach(parsetree_item, parsetree_list)
        {
                Node       *parsetree = (Node *) lfirst(parsetree_item);
+               Snapshot        mySnapshot = NULL;
                const char *commandTag;
                char            completionTag[COMPLETION_TAG_BUFSIZE];
                List       *querytree_list,
@@ -864,6 +868,15 @@ exec_simple_query(const char *query_string)
                /* If we got a cancel signal in parsing or prior command, quit */
                CHECK_FOR_INTERRUPTS();
 
+               /*
+                * Set up a snapshot if parse analysis/planning will need one.
+                */
+               if (analyze_requires_snapshot(parsetree))
+               {
+                       mySnapshot = CopySnapshot(GetTransactionSnapshot());
+                       ActiveSnapshot = mySnapshot;
+               }
+
                /*
                 * OK to analyze, rewrite, and plan this query.
                 *
@@ -875,7 +888,12 @@ exec_simple_query(const char *query_string)
                querytree_list = pg_analyze_and_rewrite(parsetree, query_string,
                                                                                                NULL, 0);
 
-               plantree_list = pg_plan_queries(querytree_list, NULL, true);
+               plantree_list = pg_plan_queries(querytree_list, NULL, false);
+
+               /* Done with the snapshot used for parsing/planning */
+               ActiveSnapshot = NULL;
+               if (mySnapshot)
+                       FreeSnapshot(mySnapshot);
 
                /* If we got a cancel signal in analysis or planning, quit */
                CHECK_FOR_INTERRUPTS();
@@ -1127,6 +1145,7 @@ exec_parse_message(const char *query_string,      /* string to execute */
        if (parsetree_list != NIL)
        {
                Node       *parsetree = (Node *) linitial(parsetree_list);
+               Snapshot        mySnapshot = NULL;
                int                     i;
 
                /*
@@ -1149,6 +1168,15 @@ exec_parse_message(const char *query_string,     /* string to execute */
                                         errmsg("current transaction is aborted, "
                                                "commands ignored until end of transaction block")));
 
+               /*
+                * Set up a snapshot if parse analysis/planning will need one.
+                */
+               if (analyze_requires_snapshot(parsetree))
+               {
+                       mySnapshot = CopySnapshot(GetTransactionSnapshot());
+                       ActiveSnapshot = mySnapshot;
+               }
+
                /*
                 * OK to analyze, rewrite, and plan this query.  Note that the
                 * originally specified parameter set is not required to be complete,
@@ -1191,7 +1219,12 @@ exec_parse_message(const char *query_string,     /* string to execute */
                if (!is_named && numParams > 0)
                        plantree_list = NIL;
                else
-                       plantree_list = pg_plan_queries(querytree_list, NULL, true);
+                       plantree_list = pg_plan_queries(querytree_list, NULL, false);
+
+               /* Done with the snapshot used for parsing/planning */
+               ActiveSnapshot = NULL;
+               if (mySnapshot)
+                       FreeSnapshot(mySnapshot);
        }
        else
        {
@@ -1401,10 +1434,18 @@ exec_bind_message(StringInfo input_message)
         */
        if (numParams > 0)
        {
+               Snapshot        mySnapshot;
                ListCell   *l;
                MemoryContext oldContext;
                int                     paramno;
 
+               /*
+                * Set a snapshot if we have parameters to fetch (since the input
+                * functions might need it).
+                */
+               mySnapshot = CopySnapshot(GetTransactionSnapshot());
+               ActiveSnapshot = mySnapshot;
+
                oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
 
                /* sizeof(ParamListInfoData) includes the first array element */
@@ -1536,6 +1577,10 @@ exec_bind_message(StringInfo input_message)
                }
 
                MemoryContextSwitchTo(oldContext);
+
+               /* Done with the snapshot used for parameter I/O */
+               ActiveSnapshot = NULL;
+               FreeSnapshot(mySnapshot);
        }
        else
                params = NULL;
@@ -3285,6 +3330,9 @@ PostgresMain(int argc, char *argv[], const char *username)
                 */
                debug_query_string = NULL;
 
+               /* No active snapshot any more either */
+               ActiveSnapshot = NULL;
+
                /*
                 * Abort the current transaction in order to recover.
                 */
index 7866472f79ea72f7809f8e888666381b649f7d78..c90517d305d88a8e0eced5970e2d047bf9815064 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.34 2006/10/04 00:30:09 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.34.2.1 2008/12/13 02:00:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,6 +21,7 @@ extern List *parse_analyze(Node *parseTree, const char *sourceText,
 extern List *parse_analyze_varparams(Node *parseTree, const char *sourceText,
                                                Oid **paramTypes, int *numParams);
 extern List *parse_sub_analyze(Node *parseTree, ParseState *parentParseState);
+extern bool analyze_requires_snapshot(Node *parseTree);
 extern List *analyzeCreateSchemaStmt(CreateSchemaStmt *stmt);
 extern void CheckSelectLocking(Query *qry);
 extern void applyLockingClause(Query *qry, Index rtindex,