Обсуждение: Wrong results from function that selects from vier after "created or replace"

Bryn Llewellyn <bryn@yugabyte.com> writes:
> The attached self-contained testcase, "pg-bad-invalidation-bug.sql", seems to me definitely to demonstrate a bug.
I'veattached the spooled output as "pg-bad-invalidation-bug.txt" Do you agree that this is a bug? 

No.  You marked the functions as "immutable", and then you broke that
promise by changing what they'd need to output.  The planner had
already inlined the original output value of f1() into the calling
expressions in f2(), on the strength of it being allegedly immutable.
There is no mechanism for undoing that (short of a forced cache flush
or session end), nor should there need to be.

If I mark the functions as either stable or volatile, I see the
desired behavior.

            regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bryn Llewellyn <bryn@yugabyte.com> writes:

> The attached self-contained testcase, "pg-bad-invalidation-bug.sql", seems to me definitely to demonstrate a bug.
I'veattached the spooled output as "pg-bad-invalidation-bug.txt" Do you agree that this is a bug? 

No.  You marked the functions as "immutable", and then you broke that
promise by changing what they'd need to output.  The planner had
already inlined the original output value of f1() into the calling
expressions in f2(), on the strength of it being allegedly immutable.
There is no mechanism for undoing that (short of a forced cache flush
or session end), nor should there need to be.

If I mark the functions as either stable or volatile, I see the
desired behavior.

            regards, tom lane


Thank you for the very quick response. This is what the Version 12 doc says on “immutable”:

> An IMMUTABLE function cannot modify the database and is guaranteed to return the same results given the same
argumentsforever. 

How do you regard the fact that, in my “f2()”, the result returned by the dynamically executed “select” _does_ change
whenview “v” suffers create of replace. And, maybe more pertinently, the result from “immutable” function “f1()”
changesimmediately when “v” is redefined. So, also, does this have an immediate effect: 

  create or replace function f2()
    returns text
    immutable
    language plpgsql
  as $body$
  declare
    t1 constant text := f1()||'?';
    t2 constant text := (select f1())||'?';
    t3 text not null := '';
  begin
    execute $$select f1()||'?'$$ into t3;
    return t1||' | '||t2||' | '||t3;
  end;
  $body$;

I couldn’t find an “all bets are off” caveat for the case where either an “immutable” function itself or any of its
dependencyparents is recompiled. Assuming I’m not simply missing it, should this caveat be added? There isn’t a single
hitfor any inflexion of “compile” on the page that defines “immutable”: 

https://www.postgresql.org/docs/12/xfunc-volatility.html

I had read “forever” to mean “forever until the function, or any of its dependency parents, is semantically changed”.
Andthis is the caveated meaning that Oracle database implements for its moral equivalent “deterministic”. Notice that
thisis the meaning that YugabyteDB implements, too, so clearly the problem of invalidation is soluble when that goal is
explicitlyspecified. 

The PG doc goes on to say “For best optimization results, you should label your functions with the strictest volatility
categorythat is valid for them.” So your reply implies that “immutable” must *never* be used in ordinary application
codethat might be patched unless the shop commits to doing every single patch, in a production system, only after
disconnectingall regular client sessions so that, on re-connecting when patching is complete, everything will be
re-compiledab initio. Where is this rule documented? 

I do appreciate that “drop... cascade” will meet the correctness requirement by brute force. But that technique, too,
meansmaking the application unavailable for the duration of the patching exercise. 

> On Oct 12, 2020, at 18:26, Bryn Llewellyn <bryn@yugabyte.com> wrote:
>
> Why are my functions not immutable? They merely select, and have no side-effects.

For a function to be immutable, it needs to not depend on the state of the database, and return the same value for the
sameinput parameters.  A SELECT that accesses a view definition depends on the state of the database, because (as you
discovered)that view could change under the function.  This is particularly important inside of PL/pgSQL functions,
becauseonce planned the plans for those functions are cached, and thus the plan could be cached and reused incorrectly.
The documentation states this: 

> Labeling a function IMMUTABLE when it really isn't might allow it to be prematurely folded to a constant during
planning,resulting in a stale value being re-used during subsequent uses of the plan. This is a hazard when using
preparedstatements or when using function languages that cache plans (such as PL/pgSQL). 

