From: Tom Lane <tgl@sss.pgh.pa.us>
To: tom@intevation.de
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15934: pg_dump output in wrong order if custom operator
 class is used as subtype_opclass in a range type
In-reply-to: <5345.1564589231@sss.pgh.pa.us>
References: <15934-58b8c8ab7a09ea15@postgresql.org>
 <5345.1564589231@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 31 Jul 2019 12:07:11 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <29011.1564597505.0@sss.pgh.pa.us>
Date: Wed, 31 Jul 2019 14:26:36 -0400
Message-ID: <29072.1564597596@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <29011.1564597505.1@sss.pgh.pa.us>

I wrote:
> Clearly, we gotta upgrade pg_dump's intelligence about handling this
> dependency structure.  The most straightforward fix would involve
> promoting pg_amop (and pg_amproc) entries to be full DumpableObjects,
> but I'm apprehensive about the overhead that would add ...

After further thought, I realized we could fix it far less invasively
than that, by teaching getDependencies() to translate pg_depend entries
for the pg_amop/amproc rows to look like dependencies for their parent
opfamily.  This looks messy, but in a typical database without any
custom opclasses, it's actually really cheap, because there won't be
any pg_depend entries satisfying the classid conditions.

I wasn't looking forward to back-patching the other idea, but this
patch seems like it should be pretty painless.

Again, thanks for the report!

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="fix-dump-dependencies-for-custom-opclasses.patch";
	charset="us-ascii"
Content-ID: <29011.1564597505.2@sss.pgh.pa.us>
Content-Description: fix-dump-dependencies-for-custom-opclasses.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8a31672..0cc9ede 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -17911,14 +17911,45 @@ getDependencies(Archive *fout)
 	query =3D createPQExpBuffer();
 =

 	/*
+	 * Messy query to collect the dependency data we need.  Note that we
+	 * ignore the sub-object column, so that dependencies of or on a column
+	 * look the same as dependencies of or on a whole table.
+	 *
 	 * PIN dependencies aren't interesting, and EXTENSION dependencies were
 	 * already processed by getExtensionMembership.
 	 */
 	appendPQExpBufferStr(query, "SELECT "
 						 "classid, objid, refclassid, refobjid, deptype "
 						 "FROM pg_depend "
-						 "WHERE deptype !=3D 'p' AND deptype !=3D 'e' "
-						 "ORDER BY 1,2");
+						 "WHERE deptype !=3D 'p' AND deptype !=3D 'e'\n");
+
+	/*
+	 * Since we don't treat pg_amop entries as separate DumpableObjects, we
+	 * have to translate their dependencies into dependencies of their paren=
t
+	 * opfamily.  Ignore internal dependencies though, as those will point t=
o
+	 * their parent opclass, which we needn't consider here (and if we did,
+	 * it'd just result in circular dependencies).  Also, "loose" opfamily
+	 * entries will have dependencies on their parent opfamily, which we
+	 * should drop since they'd likewise become useless self-dependencies.
+	 * (But be sure to keep deps on *other* opfamilies; see amopsortfamily.)
+	 */
+	appendPQExpBufferStr(query, "UNION ALL\n"
+						 "SELECT 'pg_opfamily'::regclass AS classid, amopfamily AS objid, r=
efclassid, refobjid, deptype "
+						 "FROM pg_depend d, pg_amop o "
+						 "WHERE deptype NOT IN ('p', 'e', 'i') AND "
+						 "classid =3D 'pg_amop'::regclass AND objid =3D o.oid "
+						 "AND NOT (refclassid =3D 'pg_opfamily'::regclass AND amopfamily =3D=
 refobjid)\n");
+
+	/* Likewise for pg_amproc entries */
+	appendPQExpBufferStr(query, "UNION ALL\n"
+						 "SELECT 'pg_opfamily'::regclass AS classid, amprocfamily AS objid,=
 refclassid, refobjid, deptype "
+						 "FROM pg_depend d, pg_amproc p "
+						 "WHERE deptype NOT IN ('p', 'e', 'i') AND "
+						 "classid =3D 'pg_amproc'::regclass AND objid =3D p.oid "
+						 "AND NOT (refclassid =3D 'pg_opfamily'::regclass AND amprocfamily =
=3D refobjid)\n");
+
+	/* Sort the output for efficiency below */
+	appendPQExpBufferStr(query, "ORDER BY 1,2");
 =

 	res =3D ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
 =


------- =_aaaaaaaaaa0--