| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | Bohyun Lee <bohyun(dot)lee(at)databricks(dot)com> |
| Cc: | shihao zhong <zhong950419(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | pg_upgrade doesn't copy ICU rules on template0 |
| Date: | 2026-10-28 12:09:50 |
| Message-ID: | 8c65df31-345e-429c-98fa-867f08d703b6@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Shihao reported at [1] that pg_upgrade doesn't copy template0's ICU
rules. I think that was an oversight commit 30a53b792959, which added
ICU rules.
I'm starting this new thread for that issue, because it's independent
from the pg_upgrade --initdb feature being discussed in that thread.
Bohyun included a fix in the latest --initdb patch, below is my reply to
that part.
On 27/10/2026 16:49, Bohyun Lee wrote:
>> 1. ICU rules are not copied.
>
> Thanks for catching that. The patch is updated so the upgrade now copies
> template0.daticurules.
> I used a parameterized update so that multiline and long ICU rules are
> preserved as well.
> + /*
> + * Copy ICU rules separately as a parameter: they can contain newlines or
> + * exceed the command and query buffers. Other databases' rules are
> + * restored by pg_dump/pg_restore.
> + */
> + if (GET_MAJOR_VERSION(new_cluster.major_version) >= 1600)
> + {
> + const char *params[] = {locale->db_icu_rules};
> + PGresult *res;
> +
> + res = PQexecParams(conn_new_template1,
> + "UPDATE pg_catalog.pg_database SET daticurules = $1 "
> + "WHERE datname = 'template0'",
> + 1, NULL, params, NULL, NULL, 0);
> + if (PQresultStatus(res) != PGRES_COMMAND_OK)
> + pg_fatal("could not set ICU rules for new cluster's template0: %s",
> + PQerrorMessage(conn_new_template1));
> + PQclear(res);
> + }
> +
PQescapeLiteral(), which is used to escape the other options, should
handle newlines just fine. And if we're worried about exceeding the
query buffer, I think we should teach executeQueryOrDie() to grow the
buffer. pg_log() might not deal nicely with arbitrarily long inputs
either, though.
Alternatively, switch to using PQexecParams() for all the queries in the
function. But I think I'd prefer executeQueryOrDie(), because on error,
it prints the full query with the params inlined, and I find that nicer
when debugging. If it was a large binary blob, that might be excessive,
but I think ICU rules should not be _that_ long.
P.S. Looking at set_locale_and_encoding(), why does it have separate
branches for different target cluster versions? We only support the
current version as the target. Looks like commit 206b44bb24b started
that trend, but it seems pointless.
- Heikki
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Greg Burd | 2026-10-28 12:23:15 | Re: Hash index inserts serialise on the metapage tuple counter |
| Previous Message | Fujii Masao | 2026-10-28 11:45:01 | Re: Reduce logging during prepared transaction recovery |