It's not a bug, but if the documentation could be improved, suggestions are certainly welcome.

--
-- Christophe Pettus
   xof@thebuild.com

Bryn Llewellyn <bryn@yugabyte.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> No.  You marked the functions as "immutable", and then you broke that
>> promise by changing what they'd need to output.

> Thank you for the very quick response. This is what the Version 12 doc says on “immutable”:

>> An IMMUTABLE function cannot modify the database and is guaranteed to return the same results given the same
argumentsforever. 

Indeed.  f1() has no arguments, therefore an immutable marking is a
promise --- made by you to the database, NOT vice versa --- that its
output will never change at all.  (f2 likewise, although in this
particular example that's not what matters.)  Optimizations made on
the strength of such a promise are not bugs.

As a general rule, immutable functions that inspect the database's
contents are probably wrongly marked.  We don't forbid such coding,
because there are narrow use-cases for it, but I tend to believe
that it's a red flag for misdesign.

> I couldn’t find an “all bets are off” caveat for the case where
> either an “immutable” function itself or any of its dependency
> parents is recompiled.

We do not attempt to document what might go wrong when you lie
about the volatility classification of a function.  There are
a lot of possibilities, many of them worse than what you have
here, and there's no reason to think that the implications will
be stable enough to be worth documenting.

> I had read “forever” to mean “forever until the function, or any of
> its dependency parents, is semantically changed”.

That's a curious reading of "forever".

> And this is the caveated meaning that Oracle database implements for
> its moral equivalent “deterministic”.

Nowhere do we claim to do exactly what Oracle does, especially when
it's not even the same syntax.  "immutable" is NOT the same thing
as "deterministic".

            regards, tom lane

Bryn Llewellyn <bryn@yugabyte.com> writes:
> I hadn’t realized that submitting a bug as the doc at www.postgresql.org/docs/12/bug-reporting.html seems to suggest
isthe recommended method (“In general, send bug reports to the bug report mailing list at
<pgsql-bugs@lists.postgresql.org>…”)would _never_ result in a bug being filed (even if it’s acknowledged to be a bug).
Sothanks for that tip off! 

As far as that goes, the closest thing we have to an "official bug report"
is an email to the pgsql-bugs mailing list --- which is what you've done.
The only thing that webform does differently is attach a subject line with
a serial number; but the subject line has little bearing on how we treat
the report.

As David indicated, what we usually use as a formal reference is the
email's message ID, which can be used conveniently to index into the
Postgres project's email archives.  We do that in part because many
bug reports arrive via other email lists.

In this case, multiple people have opined that it isn't a bug, but you'd
have gotten the exact same responses if you'd gone via the webform.

            regards, tom lane

On 12-Oct-2020, at 18:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bryn Llewellyn <bryn@yugabyte.com> writes:
> I hadn’t realized that submitting a bug as the doc at www.postgresql.org/docs/12/bug-reporting.html seems to suggest
isthe recommended method (“In general, send bug reports to the bug report mailing list at
<pgsql-bugs@lists.postgresql.org>…”)would _never_ result in a bug being filed (even if it’s acknowledged to be a bug).
Sothanks for that tip off! 

As far as that goes, the closest thing we have to an "official bug report"
is an email to the pgsql-bugs mailing list --- which is what you've done.
The only thing that webform does differently is attach a subject line with
a serial number; but the subject line has little bearing on how we treat
the report.

As David indicated, what we usually use as a formal reference is the
email's message ID, which can be used conveniently to index into the
Postgres project's email archives.  We do that in part because many
bug reports arrive via other email lists.

In this case, multiple people have opined that it isn't a bug, but you'd
have gotten the exact same responses if you'd gone via the webform.

            regards, tom lane

Thank you very much for this clarification. I’ll know what to do next time. I’ll also be sure to think very much harder
aboutmy analysis before sticking my neck out! 

Tom Lane <tgl@sss.pgh.pa.us> wrote:

…We do not attempt to document what might go wrong when you lie
about the volatility classification of a function.  There are
a lot of possibilities, many of them worse than what you have
here, and there's no reason to think that the implications will
be stable enough to be worth documenting.

> I had read “forever” to mean “forever until the function, or any of
> its dependency parents, is semantically changed”.

That's a curious reading of "forever”.

            regards, tom lane

Thanks again, Tom. And thanks, too, to Christophe Pettus and to David Johnston.

I see the now that my analysis was faulty. And that, moreover, I described Oracle Database’s meaning of “deterministic”
wrongly.Yes indeed, both Postgres’s “immutable” and Oracle’s “deterministic” rule out sensitivity to database state.
And“select” from table or a view must be defined to be reading database state—even when I happen to know that the view
textspecifies a result that does not depend on database state. 

I’m going you risk overtaxing your patience with a follow-up question to test what you’ve just said. What’s your stance
onthis? 

drop function if exists my_dependant() cascade;
drop function if exists my_constant() cascade;

create function my_constant()
  returns int
  immutable
  language plpgsql
as $body$
begin
  return 42;
end;
$body$;

create function my_dependant()
  returns int
  immutable
  language plpgsql
as $body$
declare
  v constant int not null := my_constant();
begin
  return v;
end;
$body$;

select 'my_dependant(): '||my_dependant()::text;

create or replace function my_constant()
  returns int
  immutable
  language plpgsql
as $body$
begin
  return 17;
end;
$body$;

select 'my_dependant(): '||my_dependant()::text;

When I run this, I see “42” at the start and “17” after recompiling the function “my_constant()”.

Should I understand that this outcome, while it might have wrongly seemed to me to support my bad mental model,
actuallytells me nothing? In other words, should I understand that the _definition_ of “immutable” would allow some
futurePG implementation justifiably still to return “42” after recompiling “my_constant()”. And should I understand
thatI might to continue to see “42” quite literally forever (even after pg-stop and pg-start) —until I actually drop
thefunction “my_constant()”. 

Bryn Llewellyn <bryn@yugabyte.com> writes:
> I see the now that my analysis was faulty. And that, moreover, I described Oracle Database’s meaning of
“deterministic”wrongly. Yes indeed, both Postgres’s “immutable” and Oracle’s “deterministic” rule out sensitivity to
databasestate. And “select” from table or a view must be defined to be reading database state—even when I happen to
knowthat the view text specifies a result that does not depend on database state. 

FWIW, when I said "database state" I meant to include the contents of the
system catalogs, not only user-defined tables.  So redefining the view v
as you did counts as a database state change.

> I’m going you risk overtaxing your patience with a follow-up question to test what you’ve just said. What’s your
stanceon this? 
> ...
> Should I understand that this outcome, while it might have wrongly seemed to me to support my bad mental model,
actuallytells me nothing? In other words, should I understand that the _definition_ of “immutable” would allow some
futurePG implementation justifiably still to return “42” after recompiling “my_constant()”. 

Yeah.  Whether you get "42" or "17" in this example is an implementation
artifact that could vary (and has varied, I think) across PG versions,
depending on the exact plan caching behavior being used.  It's even
possible that the result would change in an asynchronous way within a
single session, since hard-to-predict cache flush events could cause the
plan in the calling query to be rebuilt.

> And should I understand that I might to continue to see “42” quite literally forever (even after pg-stop and
pg-start)—until I actually drop the function “my_constant()”. 

In our current implementation, you would not see the effects of the old
function contents persisting into new backend processes, *in an example of
this sort*.  The reason why we insist that "immutable" means "no changes
for the foreseeable future" is that immutable functions are allowed in
index definitions, and there is no mechanism for rebuilding an index
after a behavioral change of a function that it depends on.  As a simple
example, if you define f(x) = x+1 and then do

create index on mytab (f(mycol));

then a query such as "select * from mytab where f(mycol) = 42" will
return rows where mycol=41.  If you then change f() so it returns x+2,
that query will still return rows where mycol=41, because the
corresponding index entries still contain 42.  You'd need to manually
reindex in order to bring the index into sync with the new function
definition.  From our standpoint, relaxing the definition of immutable
would entail that the database undertakes to make that sort of thing
happen transparently.  While it's probably not impossible, it's not
something we care to invest the required amount of effort in.

            regards, tom lane

Thanks, again, Tom. I believe that I now have the right understanding of the semantics of “immutable”. And I’m going to
holdfirm that the semantics definition cannot refer to aspects of the current, or possible future implementation. 

1. The word is to be taken in the mathematical sense of “deterministic”. (This, of course, has nothing to do with any
meaningthat Oracle Database might give to it.) For example, from Wikipedia: 

> a deterministic algorithm is an algorithm which, given a particular input, will always produce the same output…
Formally,a deterministic algorithm computes a mathematical function; a function has a unique value for any input in its
domain,and the algorithm is a process that produces this particular value as output. 

So this informs the meaning of “forever”. Another way to see it is that time cannot be in the picture. The algorithm
simplyis what it is, in some platonic sense of the term. Like sin(x). 

2. When a Postgres function is marked “immutable”, the implementation has permission to cache the output value produced
forany set of actuals—and to maintain the values in that cache literally for ever (never mind what is practically
feasible).However, the cache must be attached to an existing function—and so it vanishes when the function is dropped.
Aslong as the function survives (and survival includes living on over “create or replace”), the implementation has
permissionto evaluate the function by accessing the cached value for the actual(s) at hand rather than running the
function’sbody. 

3. Nothing in #2 says that the values are guaranteed to be cached, or that a cached value for some set of actuals will
infact be accessed when those values are presented again. 

4. Unless you promise that a function that you mark “immutable” meets the criteria set out in #1, you’ll risk getting
wrongresults. 

5. if you do empirical tests that attempt to determine how the implementation does the caching, and uses cached values,
thenyou will learn nothing—and you will risk drawing groundless conclusions. (Just like I did.) 

On 12-Oct-2020, at 20:28, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bryn Llewellyn <bryn@yugabyte.com> writes:
> I see the now that my analysis was faulty. And that, moreover, I described Oracle Database’s meaning of
“deterministic”wrongly. Yes indeed, both Postgres’s “immutable” and Oracle’s “deterministic” rule out sensitivity to
databasestate. And “select” from table or a view must be defined to be reading database state—even when I happen to
knowthat the view text specifies a result that does not depend on database state. 

FWIW, when I said "database state" I meant to include the contents of the
system catalogs, not only user-defined tables.  So redefining the view v
as you did counts as a database state change.

> I’m going you risk overtaxing your patience with a follow-up question to test what you’ve just said. What’s your
stanceon this? 
> ...
> Should I understand that this outcome, while it might have wrongly seemed to me to support my bad mental model,
actuallytells me nothing? In other words, should I understand that the _definition_ of “immutable” would allow some
futurePG implementation justifiably still to return “42” after recompiling “my_constant()”. 

Yeah.  Whether you get "42" or "17" in this example is an implementation
artifact that could vary (and has varied, I think) across PG versions,
depending on the exact plan caching behavior being used.  It's even
possible that the result would change in an asynchronous way within a
single session, since hard-to-predict cache flush events could cause the
plan in the calling query to be rebuilt.

> And should I understand that I might to continue to see “42” quite literally forever (even after pg-stop and
pg-start)—until I actually drop the function “my_constant()”. 

In our current implementation, you would not see the effects of the old
function contents persisting into new backend processes, *in an example of
this sort*.  The reason why we insist that "immutable" means "no changes
for the foreseeable future" is that immutable functions are allowed in
index definitions, and there is no mechanism for rebuilding an index
after a behavioral change of a function that it depends on.  As a simple
example, if you define f(x) = x+1 and then do

create index on mytab (f(mycol));

then a query such as "select * from mytab where f(mycol) = 42" will
return rows where mycol=41.  If you then change f() so it returns x+2,
that query will still return rows where mycol=41, because the
corresponding index entries still contain 42.  You'd need to manually
reindex in order to bring the index into sync with the new function
definition.  From our standpoint, relaxing the definition of immutable
would entail that the database undertakes to make that sort of thing
happen transparently.  While it's probably not impossible, it's not
something we care to invest the required amount of effort in.

            regards, tom lane