Thread: BUG #18392: Can't restore database (using pg_restore) after latest Feb 8 update

The following bug has been logged on the website:

Bug reference:      18392
Logged by:          Carl Smith
Email address:      carl@msupply.foundation
PostgreSQL version: 14.11
Operating system:   MacOS
Description:

We regularly save snapshots of our app state using pg_dump and restore them
with pg_restore. However, after the latest update (12.18 etc), we can no
longer use pg_restore to restore the database -- it  just throws a bunch of
errors, mainly to do with tables not existing (that should exist).

The command we've been dumping with is:
`pg_dump -U postgres tmf_app_manager --format=custom -f
output_folder/database.dump`

And restoring with:
`pg_restore -U postgres --clean --if-exists --dbname tmf_app_manager
output_folder/database.dump`

This seems to have become a problem with ALL of the recent (Feb 8) minor
updates -- I can restore fine with 12.17, 14.10, 15.5 etc, but get the
identical failures with 12.18, 14.11, 15.6 etc. So currently there is no
"current" version we can use, which is frustrating as most repos only offer
the latest minor version (easily anyway), so it's become very
difficult/impossible to set up on a new system.

I presume there is something about our database state that is "illegal" in
some way with respect to the new updates, but we have no way to know what
exactly the problem is. Reading through the release notes doesn't offer any
insight.

I'm happy to share a copy of one of our database dumps in order to
demonstrate the problem.

On Thu, 2024-03-14 at 03:06 +0000, PG Bug reporting form wrote:
> PostgreSQL version: 14.11
> Operating system:   MacOS
>
> We regularly save snapshots of our app state using pg_dump and restore them
> with pg_restore. However, after the latest update (12.18 etc), we can no
> longer use pg_restore to restore the database -- it  just throws a bunch of
> errors, mainly to do with tables not existing (that should exist).

Please don't keep us guessing and show us the errors.  The first couple of
error messages will be enough, the remainder is often a consequence of those.

Yours,
Laurenz Albe

On Thu, 2024-03-14 at 19:03 +1300, Carl Smith wrote:
> Command failed: pg_restore -U postgres --clean --if-exists --dbname tmf_app_manager
/Users/carl/GitHub/conforma/conforma-server/database/_snapshots/TEST_14_11_2024-03-14_15-27-40/database.dump
> pg_restore: error: could not execute query: ERROR: relation "public.template" does not exist
> LINE 5: public.template
> ^
> QUERY:
> SELECT
> template.code
> FROM
> public.template
> JOIN public.template_section ON template_id = template.id
> WHERE
> template_section.id = $1;
>
>
> CONTEXT: SQL function "get_template_code" during inlining
> Command was: CREATE TABLE public.template_element (
> id integer NOT NULL,
> section_id integer NOT NULL,
> code character varying NOT NULL,
> index integer,
> title character varying,
> category public.template_element_category,
> element_type_plugin_code character varying,
> visibility_condition jsonb DEFAULT 'true'::jsonb,
> is_required jsonb DEFAULT 'true'::jsonb,
> is_editable jsonb DEFAULT 'true'::jsonb,
> validation jsonb DEFAULT 'true'::jsonb,
> initial_value jsonb,
> validation_message character varying,
> help_text character varying,
> parameters jsonb,
> reviewability public.reviewability DEFAULT 'ONLY_IF_APPLICANT_ANSWER'::public.reviewability NOT NULL,
> template_code character varying GENERATED ALWAYS AS (public.get_template_code(section_id)) STORED,
> template_version character varying GENERATED ALWAYS AS (public.get_template_version(section_id)) STORED
> );

That's not a PostgreSQL bug, that's a bug in your code.

You must have declared the function public.get_template_code(integer) as IMMUTABLE, but
it SELECTs from public.template, so it clearly is *not* immutable.  It depends on the state
of the database, in your case on the existence of a certain table (and on its contents).

So you lied when you declared the function IMMUTABLE, and the consequences are your fault.

You should instead use a trigger to populate template_code.  That is the correct solution,
and then you wouldn't get that error.


If that worked previously, it's purely by coincidence: if public.template happens to exist,
the restore doesn't fail.

