Thread: pg_upgrade vs user created range type extension
I have just encountered an apparent bug in pg_upgrade (or possibly pg_dump). To recreate, install the cranges extension - which can be obtained via "git clone https://bitbucket.org/adunstan/pg-closed-ranges.git" - for both 9.4 and 9.5. Create a fresh 9.4 instance, create a database and in it run "create extension cranges schema pg_catalog". Then create a fresh 9.5 instance and try to pg_upgrade from the 9.4 instance to the 9.5 instance. Here's the tail of the log: pg_restore: creating SCHEMA "public" pg_restore: creating COMMENT "SCHEMA "public"" pg_restore: creating EXTENSION"cranges" pg_restore: creating COMMENT "EXTENSION "cranges"" pg_restore: creating SHELL TYPE "pg_catalog.cdaterange" pg_restore: creating FUNCTION "pg_catalog.cdaterange_canonical("cdaterange")" pg_restore: creatingTYPE "pg_catalog.cdaterange" pg_restore: creating SHELL TYPE "pg_catalog.cint4range" pg_restore: creating FUNCTION "pg_catalog.cint4range_canonical("cint4range")" pg_restore: creating TYPE "pg_catalog.cint4range" pg_restore:creating SHELL TYPE "pg_catalog.cint8range" pg_restore: creating FUNCTION "pg_catalog.cint8range_canonical("cint8range")" pg_restore: creating TYPE "pg_catalog.cint8range" pg_restore: creatingFUNCTION "pg_catalog.cdaterange("date", "date")" pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore:[archiver (db)] Error from TOC entry 191; 1255 16389 FUNCTION cdaterange("date", "date") andrew pg_restore:[archiver (db)] could not execute query: ERROR: function "cdaterange" already exists with same argument types Command was: CREATE FUNCTION "cdaterange"("date", "date") RETURNS "cdaterange" LANGUAGE "internal" IMMUTABLE AS $$range_construct... The end result is that I currently can't upgrade a database using this extension, which is rather ugly. Similar things happen if I put the extension in public instead of pg_catalog. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> I have just encountered an apparent bug in pg_upgrade (or possibly pg_dump).
Hmm, it sort of looks like pg_dump believes it should dump the range's
constructor function in binary-upgrade mode, while the backend is creating
the constructor function during CREATE TYPE anyway. But if that's the
case, upgrade of user-defined range types would never have worked ...
seems like we should have noticed before now.
If that diagnosis is correct, we should either change pg_dump to not
dump that function, or change CREATE TYPE AS RANGE to not auto-create
the constructor functions in binary-upgrade mode. The latter might be
more flexible in the long run.
regards, tom lane
On 09/22/2016 07:33 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> I have just encountered an apparent bug in pg_upgrade (or possibly pg_dump). > Hmm, it sort of looks like pg_dump believes it should dump the range's > constructor function in binary-upgrade mode, while the backend is creating > the constructor function during CREATE TYPE anyway. But if that's the > case, upgrade of user-defined range types would never have worked ... > seems like we should have noticed before now. > > If that diagnosis is correct, we should either change pg_dump to not > dump that function, or change CREATE TYPE AS RANGE to not auto-create > the constructor functions in binary-upgrade mode. The latter might be > more flexible in the long run. > > Yeah, I think your diagnosis is correct. I'm not sure I see the point of the flexibility given that you can't specify a constructor function for range types (if that feature had been available I would probably have used it in this extension). cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> On 09/22/2016 07:33 PM, Tom Lane wrote:
>> If that diagnosis is correct, we should either change pg_dump to not
>> dump that function, or change CREATE TYPE AS RANGE to not auto-create
>> the constructor functions in binary-upgrade mode. The latter might be
>> more flexible in the long run.
> Yeah, I think your diagnosis is correct. I'm not sure I see the point of
> the flexibility given that you can't specify a constructor function for
> range types (if that feature had been available I would probably have
> used it in this extension).
It turns out that pg_dump already contains logic that's meant to exclude
constructor functions from getting dumped, but it's broken for binary
upgrades from pre-9.6 branches because somebody fat-fingered the AND/OR
nesting in the giant WHERE clauses in getFuncs(). Curiously, it's *not*
broken when dumping from >= 9.6, which explains why I couldn't reproduce
this in HEAD. It looks like Stephen fixed this while adding the
pg_init_privs logic, while not realizing that he'd left it broken in the
existing cases.
The comment in front of all this is nearly as confused as the code is,
too. Will fix.
regards, tom lane
On 09/23/2016 11:55 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 09/22/2016 07:33 PM, Tom Lane wrote: >>> If that diagnosis is correct, we should either change pg_dump to not >>> dump that function, or change CREATE TYPE AS RANGE to not auto-create >>> the constructor functions in binary-upgrade mode. The latter might be >>> more flexible in the long run. >> Yeah, I think your diagnosis is correct. I'm not sure I see the point of >> the flexibility given that you can't specify a constructor function for >> range types (if that feature had been available I would probably have >> used it in this extension). > It turns out that pg_dump already contains logic that's meant to exclude > constructor functions from getting dumped, but it's broken for binary > upgrades from pre-9.6 branches because somebody fat-fingered the AND/OR > nesting in the giant WHERE clauses in getFuncs(). Curiously, it's *not* > broken when dumping from >= 9.6, which explains why I couldn't reproduce > this in HEAD. It looks like Stephen fixed this while adding the > pg_init_privs logic, while not realizing that he'd left it broken in the > existing cases. > > The comment in front of all this is nearly as confused as the code is, > too. Will fix. > > Great, Thanks. cheers andrew