Subject: Re: Move defaults toward ICU in 16?

From: Jeff Davis <[email-masked]>
Date: Fri, 17 Feb 2023 00:06:06 -0800

On Tue, 2023-02-14 at 09:59 -0800, Andres Freund wrote:
> I am saying that pg_upgrade should be able to deal with the
> difference. The
> details of how to implement that, don't matter that much.

To clarify, you're saying that pg_upgrade should simply update
pg_database to set the new databases' collation fields equal to that of
the old cluster?

I'll submit it as a separate patch because it would be independently
useful.

Regards,
	Jeff Davis

---

From: Jeff Davis <[email-masked]>
Date: Fri, 17 Feb 2023 09:01:54 -0800

> pg_database to set the new databases' collation fields equal to that

Thinking about this more, it's not clear to me if this would be in
scope for pg_upgrade or not. If pg_upgrade is fixing up the new cluster
rather than checking for compatibility, why doesn't it just take over
and do the initdb for the new cluster itself? That would be less
confusing for users, and avoid some weirdness (like, if you drop the
database "postgres" on the original, why does it reappear after an
upgrade?).

Someone might want to do something interesting to the new cluster
before the upgrade, but it's not clear from the docs what would be both
useful and safe.

Regards,
	Jeff Davis

---

From: Andres Freund <[email-masked]>
Date: Fri, 17 Feb 2023 09:05:23 -0800

Hi,

> > pg_database to set the new databases' collation fields equal to that

Yes.

> scope for pg_upgrade or not.

I don't think we should consider changing the default collation provider
without making this more seamless, one way or another.

> If pg_upgrade is fixing up the new cluster rather than checking for
> compatibility, why doesn't it just take over and do the initdb for the new
> cluster itself? That would be less confusing for users, and avoid some
> weirdness (like, if you drop the database "postgres" on the original, why
> does it reappear after an upgrade?).

I've wondered about that as well. There are some initdb-time options you can
set, but pg_upgrade could forward those.

Greetings,

Andres Freund

---

From: Jeff Davis <[email-masked]>
Date: Fri, 17 Feb 2023 10:00:41 -0800

> without making this more seamless, one way or another.

I guess I'm fine hacking pg_upgrade, but I think I'd like to make it
conditional on this specific case: only perform the fixup if the old
cluster is 15 or earlier and using libc and the newer cluster is 16 or
later and using icu.

There's already a check that the new cluster is empty, so I think it's
safe to hack the pg_database locale fields.

Regards,
	Jeff Davis

---

From: Andres Freund <[email-masked]>
Date: Fri, 17 Feb 2023 10:09:09 -0800

Hi,

> cluster is 15 or earlier and using libc and the newer cluster is 16 or

-1. That's just going to cause pain one major version upgrade further down the
line. Why would we want to incur that pain?

> safe to hack the pg_database locale fields.

I don't think we need to, we do issue the CREATE DATABASEs. So we just need to
make sure that includes the collation provider info, and the proper template
database, in pg_upgrade mode.

Greetings,

Andres Freund

---

From: Justin Pryzby <[email-masked]>
Date: Fri, 17 Feb 2023 12:41:40 -0600

> before the upgrade, but it's not clear from the docs what would be both

This came up before - I'm of the opinion that it's unsupported and/or
useless to try to do anything on the new cluster between initdb and
pg_upgrade.

https://www.postgresql.org/message-id/[email-masked]
https://www.postgresql.org/message-id/[email-masked]

-- 
Justin

---

From: Jeff Davis <[email-masked]>
Date: Fri, 17 Feb 2023 12:36:05 -0800

> line. Why would we want to incur that pain?

OK, we can just always do the fixup as long as the old one is libc and
the new one is ICU. I'm just trying to avoid this becoming a general
mechanism to fix up an incompatible new cluster.

> database, in pg_upgrade mode.

We must fixup template1/postgres in the new cluster (change it to libc
to match the old cluster), because any objects existing in those
databases in the old cluster may depend on the default collation. I
don't see how we can do that without updating pg_database, so I'm not
following your point.

(I think you're right that template0 is optional; but since we're
fixing up the other databases it would be less surprising if we also
fixed up template0.)

And if we do fixup template0/template1/postgres to match the old
cluster, then CREATE DATABASE will have no issue.

-- 
Jeff Davis
PostgreSQL Contributor Team - AWS

---

From: Andres Freund <[email-masked]>
Date: Fri, 17 Feb 2023 12:50:09 -0800

Hi,

> don't see how we can do that without updating pg_database, so I'm not

I think we just drop/recreate template1 and postgres during pg_upgrade.  Yep,
looks like it. See create_new_objects():

		/*
		 * template1 database will already exist in the target installation,
		 * so tell pg_restore to drop and recreate it; otherwise we would fail
		 * to propagate its database-level properties.
		 */
		create_opts = "--clean --create";

and then:

		/*
		 * postgres database will already exist in the target installation, so
		 * tell pg_restore to drop and recreate it; otherwise we would fail to
		 * propagate its database-level properties.
		 */
		if (strcmp(old_db->db_name, "postgres") == 0)
			create_opts = "--clean --create";
		else
			create_opts = "--create";

Greetings,

Andres Freund

---

From: Robert Haas <[email-masked]>
Date: Sat, 18 Feb 2023 14:08:46 +0530

> before the upgrade, but it's not clear from the docs what would be both

I agree with all of this. I think it would be fantastic if pg_upgrade
did the initdb itself. It would be simple to make this optional
behavior, too: if the destination directory does not exist or is
empty, initdb into it, otherwise skip that. That might be too
automagical, so we could add add a --no-initdb option. If not
specified, the destination directory must either not exist or be
empty; else it must exist and look like a data directory.

I completely concur with the idea that doing something with the new
cluster before the upgrade is weird, and I don't think we should
encourage people to do it. Nevertheless, as the threads to which
Justin linked probably say, I'm not sure that it's a good idea to
completely slam the door shut on that option. If we did want to move
in that direction, though, having pg_upgrade do the initdb would be an
excellent first step. We could at some later time decide to remove the
--no-initdb option; or maybe we'll decide that it's good to keep it
for emergencies, which is my present bias. In any event, the resulting
system would be more usable and less error-prone than what we have
today.

-- 
Robert Haas
EDB: http://www.enterprisedb.com