From c0b00760365c74308e9e0719c993eadfbcd090c2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 28 Feb 2011 23:27:18 -0500 Subject: [PATCH] Rearrange snapshot handling to make rule expansion more consistent. With this patch, portals, SQL functions, and SPI all agree that there should be only a CommandCounterIncrement between the queries that are generated from a single SQL command by rule expansion. Fetching a whole new snapshot now happens only between original queries. This is equivalent to the existing behavior of EXPLAIN ANALYZE, and it was judged to be the best choice since it eliminates one source of concurrency hazards for rules. The patch should also make things marginally faster by reducing the number of snapshot push/pop operations. The patch removes pg_parse_and_rewrite(), which is no longer used anywhere. There was considerable discussion about more aggressive refactoring of the query-processing functions exported by postgres.c, but for the moment nothing more has been done there. I also took the opportunity to refactor snapmgr.c's API slightly: the former PushUpdatedSnapshot() has been split into two functions. Marko Tiikkaja, reviewed by Steve Singer and Tom Lane --- src/backend/catalog/pg_proc.c | 27 ++- src/backend/commands/copy.c | 3 +- src/backend/commands/explain.c | 3 +- src/backend/executor/functions.c | 346 ++++++++++++++++++++++--------- src/backend/executor/spi.c | 96 +++++---- src/backend/tcop/postgres.c | 41 ---- src/backend/tcop/pquery.c | 52 ++++- src/backend/utils/time/snapmgr.c | 33 ++- src/include/tcop/tcopprot.h | 2 - src/include/utils/snapmgr.h | 3 +- 10 files changed, 389 insertions(+), 217 deletions(-) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 3f3877da28..2523653f37 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -764,7 +764,9 @@ fmgr_sql_validator(PG_FUNCTION_ARGS) Oid funcoid = PG_GETARG_OID(0); HeapTuple tuple; Form_pg_proc proc; + List *raw_parsetree_list; List *querytree_list; + ListCell *lc; bool isnull; Datum tmp; char *prosrc; @@ -835,17 +837,32 @@ fmgr_sql_validator(PG_FUNCTION_ARGS) * We can run the text through the raw parser though; this will at * least catch silly syntactic errors. */ + raw_parsetree_list = pg_parse_query(prosrc); + if (!haspolyarg) { - querytree_list = pg_parse_and_rewrite(prosrc, - proc->proargtypes.values, - proc->pronargs); + /* + * OK to do full precheck: analyze and rewrite the queries, + * then verify the result type. + */ + querytree_list = NIL; + foreach(lc, raw_parsetree_list) + { + Node *parsetree = (Node *) lfirst(lc); + List *querytree_sublist; + + querytree_sublist = pg_analyze_and_rewrite(parsetree, + prosrc, + proc->proargtypes.values, + proc->pronargs); + querytree_list = list_concat(querytree_list, + querytree_sublist); + } + (void) check_sql_fn_retval(funcoid, proc->prorettype, querytree_list, NULL, NULL); } - else - querytree_list = pg_parse_query(prosrc); error_context_stack = sqlerrcontext.previous; } diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 4b32c5dc5c..3af0b09719 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -1216,7 +1216,8 @@ BeginCopy(bool is_from, * Use a snapshot with an updated command ID to ensure this query sees * results of any previously executed queries. */ - PushUpdatedSnapshot(GetActiveSnapshot()); + PushCopiedSnapshot(GetActiveSnapshot()); + UpdateActiveSnapshotCommandId(); /* Create dest receiver for COPY OUT */ dest = CreateDestReceiver(DestCopyOut); diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 23819a0fdb..cc7acb3320 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -366,7 +366,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es, * Use a snapshot with an updated command ID to ensure this query sees * results of any previously executed queries. */ - PushUpdatedSnapshot(GetActiveSnapshot()); + PushCopiedSnapshot(GetActiveSnapshot()); + UpdateActiveSnapshotCommandId(); /* Create a QueryDesc requesting no output */ queryDesc = CreateQueryDesc(plannedstmt, queryString, diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index d5385438e3..a2f4fac74c 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -47,6 +47,10 @@ typedef struct * We have an execution_state record for each query in a function. Each * record contains a plantree for its query. If the query is currently in * F_EXEC_RUN state then there's a QueryDesc too. + * + * The "next" fields chain together all the execution_state records generated + * from a single original parsetree. (There will only be more than one in + * case of rule expansion of the original parsetree.) */ typedef enum { @@ -93,15 +97,20 @@ typedef struct JunkFilter *junkFilter; /* will be NULL if function returns VOID */ - /* head of linked list of execution_state records */ - execution_state *func_state; + /* + * func_state is a List of execution_state records, each of which is the + * first for its original parsetree, with any additional records chained + * to it via the "next" fields. This sublist structure is needed to keep + * track of where the original query boundaries are. + */ + List *func_state; } SQLFunctionCache; typedef SQLFunctionCache *SQLFunctionCachePtr; /* non-export function prototypes */ -static execution_state *init_execution_state(List *queryTree_list, +static List *init_execution_state(List *queryTree_list, SQLFunctionCachePtr fcache, bool lazyEvalOK); static void init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK); @@ -122,62 +131,78 @@ static void sqlfunction_shutdown(DestReceiver *self); static void sqlfunction_destroy(DestReceiver *self); -/* Set up the list of per-query execution_state records for a SQL function */ -static execution_state * +/* + * Set up the per-query execution_state records for a SQL function. + * + * The input is a List of Lists of parsed and rewritten, but not planned, + * querytrees. The sublist structure denotes the original query boundaries. + */ +static List * init_execution_state(List *queryTree_list, SQLFunctionCachePtr fcache, bool lazyEvalOK) { - execution_state *firstes = NULL; - execution_state *preves = NULL; + List *eslist = NIL; execution_state *lasttages = NULL; - ListCell *qtl_item; + ListCell *lc1; - foreach(qtl_item, queryTree_list) + foreach(lc1, queryTree_list) { - Query *queryTree = (Query *) lfirst(qtl_item); - Node *stmt; - execution_state *newes; + List *qtlist = (List *) lfirst(lc1); + execution_state *firstes = NULL; + execution_state *preves = NULL; + ListCell *lc2; - Assert(IsA(queryTree, Query)); + foreach(lc2, qtlist) + { + Query *queryTree = (Query *) lfirst(lc2); + Node *stmt; + execution_state *newes; - if (queryTree->commandType == CMD_UTILITY) - stmt = queryTree->utilityStmt; - else - stmt = (Node *) pg_plan_query(queryTree, 0, NULL); + Assert(IsA(queryTree, Query)); - /* Precheck all commands for validity in a function */ - if (IsA(stmt, TransactionStmt)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - /* translator: %s is a SQL statement name */ - errmsg("%s is not allowed in a SQL function", - CreateCommandTag(stmt)))); + /* Plan the query if needed */ + if (queryTree->commandType == CMD_UTILITY) + stmt = queryTree->utilityStmt; + else + stmt = (Node *) pg_plan_query(queryTree, 0, NULL); - if (fcache->readonly_func && !CommandIsReadOnly(stmt)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - /* translator: %s is a SQL statement name */ - errmsg("%s is not allowed in a non-volatile function", - CreateCommandTag(stmt)))); + /* Precheck all commands for validity in a function */ + if (IsA(stmt, TransactionStmt)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /* translator: %s is a SQL statement name */ + errmsg("%s is not allowed in a SQL function", + CreateCommandTag(stmt)))); - newes = (execution_state *) palloc(sizeof(execution_state)); - if (preves) - preves->next = newes; - else - firstes = newes; + if (fcache->readonly_func && !CommandIsReadOnly(stmt)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /* translator: %s is a SQL statement name */ + errmsg("%s is not allowed in a non-volatile function", + CreateCommandTag(stmt)))); - newes->next = NULL; - newes->status = F_EXEC_START; - newes->setsResult = false; /* might change below */ - newes->lazyEval = false; /* might change below */ - newes->stmt = stmt; - newes->qd = NULL; + /* OK, build the execution_state for this query */ + newes = (execution_state *) palloc(sizeof(execution_state)); + if (preves) + preves->next = newes; + else + firstes = newes; - if (queryTree->canSetTag) - lasttages = newes; + newes->next = NULL; + newes->status = F_EXEC_START; + newes->setsResult = false; /* might change below */ + newes->lazyEval = false; /* might change below */ + newes->stmt = stmt; + newes->qd = NULL; - preves = newes; + if (queryTree->canSetTag) + lasttages = newes; + + preves = newes; + } + + eslist = lappend(eslist, firstes); } /* @@ -211,7 +236,7 @@ init_execution_state(List *queryTree_list, } } - return firstes; + return eslist; } /* Initialize the SQLFunctionCache for a SQL function */ @@ -225,7 +250,10 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK) SQLFunctionCachePtr fcache; Oid *argOidVect; int nargs; + List *raw_parsetree_list; List *queryTree_list; + List *flat_query_list; + ListCell *lc; Datum tmp; bool isNull; @@ -318,9 +346,32 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK) fcache->src = TextDatumGetCString(tmp); /* - * Parse and rewrite the queries in the function text. + * Parse and rewrite the queries in the function text. Use sublists to + * keep track of the original query boundaries. But we also build a + * "flat" list of the rewritten queries to pass to check_sql_fn_retval. + * This is because the last canSetTag query determines the result type + * independently of query boundaries --- and it might not be in the last + * sublist, for example if the last query rewrites to DO INSTEAD NOTHING. + * (It might not be unreasonable to throw an error in such a case, but + * this is the historical behavior and it doesn't seem worth changing.) */ - queryTree_list = pg_parse_and_rewrite(fcache->src, argOidVect, nargs); + raw_parsetree_list = pg_parse_query(fcache->src); + + queryTree_list = NIL; + flat_query_list = NIL; + foreach(lc, raw_parsetree_list) + { + Node *parsetree = (Node *) lfirst(lc); + List *queryTree_sublist; + + queryTree_sublist = pg_analyze_and_rewrite(parsetree, + fcache->src, + argOidVect, + nargs); + queryTree_list = lappend(queryTree_list, queryTree_sublist); + flat_query_list = list_concat(flat_query_list, + list_copy(queryTree_sublist)); + } /* * Check that the function returns the type it claims to. Although in @@ -343,7 +394,7 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK) */ fcache->returnsTuple = check_sql_fn_retval(foid, rettype, - queryTree_list, + flat_query_list, NULL, &fcache->junkFilter); @@ -375,24 +426,12 @@ init_sql_fcache(FmgrInfo *finfo, bool lazyEvalOK) static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache) { - Snapshot snapshot; DestReceiver *dest; Assert(es->qd == NULL); - /* - * In a read-only function, use the surrounding query's snapshot; - * otherwise take a new snapshot for each query. The snapshot should - * include a fresh command ID so that all work to date in this transaction - * is visible. - */ - if (fcache->readonly_func) - snapshot = GetActiveSnapshot(); - else - { - CommandCounterIncrement(); - snapshot = GetTransactionSnapshot(); - } + /* Caller should have ensured a suitable snapshot is active */ + Assert(ActiveSnapshotSet()); /* * If this query produces the function result, send its output to the @@ -416,18 +455,17 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache) if (IsA(es->stmt, PlannedStmt)) es->qd = CreateQueryDesc((PlannedStmt *) es->stmt, fcache->src, - snapshot, InvalidSnapshot, + GetActiveSnapshot(), + InvalidSnapshot, dest, fcache->paramLI, 0); else es->qd = CreateUtilityQueryDesc(es->stmt, fcache->src, - snapshot, + GetActiveSnapshot(), dest, fcache->paramLI); - /* We assume we don't need to set up ActiveSnapshot for ExecutorStart */ - /* Utility commands don't need Executor. */ if (es->qd->utilitystmt == NULL) { @@ -457,9 +495,6 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache) { bool result; - /* Make our snapshot the active one for any called functions */ - PushActiveSnapshot(es->qd->snapshot); - if (es->qd->utilitystmt) { /* ProcessUtility needs the PlannedStmt for DECLARE CURSOR */ @@ -487,8 +522,6 @@ postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache) result = (count == 0L || es->qd->estate->es_processed == 0); } - PopActiveSnapshot(); - return result; } @@ -502,13 +535,8 @@ postquel_end(execution_state *es) /* Utility commands don't need Executor. */ if (es->qd->utilitystmt == NULL) { - /* Make our snapshot the active one for any called functions */ - PushActiveSnapshot(es->qd->snapshot); - ExecutorFinish(es->qd); ExecutorEnd(es->qd); - - PopActiveSnapshot(); } (*es->qd->dest->rDestroy) (es->qd->dest); @@ -619,9 +647,13 @@ fmgr_sql(PG_FUNCTION_ARGS) ErrorContextCallback sqlerrcontext; bool randomAccess; bool lazyEvalOK; + bool is_first; + bool pushed_snapshot; execution_state *es; TupleTableSlot *slot; Datum result; + List *eslist; + ListCell *eslc; /* * Switch to context in which the fcache lives. This ensures that @@ -673,13 +705,33 @@ fmgr_sql(PG_FUNCTION_ARGS) init_sql_fcache(fcinfo->flinfo, lazyEvalOK); fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra; } - es = fcache->func_state; + eslist = fcache->func_state; + + /* + * Find first unfinished query in function, and note whether it's the + * first query. + */ + es = NULL; + is_first = true; + foreach(eslc, eslist) + { + es = (execution_state *) lfirst(eslc); + + while (es && es->status == F_EXEC_DONE) + { + is_first = false; + es = es->next; + } + + if (es) + break; + } /* * Convert params to appropriate format if starting a fresh execution. (If * continuing execution, we can re-use prior params.) */ - if (es && es->status == F_EXEC_START) + if (is_first && es && es->status == F_EXEC_START) postquel_sub_params(fcache, fcinfo); /* @@ -689,22 +741,57 @@ fmgr_sql(PG_FUNCTION_ARGS) if (!fcache->tstore) fcache->tstore = tuplestore_begin_heap(randomAccess, false, work_mem); - /* - * Find first unfinished query in function. - */ - while (es && es->status == F_EXEC_DONE) - es = es->next; - /* * Execute each command in the function one after another until we either * run out of commands or get a result row from a lazily-evaluated SELECT. + * + * Notes about snapshot management: + * + * In a read-only function, we just use the surrounding query's snapshot. + * + * In a non-read-only function, we rely on the fact that we'll never + * suspend execution between queries of the function: the only reason to + * suspend execution before completion is if we are returning a row from + * a lazily-evaluated SELECT. So, when first entering this loop, we'll + * either start a new query (and push a fresh snapshot) or re-establish + * the active snapshot from the existing query descriptor. If we need to + * start a new query in a subsequent execution of the loop, either we need + * a fresh snapshot (and pushed_snapshot is false) or the existing + * snapshot is on the active stack and we can just bump its command ID. */ + pushed_snapshot = false; while (es) { bool completed; if (es->status == F_EXEC_START) + { + /* + * If not read-only, be sure to advance the command counter for + * each command, so that all work to date in this transaction is + * visible. Take a new snapshot if we don't have one yet, + * otherwise just bump the command ID in the existing snapshot. + */ + if (!fcache->readonly_func) + { + CommandCounterIncrement(); + if (!pushed_snapshot) + { + PushActiveSnapshot(GetTransactionSnapshot()); + pushed_snapshot = true; + } + else + UpdateActiveSnapshotCommandId(); + } + postquel_start(es, fcache); + } + else if (!fcache->readonly_func && !pushed_snapshot) + { + /* Re-establish active snapshot when re-entering function */ + PushActiveSnapshot(es->qd->snapshot); + pushed_snapshot = true; + } completed = postquel_getnext(es, fcache); @@ -730,7 +817,31 @@ fmgr_sql(PG_FUNCTION_ARGS) */ if (es->status != F_EXEC_DONE) break; + + /* + * Advance to next execution_state, which might be in the next list. + */ es = es->next; + while (!es) + { + eslc = lnext(eslc); + if (!eslc) + break; /* end of function */ + + es = (execution_state *) lfirst(eslc); + + /* + * Flush the current snapshot so that we will take a new one + * for the new query list. This ensures that new snaps are + * taken at original-query boundaries, matching the behavior + * of interactive execution. + */ + if (pushed_snapshot) + { + PopActiveSnapshot(); + pushed_snapshot = false; + } + } } /* @@ -857,17 +968,24 @@ fmgr_sql(PG_FUNCTION_ARGS) tuplestore_clear(fcache->tstore); } + /* Pop snapshot if we have pushed one */ + if (pushed_snapshot) + PopActiveSnapshot(); + /* * If we've gone through every command in the function, we are done. Reset * the execution states to start over again on next call. */ if (es == NULL) { - es = fcache->func_state; - while (es) + foreach(eslc, fcache->func_state) { - es->status = F_EXEC_START; - es = es->next; + es = (execution_state *) lfirst(eslc); + while (es) + { + es->status = F_EXEC_START; + es = es->next; + } } } @@ -918,18 +1036,25 @@ sql_exec_error_callback(void *arg) { execution_state *es; int query_num; + ListCell *lc; - es = fcache->func_state; + es = NULL; query_num = 1; - while (es) + foreach(lc, fcache->func_state) { - if (es->qd) + es = (execution_state *) lfirst(lc); + while (es) { - errcontext("SQL function \"%s\" statement %d", - fcache->fname, query_num); - break; + if (es->qd) + { + errcontext("SQL function \"%s\" statement %d", + fcache->fname, query_num); + break; + } + es = es->next; } - es = es->next; + if (es) + break; query_num++; } if (es == NULL) @@ -961,16 +1086,31 @@ static void ShutdownSQLFunction(Datum arg) { SQLFunctionCachePtr fcache = (SQLFunctionCachePtr) DatumGetPointer(arg); - execution_state *es = fcache->func_state; + execution_state *es; + ListCell *lc; - while (es != NULL) + foreach(lc, fcache->func_state) { - /* Shut down anything still running */ - if (es->status == F_EXEC_RUN) - postquel_end(es); - /* Reset states to START in case we're called again */ - es->status = F_EXEC_START; - es = es->next; + es = (execution_state *) lfirst(lc); + while (es) + { + /* Shut down anything still running */ + if (es->status == F_EXEC_RUN) + { + /* Re-establish active snapshot for any called functions */ + if (!fcache->readonly_func) + PushActiveSnapshot(es->qd->snapshot); + + postquel_end(es); + + if (!fcache->readonly_func) + PopActiveSnapshot(); + } + + /* Reset states to START in case we're called again */ + es->status = F_EXEC_START; + es = es->next; + } } /* Release tuplestore if we have one */ diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 0865e3ebef..a717a0deea 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -1768,7 +1768,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, Oid my_lastoid = InvalidOid; SPITupleTable *my_tuptable = NULL; int res = 0; - bool have_active_snap = ActiveSnapshotSet(); + bool pushed_active_snap = false; ErrorContextCallback spierrcontext; CachedPlan *cplan = NULL; ListCell *lc1; @@ -1781,6 +1781,40 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, spierrcontext.previous = error_context_stack; error_context_stack = &spierrcontext; + /* + * We support four distinct snapshot management behaviors: + * + * snapshot != InvalidSnapshot, read_only = true: use exactly the given + * snapshot. + * + * snapshot != InvalidSnapshot, read_only = false: use the given + * snapshot, modified by advancing its command ID before each querytree. + * + * snapshot == InvalidSnapshot, read_only = true: use the entry-time + * ActiveSnapshot, if any (if there isn't one, we run with no snapshot). + * + * snapshot == InvalidSnapshot, read_only = false: take a full new + * snapshot for each user command, and advance its command ID before each + * querytree within the command. + * + * In the first two cases, we can just push the snap onto the stack + * once for the whole plan list. + */ + if (snapshot != InvalidSnapshot) + { + if (read_only) + { + PushActiveSnapshot(snapshot); + pushed_active_snap = true; + } + else + { + /* Make sure we have a private copy of the snapshot to modify */ + PushCopiedSnapshot(snapshot); + pushed_active_snap = true; + } + } + foreach(lc1, plan->plancache_list) { CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc1); @@ -1802,12 +1836,23 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, stmt_list = plansource->plan->stmt_list; } + /* + * In the default non-read-only case, get a new snapshot, replacing + * any that we pushed in a previous cycle. + */ + if (snapshot == InvalidSnapshot && !read_only) + { + if (pushed_active_snap) + PopActiveSnapshot(); + PushActiveSnapshot(GetTransactionSnapshot()); + pushed_active_snap = true; + } + foreach(lc2, stmt_list) { Node *stmt = (Node *) lfirst(lc2); bool canSetTag; DestReceiver *dest; - bool pushed_active_snap = false; _SPI_current->processed = 0; _SPI_current->lastoid = InvalidOid; @@ -1848,48 +1893,16 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, /* * If not read-only mode, advance the command counter before each - * command. + * command and update the snapshot. */ if (!read_only) + { CommandCounterIncrement(); + UpdateActiveSnapshotCommandId(); + } dest = CreateDestReceiver(canSetTag ? DestSPI : DestNone); - if (snapshot == InvalidSnapshot) - { - /* - * Default read_only behavior is to use the entry-time - * ActiveSnapshot, if any; if read-write, grab a full new - * snap. - */ - if (read_only) - { - if (have_active_snap) - { - PushActiveSnapshot(GetActiveSnapshot()); - pushed_active_snap = true; - } - } - else - { - PushActiveSnapshot(GetTransactionSnapshot()); - pushed_active_snap = true; - } - } - else - { - /* - * We interpret read_only with a specified snapshot to be - * exactly that snapshot, but read-write means use the snap - * with advancing of command ID. - */ - if (read_only) - PushActiveSnapshot(snapshot); - else - PushUpdatedSnapshot(snapshot); - pushed_active_snap = true; - } - if (IsA(stmt, PlannedStmt) && ((PlannedStmt *) stmt)->utilityStmt == NULL) { @@ -1925,9 +1938,6 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, res = SPI_OK_UTILITY; } - if (pushed_active_snap) - PopActiveSnapshot(); - /* * The last canSetTag query sets the status values returned to the * caller. Be careful to free any tuptables not returned, to @@ -1970,6 +1980,10 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, fail: + /* Pop the snapshot off the stack if we pushed one */ + if (pushed_active_snap) + PopActiveSnapshot(); + /* We no longer need the cached plan refcount, if any */ if (cplan) ReleaseCachedPlan(cplan, true); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 98b56d6582..39b7b5b678 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -513,47 +513,6 @@ client_read_ended(void) } -/* - * Parse a query string and pass it through the rewriter. - * - * A list of Query nodes is returned, since the string might contain - * multiple queries and/or the rewriter might expand one query to several. - * - * NOTE: this routine is no longer used for processing interactive queries, - * but it is still needed for parsing of SQL function bodies. - */ -List * -pg_parse_and_rewrite(const char *query_string, /* string to execute */ - Oid *paramTypes, /* parameter types */ - int numParams) /* number of parameters */ -{ - List *raw_parsetree_list; - List *querytree_list; - ListCell *list_item; - - /* - * (1) parse the request string into a list of raw parse trees. - */ - raw_parsetree_list = pg_parse_query(query_string); - - /* - * (2) Do parse analysis and rule rewrite. - */ - querytree_list = NIL; - foreach(list_item, raw_parsetree_list) - { - Node *parsetree = (Node *) lfirst(list_item); - - querytree_list = list_concat(querytree_list, - pg_analyze_and_rewrite(parsetree, - query_string, - paramTypes, - numParams)); - } - - return querytree_list; -} - /* * Do raw parsing (only). * diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index a338006a3a..edde5642ee 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -169,11 +169,6 @@ ProcessQuery(PlannedStmt *plan, elog(DEBUG3, "ProcessQuery"); - /* - * Must always set a snapshot for plannable queries. - */ - PushActiveSnapshot(GetTransactionSnapshot()); - /* * Create the QueryDesc object */ @@ -233,8 +228,6 @@ ProcessQuery(PlannedStmt *plan, ExecutorEnd(queryDesc); FreeQueryDesc(queryDesc); - - PopActiveSnapshot(); } /* @@ -1164,8 +1157,8 @@ PortalRunUtility(Portal portal, Node *utilityStmt, bool isTopLevel, * seems to be to enumerate those that do not need one; this is a short * list. Transaction control, LOCK, and SET must *not* set a snapshot * since they need to be executable at the start of a transaction-snapshot - * mode transaction without freezing a snapshot. By extension we allow SHOW - * not to set a snapshot. The other stmts listed are just efficiency + * mode transaction without freezing a snapshot. By extension we allow + * SHOW not to set a snapshot. The other stmts listed are just efficiency * hacks. Beware of listing anything that can modify the database --- if, * say, it has to update an index with expressions that invoke * user-defined functions, then it had better have a snapshot. @@ -1219,6 +1212,7 @@ PortalRunMulti(Portal portal, bool isTopLevel, DestReceiver *dest, DestReceiver *altdest, char *completionTag) { + bool active_snapshot_set = false; ListCell *stmtlist_item; /* @@ -1262,6 +1256,20 @@ PortalRunMulti(Portal portal, bool isTopLevel, if (log_executor_stats) ResetUsage(); + /* + * Must always have a snapshot for plannable queries. First time + * through, take a new snapshot; for subsequent queries in the + * same portal, just update the snapshot's copy of the command + * counter. + */ + if (!active_snapshot_set) + { + PushActiveSnapshot(GetTransactionSnapshot()); + active_snapshot_set = true; + } + else + UpdateActiveSnapshotCommandId(); + if (pstmt->canSetTag) { /* statement can set tag string */ @@ -1291,11 +1299,29 @@ PortalRunMulti(Portal portal, bool isTopLevel, * * These are assumed canSetTag if they're the only stmt in the * portal. + * + * We must not set a snapshot here for utility commands (if one is + * needed, PortalRunUtility will do it). If a utility command is + * alone in a portal then everything's fine. The only case where + * a utility command can be part of a longer list is that rules + * are allowed to include NotifyStmt. NotifyStmt doesn't care + * whether it has a snapshot or not, so we just leave the current + * snapshot alone if we have one. */ if (list_length(portal->stmts) == 1) - PortalRunUtility(portal, stmt, isTopLevel, dest, completionTag); + { + Assert(!active_snapshot_set); + /* statement can set tag string */ + PortalRunUtility(portal, stmt, isTopLevel, + dest, completionTag); + } else - PortalRunUtility(portal, stmt, isTopLevel, altdest, NULL); + { + Assert(IsA(stmt, NotifyStmt)); + /* stmt added by rewrite cannot set tag */ + PortalRunUtility(portal, stmt, isTopLevel, + altdest, NULL); + } } /* @@ -1313,6 +1339,10 @@ PortalRunMulti(Portal portal, bool isTopLevel, MemoryContextDeleteChildren(PortalGetHeapMemory(portal)); } + /* Pop the snapshot if we pushed one. */ + if (active_snapshot_set) + PopActiveSnapshot(); + /* * If a command completion tag was supplied, use it. Otherwise use the * portal's commandTag as the default completion tag. diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index c2ff5e542b..9100c818f8 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -299,22 +299,33 @@ PushActiveSnapshot(Snapshot snap) } /* - * PushUpdatedSnapshot - * As above, except we set the snapshot's CID to the current CID. + * PushCopiedSnapshot + * As above, except forcibly copy the presented snapshot. + * + * This should be used when the ActiveSnapshot has to be modifiable, for + * example if the caller intends to call UpdateActiveSnapshotCommandId. + * The new snapshot will be released when popped from the stack. */ void -PushUpdatedSnapshot(Snapshot snapshot) +PushCopiedSnapshot(Snapshot snapshot) { - Snapshot newsnap; + PushActiveSnapshot(CopySnapshot(snapshot)); +} - /* - * We cannot risk modifying a snapshot that's possibly already used - * elsewhere, so make a new copy to scribble on. - */ - newsnap = CopySnapshot(snapshot); - newsnap->curcid = GetCurrentCommandId(false); +/* + * UpdateActiveSnapshotCommandId + * + * Update the current CID of the active snapshot. This can only be applied + * to a snapshot that is not referenced elsewhere. + */ +void +UpdateActiveSnapshotCommandId(void) +{ + Assert(ActiveSnapshot != NULL); + Assert(ActiveSnapshot->as_snap->active_count == 1); + Assert(ActiveSnapshot->as_snap->regd_count == 0); - PushActiveSnapshot(newsnap); + ActiveSnapshot->as_snap->curcid = GetCurrentCommandId(false); } /* diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index 3d26144d8a..bf21cd9d06 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -45,8 +45,6 @@ typedef enum extern int log_statement; -extern List *pg_parse_and_rewrite(const char *query_string, - Oid *paramTypes, int numParams); extern List *pg_parse_query(const char *query_string); extern List *pg_analyze_and_rewrite(Node *parsetree, const char *query_string, Oid *paramTypes, int numParams); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 6d784d22c5..a7e7d3dc2b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -28,7 +28,8 @@ extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); extern void PushActiveSnapshot(Snapshot snapshot); -extern void PushUpdatedSnapshot(Snapshot snapshot); +extern void PushCopiedSnapshot(Snapshot snapshot); +extern void UpdateActiveSnapshotCommandId(void); extern void PopActiveSnapshot(void); extern Snapshot GetActiveSnapshot(void); extern bool ActiveSnapshotSet(void);