Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> So what's happening here is that the function get_bug_id, being stable,
> is being called speculatively at plan time for the query where it
> appears in the WHERE clause. For whatever reason, the snapshot it's
> being run in at that time is not the same one actually used for the
> later execution of the query, and the plan-time snapshot doesn't see the
> just-inserted row.
> It looks like what's going on here is that SPI does GetCachedPlan -
> which is where planning will happen - _before_ establishing the new
> snapshot in the non-read-only case (read_only is false here because the
> calling function, test_bug(), is volatile).
Yeah, I came to the same conclusion. I think it's basically accidental
that the given test case works before 9.2: the reason seems to be that
in 9.1, the plancache doesn't pass through the parameter list containing
the value of "my_text", so that the planner is unable to speculatively
execute get_bug_id(). The order of operations in _SPI_execute_plan
is just as wrong though.
The attached patch makes it better and doesn't seem to break any
regression tests. I'm not sure how nervous I am about unexpected
side-effects ...
regards, tom lane
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 9fc4431..45e62ba 100644
*** a/src/backend/executor/spi.c
--- b/src/backend/executor/spi.c
*************** _SPI_execute_plan(SPIPlanPtr plan, Param
*** 2066,2071 ****
--- 2066,2083 ----
spierrcontext.arg = (void *) plansource->query_string;
/*
+ * 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;
+ }
+
+ /*
* If this is a one-shot plan, we still need to do parse analysis.
*/
if (plan->oneshot)
*************** _SPI_execute_plan(SPIPlanPtr plan, Param
*** 2117,2134 ****
cplan = GetCachedPlan(plansource, paramLI, plan->saved, _SPI_current->queryEnv);
stmt_list = cplan->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)
{
PlannedStmt *stmt = lfirst_node(PlannedStmt, lc2);
--- 2129,2134 ----