BUG #18151: pg_upgradecluster fails when column default refers to column

PG Bug reporting form <noreply@postgresql.org> writes:

I was trying to upgrade to PG 16:
pg_upgradecluster 15 main

I got this error:
pg_restore: error: could not execute query: ERROR: relation "event" does
not exist

The reason is the the default value for column "event_id" of table "event"
is "event_id_nextval()". That function is:
SELECT COALESCE(MAX(event_id) + 1, 1) FROM event;

Thanks for the report!

I do not understand why it would have to execute "event_id_nextval()" to do
the restore.

It shouldn't. I think this might be a variant of the bug recently
reported here:

/messages/by-id/75a7b7483aeb331aa017328d606d568fc715b90d.camel@cybertec.at

However, that doesn't seem quite right because that'd result in a useless
extra evaluation during COPY to the table, at which time the table
surely must exist. Also, pg_upgrade shouldn't need to use COPY at all.

Is that function written in old-style (with a string literal for the
body) or new-style with BEGIN ATOMIC? In the latter case it's possible
that you've got a circular dependency that pg_dump is failing to work
around.

Can you show us the exact DDL definition of both the table and the
function?

A related issue: It would have been better if pg_upgradecluster had not
output a success message and switched to the new cluster. It should have
detected the error, output an error message, and left the old cluster
running.

As far as that goes, you'd have to complain to the Debian maintainers
of pg_upgradecluster. That code doesn't belong to the core project.

regards, tom lane

On Sunday, October 8, 2023, PG Bug reporting form <noreply@postgresql.org>
wrote:

The following bug has been logged on the website:

Bug reference: 18151
Logged by: Liam Morland
Email address: liam@morland.ca
PostgreSQL version: 15.4
Operating system: Debian
Description:

I was trying to upgrade to PG 16:
pg_upgradecluster 15 main

I got this error:
pg_restore: error: could not execute query: ERROR: relation "event" does
not exist

The reason is the the default value for column "event_id" of table "event"
is "event_id_nextval()". That function is:
SELECT COALESCE(MAX(event_id) + 1, 1) FROM event;

You are expressly forbidden to have the default value expression of a
column reference a subquery. You must use a trigger. The failure to
prevent the exoressions creation or consistently report such a failure is
the bug, not this. Unfortunately preventing the behavior is not
reasonable, we can only document its forbiddance.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

On Sunday, October 8, 2023, PG Bug reporting form <noreply@postgresql.org>
wrote:

The reason is the the default value for column "event_id" of table "event"
is "event_id_nextval()". That function is:
SELECT COALESCE(MAX(event_id) + 1, 1) FROM event;

You are expressly forbidden to have the default value expression of a
column reference a subquery. You must use a trigger.

Well, you're not allowed to do it directly:

regression=# create table foo (f1 int default ((select max(f1) from foo)));
ERROR: cannot use subquery in DEFAULT expression
LINE 1: create table foo (f1 int default ((select max(f1) from foo))...
^

but that's just a minor implementation restriction. Doing it through
a function is fine. (Whether it's a good idea is another question.)
Unlike, say, CHECK constraints, there's not any expectation that
a default expression be immutable or avoid dependence on database
state --- if there was, "default nextval(...)" would be problematic.

In any case, dump/restore ought not fail like this.

regards, tom lane

2023-10-09 10:11-0400 Tom Lane <tgl@sss.pgh.pa.us> wrote: [...]

Is that function written in old-style (with a string literal for the
body) or new-style with BEGIN ATOMIC? [...]

It is old-style. I wrote it before PG 14.

Can you show us the exact DDL definition of both the table and the
function? [...]

From the output of pg_dump:

CREATE FUNCTION public.event_id_nextval() RETURNS integer
LANGUAGE sql
AS $$
SELECT COALESCE(MAX(event_id) + 1, 1) FROM event;
$$;

CREATE TABLE public.event (
event_id integer DEFAULT public.event_id_nextval() NOT NULL,
date_start date NOT NULL,
date_end date NOT NULL,
title text NOT NULL,
CONSTRAINT date_end_gt_date_start CHECK ((date_end >= date_start))
);

ALTER TABLE ONLY public.event
ADD CONSTRAINT event_pkey PRIMARY KEY (event_id);

Regards,
Liam

Liam Morland <liam@morland.ca> writes:

2023-10-09 10:11-0400 Tom Lane <tgl@sss.pgh.pa.us> wrote: [...]

Is that function written in old-style (with a string literal for the
body) or new-style with BEGIN ATOMIC? [...]

It is old-style. I wrote it before PG 14.

OK, so it should work ...

Can you show us the exact DDL definition of both the table and the
function? [...]

From the output of pg_dump:

CREATE FUNCTION public.event_id_nextval() RETURNS integer
LANGUAGE sql
AS $$
SELECT COALESCE(MAX(event_id) + 1, 1) FROM event;
$$;

CREATE TABLE public.event (
event_id integer DEFAULT public.event_id_nextval() NOT NULL,

Oh ... I thought of a plausible explanation, or part of an
explanation. That function is not search-path-safe: if it's run with
a search_path that doesn't include "public", it'll fail as described.
And indeed dump/restore will use a restrictive search_path setting.
So the COPY bug I alluded to before could trigger the reported
failure, if the upgrade is transferring data to the new cluster using
COPY rather than physically moving files around.

I see that pg_upgradecluster defaults to using dump/restore rather
than pg_upgrade, which surprises me, but if you used that mode then
all is explained.

Bug or no bug, that function would be better written as

SELECT COALESCE(MAX(event_id) + 1, 1) FROM public.event;

so that it still works under a restrictive search path.

regards, tom lane