Yours,
Laurenz Albe

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> That's not a PostgreSQL bug, that's a bug in your code.

> You must have declared the function public.get_template_code(integer) as IMMUTABLE, but
> it SELECTs from public.template, so it clearly is *not* immutable.  It depends on the state
> of the database, in your case on the existence of a certain table (and on its contents).

Yeah.  The concrete problem so far as pg_dump is concerned is that
it can't see the dependencies that the body of get_template_code has,
so it doesn't realize that it would have to postpone creation of this
table till after public.template is created.  There are various ways
that you could hack around that by preventing the function from being
considered for inlining, but as Laurenz suggests, that's just
band-aiding over a fundamentally unsafe design.  Pretending that this
function is immutable will bite you eventually.

BTW, I believe that what broke it for you as of the current releases
is commits 743ddafc7 et al, which caused GENERATED expressions to be
run through expression preprocessing at CREATE TABLE time:

    Two actual bugs of this ilk are fixed here.  We failed to preprocess
    column GENERATED expressions before checking mutability, so that the
    code could fail to detect the use of a volatile function
    default-argument expression, or it could reject a polymorphic function
    that is actually immutable on the datatype of interest.  Likewise,
    column DEFAULT expressions weren't preprocessed before determining if
    it's safe to apply the attmissingval mechanism.  A false negative
    would just result in an unnecessary table rewrite, but a false
    positive could allow the attmissingval mechanism to be used in a case
    where it should not be, resulting in unexpected initial values in a
    new column.

One of the things that happens in that preprocessing is inlining
of inline-able SQL functions.  This particular function is not
inline-able I think, but we'd hit the parser error on the way to
discovering that.  (Hmm, I wonder if we could make inline_function
do some tests on the raw parse tree so it could bail out earlier;
I think we could verify that it's a SELECT with no FROM before
invoking parse analysis.  The extra complication could be justified
as saving cycles for obviously non-inlinable SQL functions.)

            regards, tom lane

Thanks Tom and Laurenz,

This is very helpful information and I’m sure will allow me to fix our database problem.

Kind regards,

Carl

> On 15 Mar 2024, at 10:49 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Laurenz Albe <laurenz.albe@cybertec.at> writes:
>> That's not a PostgreSQL bug, that's a bug in your code.
>
>> You must have declared the function public.get_template_code(integer) as IMMUTABLE, but
>> it SELECTs from public.template, so it clearly is *not* immutable.  It depends on the state
>> of the database, in your case on the existence of a certain table (and on its contents).
>
> Yeah.  The concrete problem so far as pg_dump is concerned is that
> it can't see the dependencies that the body of get_template_code has,
> so it doesn't realize that it would have to postpone creation of this
> table till after public.template is created.  There are various ways
> that you could hack around that by preventing the function from being
> considered for inlining, but as Laurenz suggests, that's just
> band-aiding over a fundamentally unsafe design.  Pretending that this
> function is immutable will bite you eventually.
>
> BTW, I believe that what broke it for you as of the current releases
> is commits 743ddafc7 et al, which caused GENERATED expressions to be
> run through expression preprocessing at CREATE TABLE time:
>
>    Two actual bugs of this ilk are fixed here.  We failed to preprocess
>    column GENERATED expressions before checking mutability, so that the
>    code could fail to detect the use of a volatile function
>    default-argument expression, or it could reject a polymorphic function
>    that is actually immutable on the datatype of interest.  Likewise,
>    column DEFAULT expressions weren't preprocessed before determining if
>    it's safe to apply the attmissingval mechanism.  A false negative
>    would just result in an unnecessary table rewrite, but a false
>    positive could allow the attmissingval mechanism to be used in a case
>    where it should not be, resulting in unexpected initial values in a
>    new column.
>
> One of the things that happens in that preprocessing is inlining
> of inline-able SQL functions.  This particular function is not
> inline-able I think, but we'd hit the parser error on the way to
> discovering that.  (Hmm, I wonder if we could make inline_function
> do some tests on the raw parse tree so it could bail out earlier;
> I think we could verify that it's a SELECT with no FROM before
> invoking parse analysis.  The extra complication could be justified
> as saving cycles for obviously non-inlinable SQL functions.)
>
>             regards, tom lane