Re: [GENERAL] pg_upgrade error regarding hstore operator

"Feld, Michael (IMS)" <FeldM(at)imsweb(dot)com> writes:
> In addition, I noticed the following differences in the pg_depend catalog, there are 4 entries in 9.1 that are missing in 9.5 which I have separated with ***:
> select classid::regclass, objid, objsubid, refclassid::regclass, refobjid, refobjsubid, deptype, opcname, opfname from pg_depend pgd left join pg_opclass on pgd.objid = pg_opclass.oid left join pg_opfamily on pgd.objid = pg_opfamily.oid where classid in ('pg_opfamily'::regclass, 'pg_opclass'::regclass) order by 7,8,9;
> ***
> "pg_opfamily";325462122;0;"pg_extension";325462055;0;"e";"";"btree_hstore_ops"
> "pg_opfamily";325462163;0;"pg_extension";325462055;0;"e";"";"gin_hstore_ops"
> "pg_opfamily";325462146;0;"pg_extension";325462055;0;"e";"";"gist_hstore_ops"
> "pg_opfamily";325462131;0;"pg_extension";325462055;0;"e";"";"hash_hstore_ops"
> ***

Ah-hah, there's the problem: the pg_upgrade process fails to restore the
extension membership of the opfamilies that are implicitly created by
hstore's CREATE OPERATOR CLASS commands. The reason for this is that in
binary-upgrade mode, the backend doesn't install any extension membership
entries automatically at all, but leaves it for the pg_dump script to do;
and pg_dump doesn't realize that it would need to do ALTER EXTENSION ADD
for the families as well as the classes.

It's hard to solve this locally in pg_dump's opclass handling, because
you can't very easily tell whether a CREATE OPERATOR CLASS command will
result in creation of an opfamily or not. (It has to not have a FAMILY
clause, *and* the opfamily has to not already exist.) I'm inclined to
think that the best fix for this is to stop leaving operator families
implicit in the dump, but create them explicitly (at which point the
extension membership would get added correctly). The comments in
dumpOpfamily say

* We want to dump the opfamily only if (1) it contains "loose" operators
* or functions, or (2) it contains an opclass with a different name or
* owner. Otherwise it's sufficient to let it be created during creation
* of the contained opclass, and not dumping it improves portability of
* the dump.

The "portability" consideration in question is whether the dump file
can be fed to a pre-8.3 server without changes. I think we could blow
that off now; there are almost certainly other things that would have
to be tweaked to load a modern dump file into such old versions.

So my proposal for fixing this is to remove the logic in pg_dump that
tries to suppress explicit dumps of operator families. This would need
to be back-patched to 9.1, because it's certainly broken like this in
all extension-supporting versions. (Surprising that nobody's noticed.)

Objections, better ideas?

regards, tom lane