On Wed, Jun 07, 2023 at 10:36:29AM -0700, Jeff Davis wrote:
> If we went almost two release cycles without anyone noticing, then
> perhaps we should just get rid of the ability to dump pg_catalog. But I
> think the attached patch clarifies things regardless.
That looks correct for v15 and HEAD, not when dumping from older
versions.
Attempting the same command with a patched pg_dump on ~14 triggers
more failures. The first assertion hit is this one, for instance:
$ pg_dump -n pg_catalog
pg_dump: pg_dump.c:13460: dumpCollation: Assertion `colliculocale != ((void *)0)' failed.
colliculocale and collicurules don't exist there, but the code is
forcing the value to NULL as PQfnumber() returns -1 for an attribute
that does not exist.
else if (collprovider[0] == 'i')
{
+ Assert(colliculocale != NULL);
+ Assert(collcollate == NULL);
+ Assert(collctype == NULL);
If ICU provides the collation, colliculocale is NULL when dumping from
~14, while both collcollate and collctype are not NULL. Hence all
three assertions are incorrect for ~14, while they are correct for
15~. It seems to me that you are going to require a per-version
handling here if you wish to keep collprovider on top of the rest.
I'd like to agree with you about the fact that having collprovider
handled before the rest makes things easier to follow in the future.
At least that's my feeling after looking at your patch.
--
Michael