From: Tom Lane Date: Mon, 26 Nov 2012 20:55:43 +0000 (-0500) Subject: Revert patch for taking fewer snapshots. X-Git-Tag: REL9_3_BETA1~692 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=532994299e2ff208a58376134fab75f5ae471e41;p=postgresql Revert patch for taking fewer snapshots. This reverts commit d573e239f03506920938bf0be56c868d9c3416da, "Take fewer snapshots". While that seemed like a good idea at the time, it caused execution to use a snapshot that had been acquired before locking any of the tables mentioned in the query. This created user-visible anomalies that were not present in any prior release of Postgres, as reported by Tomas Vondra. While this whole area could do with a redesign (since there are related cases that have anomalies anyway), it doesn't seem likely that any future patch would be reasonably back-patchable; and we don't want 9.2 to exhibit a behavior that's subtly unlike either past or future releases. Hence, revert to prior code while we rethink the problem. --- diff --git a/doc/src/sgml/release-9.2.sgml b/doc/src/sgml/release-9.2.sgml index 840e7a7a0d..c641606fd5 100644 --- a/doc/src/sgml/release-9.2.sgml +++ b/doc/src/sgml/release-9.2.sgml @@ -809,13 +809,6 @@ - - - Take fewer MVCC snapshots - (Robert Haas) - - - Make the number of CLOG buffers scale based on strategy == PORTAL_ONE_SELECT); diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index 9f993de6f1..91ef5507e2 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -289,7 +289,7 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause, /* * Run the portal as appropriate. */ - PortalStart(portal, paramLI, eflags, true); + PortalStart(portal, paramLI, eflags, GetActiveSnapshot()); (void) PortalRun(portal, count, false, dest, dest, completionTag); diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index bf8c4c7113..416a2c4f3b 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -1127,6 +1127,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, CachedPlan *cplan; List *stmt_list; char *query_string; + Snapshot snapshot; MemoryContext oldcontext; Portal portal; @@ -1269,6 +1270,15 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, } } + /* Set up the snapshot to use. */ + if (read_only) + snapshot = GetActiveSnapshot(); + else + { + CommandCounterIncrement(); + snapshot = GetTransactionSnapshot(); + } + /* * If the plan has parameters, copy them into the portal. Note that this * must be done after revalidating the plan, because in dynamic parameter @@ -1284,13 +1294,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, /* * Start portal execution. */ - if (read_only) - PortalStart(portal, paramLI, 0, true); - else - { - CommandCounterIncrement(); - PortalStart(portal, paramLI, 0, false); - } + PortalStart(portal, paramLI, 0, snapshot); Assert(portal->strategy != PORTAL_MULTI_QUERY); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 585db1af89..1fd39f2d99 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -976,6 +976,10 @@ exec_simple_query(const char *query_string) plantree_list = pg_plan_queries(querytree_list, 0, NULL); + /* Done with the snapshot used for parsing/planning */ + if (snapshot_set) + PopActiveSnapshot(); + /* If we got a cancel signal in analysis or planning, quit */ CHECK_FOR_INTERRUPTS(); @@ -1000,19 +1004,9 @@ exec_simple_query(const char *query_string) NULL); /* - * Start the portal. - * - * If we took a snapshot for parsing/planning, the portal may be able - * to reuse it for the execution phase. Currently, this will only - * happen in PORTAL_ONE_SELECT mode. But even if PortalStart doesn't - * end up being able to do this, keeping the parse/plan snapshot - * around until after we start the portal doesn't cost much. + * Start the portal. No parameters here. */ - PortalStart(portal, NULL, 0, snapshot_set); - - /* Done with the snapshot used for parsing/planning */ - if (snapshot_set) - PopActiveSnapshot(); + PortalStart(portal, NULL, 0, InvalidSnapshot); /* * Select the appropriate output format: text unless we are doing a @@ -1735,19 +1729,15 @@ exec_bind_message(StringInfo input_message) cplan->stmt_list, cplan); - /* - * And we're ready to start portal execution. - * - * If we took a snapshot for parsing/planning, we'll try to reuse it for - * query execution (currently, reuse will only occur if PORTAL_ONE_SELECT - * mode is chosen). - */ - PortalStart(portal, params, 0, snapshot_set); - /* Done with the snapshot used for parameter I/O and parsing/planning */ if (snapshot_set) PopActiveSnapshot(); + /* + * And we're ready to start portal execution. + */ + PortalStart(portal, params, 0, InvalidSnapshot); + /* * Apply the result format requests to the portal. */ diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 2cb9a8ee2f..8ad0aaa55e 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -447,18 +447,17 @@ FetchStatementTargetList(Node *stmt) * currently only honored for PORTAL_ONE_SELECT portals). Most callers * should simply pass zero. * - * The use_active_snapshot parameter is currently used only for - * PORTAL_ONE_SELECT portals. If it is true, the active snapshot will - * be used when starting up the executor; if false, a new snapshot will - * be taken. This is used both for cursors and to avoid taking an entirely - * new snapshot when it isn't necessary. + * The caller can optionally pass a snapshot to be used; pass InvalidSnapshot + * for the normal behavior of setting a new snapshot. This parameter is + * presently ignored for non-PORTAL_ONE_SELECT portals (it's only intended + * to be used for cursors). * * On return, portal is ready to accept PortalRun() calls, and the result * tupdesc (if any) is known. */ void PortalStart(Portal portal, ParamListInfo params, - int eflags, bool use_active_snapshot) + int eflags, Snapshot snapshot) { Portal saveActivePortal; ResourceOwner saveResourceOwner; @@ -500,8 +499,8 @@ PortalStart(Portal portal, ParamListInfo params, case PORTAL_ONE_SELECT: /* Must set snapshot before starting executor. */ - if (use_active_snapshot) - PushActiveSnapshot(GetActiveSnapshot()); + if (snapshot) + PushActiveSnapshot(snapshot); else PushActiveSnapshot(GetTransactionSnapshot()); diff --git a/src/include/tcop/pquery.h b/src/include/tcop/pquery.h index 22aad2e96c..c77bb8be94 100644 --- a/src/include/tcop/pquery.h +++ b/src/include/tcop/pquery.h @@ -28,7 +28,7 @@ extern List *FetchPortalTargetList(Portal portal); extern List *FetchStatementTargetList(Node *stmt); extern void PortalStart(Portal portal, ParamListInfo params, - int eflags, bool use_active_snapshot); + int eflags, Snapshot snapshot); extern void PortalSetResultFormat(Portal portal, int nFormats, int16 *formats);