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've attached 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

On Mon, Oct 12, 2020 at 5:01 PM Bryn Llewellyn <bryn@yugabyte.com> wrote:

*WRONG RESULTS FROM FUNCTION THAT SELECTS FROM VIER AFTER "CREATED OR
REPLACE"*

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

Haven't experimented myself but the function declarations are themselves
wrong - they are not immutable.

Please use the information here to file it.

What's the number of, and URL to, the newly-filed bug?

You've just filed a bug report. Since you did so directly instead of using
the form it doesn't get assigned a bug number. All email sent to this
list, or bugs filed using the form, get publicly posted to the mailing list
and are archived as described on the mailing list related web pages. This
is the permanent link for this thread in the archive:

/messages/by-id/4DE6C9D8-77A5-4858-82CB-18728F59D715@yugabyte.com

David J.

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've attached 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 arguments forever.

How do you regard the fact that, in my “f2()”, the result returned by the dynamically executed “select” _does_ change when view “v” suffers create of replace. And, maybe more pertinently, the result from “immutable” function “f1()” changes immediately 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 dependency parents is recompiled. Assuming I’m not simply missing it, should this caveat be added? There isn’t a single hit for 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”. And this is the caveated meaning that Oracle database implements for its moral equivalent “deterministic”. Notice that this is the meaning that YugabyteDB implements, too, so clearly the problem of invalidation is soluble when that goal is explicitly specified.

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

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

Why are my functions not immutable? They merely select, and have no side-effects. This meets the criteria documented here:

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

I hadn’t realized that submitting a bug as the doc at www.postgresql.org/docs/12/bug-reporting.html seems to suggest is the 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). So thanks for that tip off!

david.g.johnston@gmail.com wrote:

On Mon, Oct 12, 2020 at 5:01 PM Bryn Llewellyn <bryn@yugabyte.com <mailto:bryn@yugabyte.com>> wrote:
WRONG RESULTS FROM FUNCTION THAT SELECTS FROM VIER AFTER "CREATED OR REPLACE"

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

Haven't experimented myself but the function declarations are themselves wrong - they are not immutable.

Please use the information here to file it.
What's the number of, and URL to, the newly-filed bug?

You've just filed a bug report. Since you did so directly instead of using the form it doesn't get assigned a bug number. All email sent to this list, or bugs filed using the form, get publicly posted to the mailing list and are archived as described on the mailing list related web pages. This is the permanent link for this thread in the archive:

/messages/by-id/4DE6C9D8-77A5-4858-82CB-18728F59D715@yugabyte.com </messages/by-id/4DE6C9D8-77A5-4858-82CB-18728F59D715@yugabyte.com&gt;

David J.

On Mon, Oct 12, 2020 at 6:15 PM Bryn Llewellyn <bryn@yugabyte.com> wrote:

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 arguments forever.

This is the guarantee that you, the function's author, makes, not the
database. The database is free to cache or not cache results as it
desires. It is not obligated to not re-evaluate an immutable function and
re-use a previous evaluation's result.

I had read “forever” to mean “forever until the function, or any of its

dependency parents, is semantically changed”.

On the create function page that sentence you quote is followed-on by:

"that is, it does not do database lookups or otherwise use information not
directly present in its argument list."

I suppose having that in the other page would have saved a bit of confusion.

And this is the caveated meaning that Oracle database implements for its

moral equivalent “deterministic”. Notice that this is the meaning that
YugabyteDB implements, too, so clearly the problem of invalidation is
soluble when that goal is explicitly specified.

PostgreSQL has defined its meaning of immutable. Your example demonstrates
what can happen if you promise your function is immutable and is it not.

The PG doc goes on to say “For best optimization results, you should label

your functions with the strictest volatility category that is valid for
them.”

So your reply implies that “immutable” must *never* be used in ordinary
application code that might be patched unless the shop commits to doing
every single patch, in a production system, only after disconnecting all
regular client sessions so that, on re-connecting when patching is
complete, everything will be re-compiled ab initio. Where is this rule
documented?

This seems more philosophical than practical. Yes, truly, and even
practically, immutable functions are rare but they are possible. Stable is
usually what ends up being promised. Your toy example has no purpose being
immutable anyway.

select add_two(d1 int, d2 int) returns select d1+d2;

Philosophically speaking the operator plus could change, but it won't and
so the function is practically immutable. The same goes for the
concatenation operator. Or even the meaning of symbol 2 if the inner query
was select 2+2.

As your quoted page says, immutable functions can rely on other immutable
functions, which is why d1+d2 works.

David J.

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 same input 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, because once 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 prepared statements 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 arguments forever.

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

On Mon, Oct 12, 2020 at 6:26 PM Bryn Llewellyn <bryn@yugabyte.com> wrote:

I hadn’t realized that submitting a bug as the doc at
www.postgresql.org/docs/12/bug-reporting.html seems to suggest is the
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). So thanks for that
tip off!

Your concept and definition of "bug report" seem to be slightly different
than what we use here. We're pretty loose but generally try to create link
chains from the -bugs mailing list emails to the patches submitted to
-hackers and the published commits.

David J.

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 is the 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). So thanks 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 is the 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). So thanks 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 about my 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 text specifies 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 on this?

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, actually tells me nothing? In other words, should I understand that the _definition_ of “immutable” would allow some future PG implementation justifiably still to return “42” after recompiling “my_constant()”. 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()”.

On Mon, Oct 12, 2020 at 7:57 PM Bryn Llewellyn <bryn@yugabyte.com> wrote:

create function my_constant()
immutable
return 42;

create function my_dependant()
immutable
language plpgsql
v constant int not null := my_constant();
return v;

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

create or replace function my_constant()
immutable
language plpgsql
return 17;

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, actually tells me nothing? In other
words, should I understand that the _definition_ of “immutable” would allow
some future PG implementation justifiably still to return “42” after
recompiling “my_constant()”.

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 terms of running ad-hoc SQL SELECT queries interactively, no. The
function my_dependant will eventually be re-planned and executed and when
that happens the new definition will be seen. In the extreme case plans do
not survive server restarts. Indeed, plans are session-local so at
worse the next time you connect you will see the 17. As seen, you may see
the 17 appear even sooner - within the my_constant function modification
session - but at that point you are observing an implementation detail that
you should not rely upon.

It's when you start doing stuff like: CHECK WHERE (age < my_constant());
and then you insert a bunch of records to that table. Now change the
function to return 17. Dump the table and restore it - every record with
age between 17 and 41 now fails the check constraint even though none of
the data in the table changed. A similar thing happens for functional
indexes - where the inputs and the function result are cached in an on-disk
index for quick future lookup. Both these features require immutable
functions so persisted data that uses those function results continue to
produce the same outcome "forever" (you can at least REINDEX in the later
case).

David J.

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 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 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 stance on this?
...
Should I understand that this outcome, while it might have wrongly seemed to me to support my bad mental model, actually tells me nothing? In other words, should I understand that the _definition_ of “immutable” would allow some future PG 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 hold firm 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 meaning that 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 simply is 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 for any 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. As long as the function survives (and survival includes living on over “create or replace”), the implementation has permission to evaluate the function by accessing the cached value for the actual(s) at hand rather than running the function’s body.

3. Nothing in #2 says that the values are guaranteed to be cached, or that a cached value for some set of actuals will in fact 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 wrong results.

5. if you do empirical tests that attempt to determine how the implementation does the caching, and uses cached values, then you 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 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 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 stance on this?
...
Should I understand that this outcome, while it might have wrongly seemed to me to support my bad mental model, actually tells me nothing? In other words, should I understand that the _definition_ of “immutable” would allow some future PG 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