Commit 53299429 authored by Tom Lane's avatar Tom Lane

Revert patch for taking fewer snapshots.

This reverts commit d573e239, "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.
parent d3237e04
...@@ -809,13 +809,6 @@ ...@@ -809,13 +809,6 @@
</para> </para>
</listitem> </listitem>
<listitem>
<para>
Take fewer <acronym>MVCC</acronym> snapshots
(Robert Haas)
</para>
</listitem>
<listitem> <listitem>
<para> <para>
Make the number of CLOG buffers scale based on <link Make the number of CLOG buffers scale based on <link
......
...@@ -121,7 +121,7 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params, ...@@ -121,7 +121,7 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
/* /*
* Start execution, inserting parameters if any. * Start execution, inserting parameters if any.
*/ */
PortalStart(portal, params, 0, true); PortalStart(portal, params, 0, GetActiveSnapshot());
Assert(portal->strategy == PORTAL_ONE_SELECT); Assert(portal->strategy == PORTAL_ONE_SELECT);
......
...@@ -289,7 +289,7 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause, ...@@ -289,7 +289,7 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
/* /*
* Run the portal as appropriate. * Run the portal as appropriate.
*/ */
PortalStart(portal, paramLI, eflags, true); PortalStart(portal, paramLI, eflags, GetActiveSnapshot());
(void) PortalRun(portal, count, false, dest, dest, completionTag); (void) PortalRun(portal, count, false, dest, dest, completionTag);
......
...@@ -1127,6 +1127,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, ...@@ -1127,6 +1127,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
CachedPlan *cplan; CachedPlan *cplan;
List *stmt_list; List *stmt_list;
char *query_string; char *query_string;
Snapshot snapshot;
MemoryContext oldcontext; MemoryContext oldcontext;
Portal portal; Portal portal;
...@@ -1269,6 +1270,15 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, ...@@ -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 * If the plan has parameters, copy them into the portal. Note that this
* must be done after revalidating the plan, because in dynamic parameter * must be done after revalidating the plan, because in dynamic parameter
...@@ -1284,13 +1294,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, ...@@ -1284,13 +1294,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
/* /*
* Start portal execution. * Start portal execution.
*/ */
if (read_only) PortalStart(portal, paramLI, 0, snapshot);
PortalStart(portal, paramLI, 0, true);
else
{
CommandCounterIncrement();
PortalStart(portal, paramLI, 0, false);
}
Assert(portal->strategy != PORTAL_MULTI_QUERY); Assert(portal->strategy != PORTAL_MULTI_QUERY);
......
...@@ -976,6 +976,10 @@ exec_simple_query(const char *query_string) ...@@ -976,6 +976,10 @@ exec_simple_query(const char *query_string)
plantree_list = pg_plan_queries(querytree_list, 0, NULL); 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 */ /* If we got a cancel signal in analysis or planning, quit */
CHECK_FOR_INTERRUPTS(); CHECK_FOR_INTERRUPTS();
...@@ -1000,19 +1004,9 @@ exec_simple_query(const char *query_string) ...@@ -1000,19 +1004,9 @@ exec_simple_query(const char *query_string)
NULL); NULL);
/* /*
* Start the portal. * Start the portal. No parameters here.
*
* 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.
*/ */
PortalStart(portal, NULL, 0, snapshot_set); PortalStart(portal, NULL, 0, InvalidSnapshot);
/* Done with the snapshot used for parsing/planning */
if (snapshot_set)
PopActiveSnapshot();
/* /*
* Select the appropriate output format: text unless we are doing a * Select the appropriate output format: text unless we are doing a
...@@ -1735,19 +1729,15 @@ exec_bind_message(StringInfo input_message) ...@@ -1735,19 +1729,15 @@ exec_bind_message(StringInfo input_message)
cplan->stmt_list, cplan->stmt_list,
cplan); 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 */ /* Done with the snapshot used for parameter I/O and parsing/planning */
if (snapshot_set) if (snapshot_set)
PopActiveSnapshot(); PopActiveSnapshot();
/*
* And we're ready to start portal execution.
*/
PortalStart(portal, params, 0, InvalidSnapshot);
/* /*
* Apply the result format requests to the portal. * Apply the result format requests to the portal.
*/ */
......
...@@ -447,18 +447,17 @@ FetchStatementTargetList(Node *stmt) ...@@ -447,18 +447,17 @@ FetchStatementTargetList(Node *stmt)
* currently only honored for PORTAL_ONE_SELECT portals). Most callers * currently only honored for PORTAL_ONE_SELECT portals). Most callers
* should simply pass zero. * should simply pass zero.
* *
* The use_active_snapshot parameter is currently used only for * The caller can optionally pass a snapshot to be used; pass InvalidSnapshot
* PORTAL_ONE_SELECT portals. If it is true, the active snapshot will * for the normal behavior of setting a new snapshot. This parameter is
* be used when starting up the executor; if false, a new snapshot will * presently ignored for non-PORTAL_ONE_SELECT portals (it's only intended
* be taken. This is used both for cursors and to avoid taking an entirely * to be used for cursors).
* new snapshot when it isn't necessary.
* *
* On return, portal is ready to accept PortalRun() calls, and the result * On return, portal is ready to accept PortalRun() calls, and the result
* tupdesc (if any) is known. * tupdesc (if any) is known.
*/ */
void void
PortalStart(Portal portal, ParamListInfo params, PortalStart(Portal portal, ParamListInfo params,
int eflags, bool use_active_snapshot) int eflags, Snapshot snapshot)
{ {
Portal saveActivePortal; Portal saveActivePortal;
ResourceOwner saveResourceOwner; ResourceOwner saveResourceOwner;
...@@ -500,8 +499,8 @@ PortalStart(Portal portal, ParamListInfo params, ...@@ -500,8 +499,8 @@ PortalStart(Portal portal, ParamListInfo params,
case PORTAL_ONE_SELECT: case PORTAL_ONE_SELECT:
/* Must set snapshot before starting executor. */ /* Must set snapshot before starting executor. */
if (use_active_snapshot) if (snapshot)
PushActiveSnapshot(GetActiveSnapshot()); PushActiveSnapshot(snapshot);
else else
PushActiveSnapshot(GetTransactionSnapshot()); PushActiveSnapshot(GetTransactionSnapshot());
......
...@@ -28,7 +28,7 @@ extern List *FetchPortalTargetList(Portal portal); ...@@ -28,7 +28,7 @@ extern List *FetchPortalTargetList(Portal portal);
extern List *FetchStatementTargetList(Node *stmt); extern List *FetchStatementTargetList(Node *stmt);
extern void PortalStart(Portal portal, ParamListInfo params, extern void PortalStart(Portal portal, ParamListInfo params,
int eflags, bool use_active_snapshot); int eflags, Snapshot snapshot);
extern void PortalSetResultFormat(Portal portal, int nFormats, extern void PortalSetResultFormat(Portal portal, int nFormats,
int16 *formats); int16 *formats);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment