Horton

Thread

Commits

  1. Record dependencies of a cast on other casts that it requires.

  2. Adjust pg_dump's priority ordering for casts.

  1. PG upgrade 14->15 fails - database contains our own extension

    david.turon@linuxbox.cz — 2022-10-13T12:40:34Z

    
Hi community,

I have problem with pg_upgrade. Tested from 14.5 to 15.0 rc2 when database
contains our extension with one new type. Using pg_dump & restore works
well.

We made workaround extension for some usage in javascript library that
contains new type that represents bigint as text. So something like auto
conversion from SELECT (2^32)::text -> bigint when data is stored and
(2^32) -> text when data is retrieved.

Im not sure if this is postgresql bug or we have something wrong in our
extension with cast. So becouse this im writing there.

Here is output of pg_upgrade:
(our extension name is lbuid, our type public.lbuid)

command: "/usr/pgsql-15/bin/pg_dump" --host /tmp/pg_upgrade_log --port
50432 --username postgres --schema-only --quote-all-identifiers
--binary-upgrade --format=custom
--file="/home/pgsql/data_new/pg_upgrade_output.d/20221013T104924.054/
dump/pg_upgrade_dump_16385.custom" 'dbname=lbstat' >>
"/home/pgsql/data_new/pg_upgrade_output.d/20221013T104924.054/log/pg_upgrade_dump_16385.log"
 2>&1


command: "/usr/pgsql-15/bin/pg_restore" --host /tmp/pg_upgrade_log --port
50432 --username postgres --create --exit-on-error --verbose --dbname
template1
"/home/pgsql/data_new/pg_upgrade_output.d/20221013T104924.054/dump/pg_upgrade_dump_1
6385.custom" >>
"/home/pgsql/data_new/pg_upgrade_output.d/20221013T104924.054/log/pg_upgrade_dump_16385.log"
 2>&1
pg_restore: connecting to database for restore
pg_restore: creating DATABASE "lbstat"
pg_restore: connecting to new database "lbstat"
pg_restore: creating DATABASE PROPERTIES "lbstat"
pg_restore: connecting to new database "lbstat"
pg_restore: creating pg_largeobject "pg_largeobject"
pg_restore: creating SCHEMA "public"
pg_restore: creating COMMENT "SCHEMA "public""
pg_restore: creating EXTENSION "lbuid"
pg_restore: creating COMMENT "EXTENSION "lbuid""
pg_restore: creating SHELL TYPE "public.lbuid"
pg_restore: creating FUNCTION "public.lbuid_in("cstring")"
pg_restore: creating FUNCTION "public.lbuid_out("public"."lbuid")"
pg_restore: creating TYPE "public.lbuid"
pg_restore: creating CAST "CAST (integer AS "public"."lbuid")"
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 3751; 2605 16393 CAST CAST (integer AS
"public"."lbuid") (no owner)
pg_restore: error: could not execute query: ERROR:  return data type of
cast function must match or be binary-coercible to target data type
Command was: CREATE CAST (integer AS "public"."lbuid") WITH FUNCTION
"pg_catalog"."int8"(integer) AS IMPLICIT;

-- For binary upgrade, handle extension membership the hard way
ALTER EXTENSION "lbuid" ADD CAST (integer AS "public"."lbuid");


Hint is good, but this cast is already member of our extension:

lbstat=# ALTER EXTENSION lbuid ADD CAST (integer AS public.lbuid);
ERROR:  cast from integer to lbuid is already a member of extension "lbuid"


Database contains this:

CREATE EXTENSION lbuid ;
CREATE TABLE test(id lbuild);
INSERT INTO test VALUES ('1344456644646645456');

Tested on our distribution based on centos7.

When i drop this cast from extension manualy a then manualy restore after
pg_upgrade, then operation is without failure.

In attachment are extension files.

Thanks.

Best regards. David T.
(See attached file: lbuid.control)(See attached file: lbuid--0.1.0.sql)

--
-------------------------------------
Ing. David TUROŇ
LinuxBox.cz, s.r.o.
28. rijna 168, 709 01 Ostrava

tel.:    +420 591 166 224
fax:    +420 596 621 273
mobil:  +420 732 589 152
www.linuxbox.cz

mobil servis: +420 737 238 656
email servis: servis@linuxbox.cz
-------------------------------------

  2. Re: PG upgrade 14->15 fails - database contains our own extension

    Robert Haas <robertmhaas@gmail.com> — 2022-10-13T14:27:56Z

    On Thu, Oct 13, 2022 at 9:57 AM David Turoň <david.turon@linuxbox.cz> wrote:
> pg_restore: creating TYPE "public.lbuid"
> pg_restore: creating CAST "CAST (integer AS "public"."lbuid")"
> pg_restore: while PROCESSING TOC:
> pg_restore: from TOC entry 3751; 2605 16393 CAST CAST (integer AS "public"."lbuid") (no owner)
> pg_restore: error: could not execute query: ERROR:  return data type of cast function must match or be binary-coercible to target data type
> Command was: CREATE CAST (integer AS "public"."lbuid") WITH FUNCTION "pg_catalog"."int8"(integer) AS IMPLICIT;

I think the error is complaining that the return type of
int8(integer), which is bigint, needs to coercible WITHOUT FUNCTION to
lbuid. Your extension contains such a cast, but at the point when the
error occurs, it hasn't been restored yet. That suggests that either
the cast didn't get included in the dump file, or it got included in
the wrong order. A quick test suggest the latter. If I execute your
SQL file and the dump the database, I get:

CREATE CAST (integer AS public.lbuid) WITH FUNCTION
pg_catalog.int8(integer) AS IMPLICIT;
CREATE CAST (bigint AS public.lbuid) WITHOUT FUNCTION AS IMPLICIT;
CREATE CAST (public.lbuid AS bigint) WITHOUT FUNCTION AS IMPLICIT;

That's not a valid dump ordering, and if I drop the casts and try to
recreate them that way, it fails in the same way you saw.

My guess is that this is a bug in Tom's commit
b55f2b6926556115155930c4b2d006c173f45e65, "Adjust pg_dump's priority
ordering for casts."

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

  3. Re: PG upgrade 14->15 fails - database contains our own extension

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-10-13T15:23:18Z

    Robert Haas <robertmhaas@gmail.com> writes:
> CREATE CAST (integer AS public.lbuid) WITH FUNCTION
> pg_catalog.int8(integer) AS IMPLICIT;
> CREATE CAST (bigint AS public.lbuid) WITHOUT FUNCTION AS IMPLICIT;
> CREATE CAST (public.lbuid AS bigint) WITHOUT FUNCTION AS IMPLICIT;

> That's not a valid dump ordering, and if I drop the casts and try to
> recreate them that way, it fails in the same way you saw.

> My guess is that this is a bug in Tom's commit
> b55f2b6926556115155930c4b2d006c173f45e65, "Adjust pg_dump's priority
> ordering for casts."

Hmm ... I think it's a very ancient bug that somehow David has avoided
tripping over up to now.  Namely, that we require the bigint->lbuid
implicit cast to exist in order to make that WITH FUNCTION cast, but
we fail to record it as a dependency during CastCreate.  So pg_dump
is flying blind as to the required restoration order, and if it ever
worked, you were just lucky.

We might be able to put in some kluge in pg_dump to make it less
likely to fail with existing DBs, but I think the true fix lies
in adding that dependency.

(I'm pretty skeptical about it being a good idea to have a set of
casts like this, but I don't suppose pg_dump is chartered to
editorialize on that.)

			regards, tom lane

  4. Re: PG upgrade 14->15 fails - database contains our own extension

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-10-13T16:06:46Z

    I wrote:
> Hmm ... I think it's a very ancient bug that somehow David has avoided
> tripping over up to now.

Looking closer, I don't see how b55f2b692 could have changed pg_dump's
opinion of the order to sort these three casts in; that sort ordering
logic is old enough to vote.  So I'm guessing that in fact this *never*
worked.  Perhaps this extension has never been through pg_upgrade before,
or at least not with these casts?

> We might be able to put in some kluge in pg_dump to make it less
> likely to fail with existing DBs, but I think the true fix lies
> in adding that dependency.

I don't see any painless way to fix this in pg_dump, and I'm inclined
not to bother trying if it's not a regression.  Better to spend the
effort on the backend-side fix.

On the backend side, really anyplace that we consult IsBinaryCoercible
during DDL is at hazard.  While there aren't a huge number of such
places, there's certainly more than just CreateCast.  I'm trying to
decide how much trouble it's worth going to there.  I could be wrong,
but I think that only the cast-vs-cast case is really likely to be
problematic for pg_dump, given that it dumps casts pretty early now.
So it might be sufficient to fix that one case.

			regards, tom lane

  5. Re: PG upgrade 14->15 fails - database contains our own extension

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-10-13T18:51:30Z

    I wrote:
> We might be able to put in some kluge in pg_dump to make it less
> likely to fail with existing DBs, but I think the true fix lies
> in adding that dependency.

Here's a draft patch for that.  I'm unsure whether it's worth
back-patching; even if we did, we couldn't guarantee that existing
databases would have the additional pg_depend entries.

If we do only put it in HEAD, maybe we should break compatibility
to the extent of changing IsBinaryCoercible's API rather than
inventing a separate call.  I'm still not excited about recording
additional dependencies elsewhere, but that path would leave us
with cleaner code if we eventually do that.

			regards, tom lane

  6. Re: PG upgrade 14->15 fails - database contains our own extension

    david.turon@linuxbox.cz — 2022-10-14T05:38:29Z

    

Hi,

I really appreciate your help and very quick response. And WOW, write patch
for this in few hours ...that's amazing!

> Looking closer, I don't see how b55f2b692 could have changed pg_dump's
> opinion of the order to sort these three casts in; that sort ordering
> logic is old enough to vote.  So I'm guessing that in fact this *never*
> worked.  Perhaps this extension has never been through pg_upgrade before,
> or at least not with these casts?

Yes its new and I tested right now with upgrade from 9.6 to 15.0 rc2 with
same result. So this behavior is probably long time there, but extension is
new and not upgraded yet. And probably nobody have this "strange" idea.


>(I'm pretty skeptical about it being a good idea to have a set of
casts like this, but I don't suppose pg_dump is chartered to
editorialize on that.)
Yes, im not proud of the creation this workaround extension and I did what
frontend develepers asked me if it's possible. I don't expect a medal of
honor:)

The problem was when bigint was taken from DB as json and stored as number
JS library cast number automaticaly to integer that cause problem.

lbstat=# SELECT json_agg(test) FROM test;
       json_agg
-----------------------
 [{"id":"4294967296"}]
(1 row)

-- ID was represnted now as text and JS library can use it and sent back
without error. But for DB is still bigint.

This was automatic way to solve this problem without casting on all places
to text. I tested and most things works well until upgrade test didn't
pass.

Thank you all.

David T.

--
-------------------------------------
Ing. David TUROŇ
LinuxBox.cz, s.r.o.
28. rijna 168, 709 01 Ostrava

tel.:    +420 591 166 224
fax:    +420 596 621 273
mobil:  +420 732 589 152
www.linuxbox.cz

mobil servis: +420 737 238 656
email servis: servis@linuxbox.cz
-------------------------------------



Od:	"Tom Lane" <tgl@sss.pgh.pa.us>
Komu:	"David Turoň" <david.turon@linuxbox.cz>
Kopie:	"Robert Haas" <robertmhaas@gmail.com>,
            pgsql-hackers@postgresql.org, "Marian Krucina"
            <marian.krucina@linuxbox.cz>
Datum:	13.10.2022 18:06
Předmět:	Re: PG upgrade 14->15 fails - database contains our own
            extension



I wrote:
> Hmm ... I think it's a very ancient bug that somehow David has avoided
> tripping over up to now.

Looking closer, I don't see how b55f2b692 could have changed pg_dump's
opinion of the order to sort these three casts in; that sort ordering
logic is old enough to vote.  So I'm guessing that in fact this *never*
worked.  Perhaps this extension has never been through pg_upgrade before,
or at least not with these casts?

> We might be able to put in some kluge in pg_dump to make it less
> likely to fail with existing DBs, but I think the true fix lies
> in adding that dependency.

I don't see any painless way to fix this in pg_dump, and I'm inclined
not to bother trying if it's not a regression.  Better to spend the
effort on the backend-side fix.

On the backend side, really anyplace that we consult IsBinaryCoercible
during DDL is at hazard.  While there aren't a huge number of such
places, there's certainly more than just CreateCast.  I'm trying to
decide how much trouble it's worth going to there.  I could be wrong,
but I think that only the cast-vs-cast case is really likely to be
problematic for pg_dump, given that it dumps casts pretty early now.
So it might be sufficient to fix that one case.

		 		 		 regards, tom lane