diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 33da6632cae..96ec841f847 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.227.2.1 2008/10/25 03:32:44 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.227.2.2 2008/12/13 02:00:29 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3469,19 +3469,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; } } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 759dc64d940..4a539a1e1e8 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -17,7 +17,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.371 2008/01/01 19:45:50 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.371.2.1 2008/12/13 02:00:29 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -227,6 +227,55 @@ 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: + /* + * We only need a snapshot in varparams case, but it doesn't seem + * worth complicating this function's API to distinguish that. + */ + result = true; + break; + + default: + /* utility statements don't have any active parse analysis */ + result = false; + break; + } + + return result; +} + /* * transformDeleteStmt - * transforms a Delete Statement diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index c3531b58449..85a266172f8 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.542.2.2 2008/04/02 18:32:00 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.542.2.3 2008/12/13 02:00:29 tgl Exp $ * * NOTES * this is the "main" module of the postgres backend and @@ -674,6 +674,9 @@ pg_plan_query(Query *querytree, int cursorOptions, 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(); @@ -866,6 +869,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, @@ -907,6 +911,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. * @@ -918,7 +931,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, 0, NULL, true); + plantree_list = pg_plan_queries(querytree_list, 0, 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(); @@ -933,7 +951,7 @@ exec_simple_query(const char *query_string) /* * We don't have to copy anything into the portal, because everything - * we are passsing here is in MessageContext, which will outlive the + * we are passing here is in MessageContext, which will outlive the * portal anyway. */ PortalDefineQuery(portal, @@ -1168,6 +1186,7 @@ exec_parse_message(const char *query_string, /* string to execute */ if (parsetree_list != NIL) { Query *query; + Snapshot mySnapshot = NULL; int i; raw_parse_tree = (Node *) linitial(parsetree_list); @@ -1192,6 +1211,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(raw_parse_tree)) + { + 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, @@ -1239,9 +1267,14 @@ exec_parse_message(const char *query_string, /* string to execute */ } else { - stmt_list = pg_plan_queries(querytree_list, 0, NULL, true); + stmt_list = pg_plan_queries(querytree_list, 0, NULL, false); fully_planned = true; } + + /* Done with the snapshot used for parsing/planning */ + ActiveSnapshot = NULL; + if (mySnapshot) + FreeSnapshot(mySnapshot); } else { @@ -1365,6 +1398,7 @@ exec_bind_message(StringInfo input_message) List *plan_list; MemoryContext oldContext; bool save_log_statement_stats = log_statement_stats; + Snapshot mySnapshot = NULL; char msec_str[32]; /* Get the fixed part of the message */ @@ -1486,6 +1520,17 @@ exec_bind_message(StringInfo input_message) else saved_stmt_name = NULL; + /* + * Set a snapshot if we have parameters to fetch (since the input + * functions might need it) or the query isn't a utility command (and + * hence could require redoing parse analysis and planning). + */ + if (numParams > 0 || analyze_requires_snapshot(psrc->raw_parse_tree)) + { + mySnapshot = CopySnapshot(GetTransactionSnapshot()); + ActiveSnapshot = mySnapshot; + } + /* * Fetch parameters, if any, and store in the portal's memory context. */ @@ -1674,7 +1719,7 @@ exec_bind_message(StringInfo input_message) */ oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal)); query_list = copyObject(cplan->stmt_list); - plan_list = pg_plan_queries(query_list, 0, params, true); + plan_list = pg_plan_queries(query_list, 0, params, false); MemoryContextSwitchTo(oldContext); /* We no longer need the cached plan refcount ... */ @@ -1683,6 +1728,11 @@ exec_bind_message(StringInfo input_message) cplan = NULL; } + /* Done with the snapshot used for parameter I/O and parsing/planning */ + ActiveSnapshot = NULL; + if (mySnapshot) + FreeSnapshot(mySnapshot); + /* * Define portal and start execution. */ @@ -3434,6 +3484,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. */ diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index bbfdd888c1c..5019df6f86a 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -33,7 +33,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.15.2.1 2008/09/15 23:37:49 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.15.2.2 2008/12/13 02:00:29 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -70,7 +70,6 @@ static List *cached_plans_list = NIL; static void StoreCachedPlan(CachedPlanSource *plansource, List *stmt_list, MemoryContext plan_context); -static List *do_planning(List *querytrees, int cursorOptions); static void AcquireExecutorLocks(List *stmt_list, bool acquire); static void AcquirePlannerLocks(List *stmt_list, bool acquire); static void LockRelid(Oid relid, LOCKMODE lockmode, void *arg); @@ -457,8 +456,8 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner) */ if (!plan) { + Snapshot saveActiveSnapshot = ActiveSnapshot; List *slist; - TupleDesc resultDesc; /* * Restore the search_path that was in use when the plan was made. @@ -467,50 +466,86 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner) PushOverrideSearchPath(plansource->search_path); /* - * Run parse analysis and rule rewriting. The parser tends to - * scribble on its input, so we must copy the raw parse tree to - * prevent corruption of the cache. Note that we do not use - * parse_analyze_varparams(), assuming that the caller never wants the - * parameter types to change from the original values. + * If a snapshot is already set (the normal case), we can just use + * that for parsing/planning. But if it isn't, install one. We must + * arrange to restore ActiveSnapshot afterward, to ensure that + * RevalidateCachedPlan has no caller-visible effects on the + * snapshot. Having to replan is an unusual case, and it seems a + * really bad idea for RevalidateCachedPlan to affect the snapshot + * only in unusual cases. (Besides, the snap might have been created + * in a short-lived context.) */ - slist = pg_analyze_and_rewrite(copyObject(plansource->raw_parse_tree), - plansource->query_string, - plansource->param_types, - plansource->num_params); - - if (plansource->fully_planned) + PG_TRY(); { - /* Generate plans for queries */ - slist = do_planning(slist, plansource->cursor_options); - } + Snapshot mySnapshot = NULL; + TupleDesc resultDesc; - /* - * Check or update the result tupdesc. XXX should we use a weaker - * condition than equalTupleDescs() here? - */ - resultDesc = PlanCacheComputeResultDesc(slist); - if (resultDesc == NULL && plansource->resultDesc == NULL) - { - /* OK, doesn't return tuples */ - } - else if (resultDesc == NULL || plansource->resultDesc == NULL || - !equalTupleDescs(resultDesc, plansource->resultDesc)) - { - MemoryContext oldcxt; + if (ActiveSnapshot == NULL) + { + mySnapshot = CopySnapshot(GetTransactionSnapshot()); + ActiveSnapshot = mySnapshot; + } - /* can we give a better error message? */ - if (plansource->fixed_result) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cached plan must not change result type"))); - oldcxt = MemoryContextSwitchTo(plansource->context); - if (resultDesc) - resultDesc = CreateTupleDescCopy(resultDesc); - if (plansource->resultDesc) - FreeTupleDesc(plansource->resultDesc); - plansource->resultDesc = resultDesc; - MemoryContextSwitchTo(oldcxt); + /* + * Run parse analysis and rule rewriting. The parser tends to + * scribble on its input, so we must copy the raw parse tree to + * prevent corruption of the cache. Note that we do not use + * parse_analyze_varparams(), assuming that the caller never wants + * the parameter types to change from the original values. + */ + slist = pg_analyze_and_rewrite(copyObject(plansource->raw_parse_tree), + plansource->query_string, + plansource->param_types, + plansource->num_params); + + if (plansource->fully_planned) + { + /* Generate plans for queries */ + slist = pg_plan_queries(slist, plansource->cursor_options, + NULL, false); + } + + /* + * Check or update the result tupdesc. XXX should we use a weaker + * condition than equalTupleDescs() here? + */ + resultDesc = PlanCacheComputeResultDesc(slist); + if (resultDesc == NULL && plansource->resultDesc == NULL) + { + /* OK, doesn't return tuples */ + } + else if (resultDesc == NULL || plansource->resultDesc == NULL || + !equalTupleDescs(resultDesc, plansource->resultDesc)) + { + MemoryContext oldcxt; + + /* can we give a better error message? */ + if (plansource->fixed_result) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cached plan must not change result type"))); + oldcxt = MemoryContextSwitchTo(plansource->context); + if (resultDesc) + resultDesc = CreateTupleDescCopy(resultDesc); + if (plansource->resultDesc) + FreeTupleDesc(plansource->resultDesc); + plansource->resultDesc = resultDesc; + MemoryContextSwitchTo(oldcxt); + } + + /* Done with snapshot */ + if (mySnapshot) + FreeSnapshot(mySnapshot); } + PG_CATCH(); + { + /* Restore global vars and propagate error */ + ActiveSnapshot = saveActiveSnapshot; + PG_RE_THROW(); + } + PG_END_TRY(); + + ActiveSnapshot = saveActiveSnapshot; /* Now we can restore current search path */ PopOverrideSearchPath(); @@ -536,49 +571,6 @@ RevalidateCachedPlan(CachedPlanSource *plansource, bool useResOwner) return plan; } -/* - * Invoke the planner on some rewritten queries. This is broken out of - * RevalidateCachedPlan just to avoid plastering "volatile" all over that - * function's variables. - */ -static List * -do_planning(List *querytrees, int cursorOptions) -{ - List *stmt_list; - - /* - * If a snapshot is already set (the normal case), we can just use that - * for planning. But if it isn't, we have to tell pg_plan_queries to make - * a snap if it needs one. In that case we should arrange to reset - * ActiveSnapshot afterward, to ensure that RevalidateCachedPlan has no - * caller-visible effects on the snapshot. Having to replan is an unusual - * case, and it seems a really bad idea for RevalidateCachedPlan to affect - * the snapshot only in unusual cases. (Besides, the snap might have been - * created in a short-lived context.) - */ - if (ActiveSnapshot != NULL) - stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, false); - else - { - PG_TRY(); - { - stmt_list = pg_plan_queries(querytrees, cursorOptions, NULL, true); - } - PG_CATCH(); - { - /* Restore global vars and propagate error */ - ActiveSnapshot = NULL; - PG_RE_THROW(); - } - PG_END_TRY(); - - ActiveSnapshot = NULL; - } - - return stmt_list; -} - - /* * ReleaseCachedPlan: release active use of a cached plan. * diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h index eb491329426..7d9361268ca 100644 --- a/src/include/parser/analyze.h +++ b/src/include/parser/analyze.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.38 2008/01/01 19:45:58 momjian Exp $ + * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.38.2.1 2008/12/13 02:00:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -25,6 +25,8 @@ extern Query *parse_analyze_varparams(Node *parseTree, const char *sourceText, extern Query *parse_sub_analyze(Node *parseTree, ParseState *parentParseState); extern Query *transformStmt(ParseState *pstate, Node *parseTree); +extern bool analyze_requires_snapshot(Node *parseTree); + extern void CheckSelectLocking(Query *qry); extern void applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait);