Re: Re: Bug with STABLE function using the wrong snapshot (probably during planning)

From: Matthijs Bomhoff <matthijs(at)quarantainenet(dot)nl>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Noah Misch <noah(at)leadboat(dot)com>, pgsql-bugs(at)postgresql(dot)org
Subject: Re: Re: Bug with STABLE function using the wrong snapshot (probably during planning)
Date: 2011-05-16 09:10:23
Message-ID: AAE5B068-2945-4D51-84BC-4248B526B657@quarantainenet.nl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs


On May 13, 2011, at 12:46 AM, Tom Lane wrote:

> Noah Misch <noah(at)leadboat(dot)com> writes:
>>> Some initial debugging by RhodiumToad on #postgresql led to the following observation: The error occurs only when the "SELECT ... WHERE i = a_bar();" is being planned, not when it is being executed, with the snapshot being used to plan the query apparently being too old to see the result of the preceding insert.
>
>> The simplest fix I can see is to have _SPI_prepare_plan() push/pop a new
>> snapshot when analyze_requires_snapshot() returns true on the raw parse tree.
>> That strategy can break down in the other direction if the caller is STABLE;
>> consider this example:
>
> Yes. I'm of the opinion that we should not change this. In general,
> marking functions STABLE that have major side effects (such as throwing
> errors) is not a good idea, and putting such things into WHERE clauses
> is a worse one. We explicitly do not guarantee anything about timing or
> order of evaluation in WHERE clauses, because to do so would cripple the
> planner's ability to optimize them. So I think this is a "don't do
> that" case rather than "we should try to make planning happen with the
> same snapshot that will be used at execution" case.

First of all, thanks to both of you for your replies to my email; at least now I understand a bit more of what went wrong.

As I know almost nothing about the internals, I am not one to argue about whether or not to change the current behavior. It would however perhaps be nice to have the documentation of the volatility categories mention explicitly that throwing an error is also considered a (major) side-effect. In particular: apparently not only are modifications to the database itself considered side-effects, but so is "irregular" control flow within a procedural language... Based on the current documentation, I thought that as my function made no changes to the database and returns the same results given the same arguments for all rows within a single statement, it could safely be marked as STABLE.

Regards,

Matthijs

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Pascal Borschneck 2011-05-16 12:04:59 Re: BUG #5984: Got FailedAssertion("!(opaque->btpo_prev == target)", File: "nbtpage.c", Line: 1166)
Previous Message Tom Lane 2011-05-15 21:02:58 Re: BUG #6021: There is no difference between default and empty access privileges with \dp