From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: Bogus tags for comments, ACLs, and security labels in pg_dump
In-reply-to: <21714.1516553459@sss.pgh.pa.us>
References: <21714.1516553459@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sun, 21 Jan 2018 11:50:59 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <25501.1516577416.0@sss.pgh.pa.us>
Date: Sun, 21 Jan 2018 18:31:21 -0500
Message-ID: <25578.1516577481@sss.pgh.pa.us>
Precedence: bulk

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

I wrote:
> What I think we should do is fix pg_dump so that these classes of
> TOC entries do indeed have tags that meet the expectation held by
> _tocEntryRequired, as per the first attached patch.

Some further poking around identified another place that was taking
dubious shortcuts to identify large-object TOC entries, further down
in _tocEntryRequired.  Since that code is executed only during
binary upgrade, which will be working from a schema-only dump anyway,
it *might* be harmless.  But I'm not sure, and it's certainly far from
future-proof.  I also noted a copy-and-pasteo in dumpSecLabel.

Hence, I intend to add the attached to the back-patched fix proposed
previously.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="more-large-object-fixes.patch";
	charset="us-ascii"
Content-ID: <25501.1516577416.2@sss.pgh.pa.us>
Content-Description: more-large-object-fixes.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_bac=
kup_archiver.c
index 41741ae..acef20f 100644
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** _tocEntryRequired(TocEntry *te, teSectio
*** 2944,2957 ****
  	if (ropt->schemaOnly)
  	{
  		/*
  		 * In binary-upgrade mode, even with schema-only set, we do not mask
  		 * out large objects.  Only large object definitions, comments and
  		 * other information should be generated in binary-upgrade mode (not
  		 * the actual data).
  		 */
  		if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") =3D=3D 0=
) &&
! 			!(ropt->binary_upgrade && strcmp(te->desc, "BLOB") =3D=3D 0) &&
! 			!(ropt->binary_upgrade && strncmp(te->tag, "LARGE OBJECT ", 13) =3D=3D=
 0))
  			res =3D res & REQ_SCHEMA;
  	}
  =

--- 2944,2965 ----
  	if (ropt->schemaOnly)
  	{
  		/*
+ 		 * The sequence_data option overrides schema-only for SEQUENCE SET.
+ 		 *
  		 * In binary-upgrade mode, even with schema-only set, we do not mask
  		 * out large objects.  Only large object definitions, comments and
  		 * other information should be generated in binary-upgrade mode (not
  		 * the actual data).
  		 */
  		if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") =3D=3D 0=
) &&
! 			!(ropt->binary_upgrade &&
! 			  (strcmp(te->desc, "BLOB") =3D=3D 0 ||
! 			   (strcmp(te->desc, "ACL") =3D=3D 0 &&
! 				strncmp(te->tag, "LARGE OBJECT ", 13) =3D=3D 0) ||
! 			   (strcmp(te->desc, "COMMENT") =3D=3D 0 &&
! 				strncmp(te->tag, "LARGE OBJECT ", 13) =3D=3D 0) ||
! 			   (strcmp(te->desc, "SECURITY LABEL") =3D=3D 0 &&
! 				strncmp(te->tag, "LARGE OBJECT ", 13) =3D=3D 0))))
  			res =3D res & REQ_SCHEMA;
  	}
  =

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 0bdd398..8e5e2d5 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** dumpSecLabel(Archive *fout, const char *
*** 14811,14817 ****
  	if (dopt->no_security_labels)
  		return;
  =

! 	/* Comments are schema not data ... except blob comments are data */
  	if (strncmp(target, "LARGE OBJECT ", 13) !=3D 0)
  	{
  		if (dopt->dataOnly)
--- 14811,14817 ----
  	if (dopt->no_security_labels)
  		return;
  =

! 	/* Security labels are schema not data ... except blob labels are data =
*/
  	if (strncmp(target, "LARGE OBJECT ", 13) !=3D 0)
  	{
  		if (dopt->dataOnly)

------- =_aaaaaaaaaa0--