All,

* Stephen Frost (sfr...@snowman.net) wrote:
> Just wanted to get a note out to -hackers about the issue, I'll see
> about getting a fix written up for it soon.

Attached is a patch which addresses this issue.  I'm not terribly
pleased with it, but I also haven't got any great ideas of what else to
do.  Suggestions welcome, of course.

Otherwise, I'll plan to start working on the back-branch changes for
this soon.

Thanks!

Stephen
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index 6480fb8..983a999
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*************** typedef struct _restoreOptions
*** 120,125 ****
--- 120,126 ----
  	int			enable_row_security;
  	int			sequence_data;	/* dump sequence data even in schema-only mode */
  	int			include_subscriptions;
+ 	int			binary_upgrade;
  } RestoreOptions;
  
  typedef struct _dumpOptions
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 929f1b5..6c340f9
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** _tocEntryRequired(TocEntry *te, teSectio
*** 2876,2882 ****
  	/* Mask it if we only want schema */
  	if (ropt->schemaOnly)
  	{
! 		if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") == 0))
  			res = res & REQ_SCHEMA;
  	}
  
--- 2876,2884 ----
  	/* Mask it if we only want schema */
  	if (ropt->schemaOnly)
  	{
! 		if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") == 0) &&
! 			!(ropt->binary_upgrade && strcmp(te->desc,"BLOB") == 0) &&
! 			!(ropt->binary_upgrade && strncmp(te->tag,"LARGE OBJECT ", 13) == 0))
  			res = res & REQ_SCHEMA;
  	}
  
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index 89db310..44d4f32
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** main(int argc, char **argv)
*** 775,781 ****
  	if (dopt.schemaOnly && dopt.sequence_data)
  		getTableData(&dopt, tblinfo, numTables, dopt.oids, RELKIND_SEQUENCE);
  
! 	if (dopt.outputBlobs)
  		getBlobs(fout);
  
  	/*
--- 775,789 ----
  	if (dopt.schemaOnly && dopt.sequence_data)
  		getTableData(&dopt, tblinfo, numTables, dopt.oids, RELKIND_SEQUENCE);
  
! 	/*
! 	 * In binary-upgrade mode, we do not have to worry about the actual blob
! 	 * data or the associated metadata that resides in the pg_largeobject and
! 	 * pg_largeobject_metadata tables, respectivly.
! 	 *
! 	 * However, we do need to collect blob information as there may be comments
! 	 * or security labels on blobs and we need to dump those out.
! 	 */
! 	if (dopt.outputBlobs || dopt.binary_upgrade)
  		getBlobs(fout);
  
  	/*
*************** main(int argc, char **argv)
*** 855,860 ****
--- 863,869 ----
  	ropt->enable_row_security = dopt.enable_row_security;
  	ropt->sequence_data = dopt.sequence_data;
  	ropt->include_subscriptions = dopt.include_subscriptions;
+ 	ropt->binary_upgrade = dopt.binary_upgrade;
  
  	if (compressLevel == -1)
  		ropt->compression = 0;
*************** getBlobs(Archive *fout)
*** 2903,2908 ****
--- 2912,2931 ----
  			PQgetisnull(res, i, i_initlomacl) &&
  			PQgetisnull(res, i, i_initrlomacl))
  			binfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL;
+ 
+ 		/*
+ 		 * In binary-upgrade mode for blobs, we do *not* dump out the data or
+ 		 * the ACLs, should any exist.  The data and ACL (if any) will be
+ 		 * copied by pg_upgrade, which simply copies the pg_largeobject and
+ 		 * pg_largeobject_metadata tables.
+ 		 *
+ 		 * We *do* dump out the definition of the blob because we need that
+ 		 * to make the restoration of the comments, and anything else, work
+ 		 * since pg_upgrade copies the files behind pg_largeobject and
+ 		 * pg_largeobject_metadata after the dump is restored.
+ 		 */
+ 		if (dopt->binary_upgrade)
+ 			binfo[i].dobj.dump &= ~(DUMP_COMPONENT_DATA | DUMP_COMPONENT_ACL);
  	}
  
  	/*
*************** dumpComment(Archive *fout, const char *t
*** 8831,8837 ****
  	}
  	else
  	{
! 		if (dopt->schemaOnly)
  			return;
  	}
  
--- 8854,8861 ----
  	}
  	else
  	{
! 		/* We do dump blob comments in binary-upgrade mode */
! 		if (dopt->schemaOnly && !dopt->binary_upgrade)
  			return;
  	}
  
*************** dumpSecLabel(Archive *fout, const char *
*** 14226,14232 ****
  	}
  	else
  	{
! 		if (dopt->schemaOnly)
  			return;
  	}
  
--- 14250,14257 ----
  	}
  	else
  	{
! 		/* We do dump blob security labels in binary-upgrade mode */
! 		if (dopt->schemaOnly && !dopt->binary_upgrade)
  			return;
  	}
  
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
new file mode 100644
index f73bf89..c51088a
*** a/src/bin/pg_dump/t/002_pg_dump.pl
--- b/src/bin/pg_dump/t/002_pg_dump.pl
*************** my %pgdump_runs = (
*** 39,49 ****
  	binary_upgrade => {
  		dump_cmd => [
  			'pg_dump',
! 			"--file=$tempdir/binary_upgrade.sql",
  			'--schema-only',
  			'--binary-upgrade',
  			'-d', 'postgres',    # alternative way to specify database
! 		], },
  	clean => {
  		dump_cmd => [
  			'pg_dump',
--- 39,55 ----
  	binary_upgrade => {
  		dump_cmd => [
  			'pg_dump',
! 			'--format=custom',
! 			"--file=$tempdir/binary_upgrade.dump",
  			'--schema-only',
  			'--binary-upgrade',
  			'-d', 'postgres',    # alternative way to specify database
! 		],
! 		restore_cmd => [
! 			'pg_restore', '-Fc',
! 			'--verbose',
! 			"--file=$tempdir/binary_upgrade.sql",
! 			"$tempdir/binary_upgrade.dump", ], },
  	clean => {
  		dump_cmd => [
  			'pg_dump',
*************** my %tests = (
*** 334,339 ****
--- 340,346 ----
  		all_runs => 1,
  		regexp   => qr/^ALTER LARGE OBJECT \d+ OWNER TO .*;/m,
  		like     => {
+ 			binary_upgrade           => 1,
  			clean                    => 1,
  			clean_if_exists          => 1,
  			column_inserts           => 1,
*************** my %tests = (
*** 348,354 ****
  			section_pre_data         => 1,
  			test_schema_plus_blobs   => 1, },
  		unlike => {
- 			binary_upgrade           => 1,
  			no_blobs                 => 1,
  			no_owner                 => 1,
  			only_dump_test_schema    => 1,
--- 355,360 ----
*************** my %tests = (
*** 666,671 ****
--- 672,678 ----
  'SELECT pg_catalog.lo_from_bytea(0, \'\\x310a320a330a340a350a360a370a380a390a\');',
  		regexp => qr/^SELECT pg_catalog\.lo_create\('\d+'\);/m,
  		like   => {
+ 			binary_upgrade           => 1,
  			clean                    => 1,
  			clean_if_exists          => 1,
  			column_inserts           => 1,
*************** my %tests = (
*** 681,687 ****
  			section_pre_data         => 1,
  			test_schema_plus_blobs   => 1, },
  		unlike => {
- 			binary_upgrade           => 1,
  			no_blobs                 => 1,
  			only_dump_test_schema    => 1,
  			only_dump_test_table     => 1,
--- 688,693 ----
diff --git a/src/test/regress/expected/large_object.out b/src/test/regress/expected/large_object.out
new file mode 100644
index ...b00d47c
*** a/src/test/regress/expected/large_object.out
--- b/src/test/regress/expected/large_object.out
***************
*** 0 ****
--- 1,15 ----
+ -- This is more-or-less DROP IF EXISTS LARGE OBJECT 3001;
+ WITH unlink AS (SELECT lo_unlink(loid) FROM pg_largeobject WHERE loid = 3001) SELECT 1;
+  ?column? 
+ ----------
+         1
+ (1 row)
+ 
+ -- Test creation of a large object and leave it for testing pg_upgrade
+ SELECT lo_create(3001);
+  lo_create 
+ -----------
+       3001
+ (1 row)
+ 
+ COMMENT ON LARGE OBJECT 3001 IS 'testing comments';
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
new file mode 100644
index f66b443..1af66ff
*** a/src/test/regress/expected/privileges.out
--- b/src/test/regress/expected/privileges.out
*************** DROP ROLE IF EXISTS regress_user3;
*** 12,18 ****
  DROP ROLE IF EXISTS regress_user4;
  DROP ROLE IF EXISTS regress_user5;
  DROP ROLE IF EXISTS regress_user6;
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
   lo_unlink 
  -----------
  (0 rows)
--- 12,18 ----
  DROP ROLE IF EXISTS regress_user4;
  DROP ROLE IF EXISTS regress_user5;
  DROP ROLE IF EXISTS regress_user6;
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000;
   lo_unlink 
  -----------
  (0 rows)
*************** SELECT lo_unlink(2002);
*** 1173,1183 ****
  
  \c -
  -- confirm ACL setting
! SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata;
   oid  |   ownername   |                                             lomacl                                             
  ------+---------------+------------------------------------------------------------------------------------------------
-  1002 | regress_user1 | 
   1001 | regress_user1 | {regress_user1=rw/regress_user1,=rw/regress_user1}
   1003 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=r/regress_user1}
   1004 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=rw/regress_user1}
   1005 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=r*w/regress_user1,regress_user3=r/regress_user2}
--- 1173,1183 ----
  
  \c -
  -- confirm ACL setting
! SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
   oid  |   ownername   |                                             lomacl                                             
  ------+---------------+------------------------------------------------------------------------------------------------
   1001 | regress_user1 | {regress_user1=rw/regress_user1,=rw/regress_user1}
+  1002 | regress_user1 | 
   1003 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=r/regress_user1}
   1004 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=rw/regress_user1}
   1005 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=r*w/regress_user1,regress_user3=r/regress_user2}
*************** DROP TABLE atest6;
*** 1546,1552 ****
  DROP TABLE atestc;
  DROP TABLE atestp1;
  DROP TABLE atestp2;
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
   lo_unlink 
  -----------
           1
--- 1546,1552 ----
  DROP TABLE atestc;
  DROP TABLE atestp1;
  DROP TABLE atestp2;
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000;
   lo_unlink 
  -----------
           1
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
new file mode 100644
index edeb2d6..1f2fb59
*** a/src/test/regress/parallel_schedule
--- b/src/test/regress/parallel_schedule
*************** test: select_into select_distinct select
*** 84,90 ****
  # ----------
  # Another group of parallel tests
  # ----------
! test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator
  
  # ----------
  # Another group of parallel tests
--- 84,90 ----
  # ----------
  # Another group of parallel tests
  # ----------
! test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator large_object
  
  # ----------
  # Another group of parallel tests
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
new file mode 100644
index 27a46d7..9ffceff
*** a/src/test/regress/serial_schedule
--- b/src/test/regress/serial_schedule
*************** test: object_address
*** 116,121 ****
--- 116,122 ----
  test: tablesample
  test: groupingsets
  test: drop_operator
+ test: large_object
  test: alter_generic
  test: alter_operator
  test: misc
diff --git a/src/test/regress/sql/large_object.sql b/src/test/regress/sql/large_object.sql
new file mode 100644
index ...a9e18b7
*** a/src/test/regress/sql/large_object.sql
--- b/src/test/regress/sql/large_object.sql
***************
*** 0 ****
--- 1,8 ----
+ 
+ -- This is more-or-less DROP IF EXISTS LARGE OBJECT 3001;
+ WITH unlink AS (SELECT lo_unlink(loid) FROM pg_largeobject WHERE loid = 3001) SELECT 1;
+ 
+ -- Test creation of a large object and leave it for testing pg_upgrade
+ SELECT lo_create(3001);
+ 
+ COMMENT ON LARGE OBJECT 3001 IS 'testing comments';
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
new file mode 100644
index 00dc7bd..82d6751
*** a/src/test/regress/sql/privileges.sql
--- b/src/test/regress/sql/privileges.sql
*************** DROP ROLE IF EXISTS regress_user4;
*** 17,23 ****
  DROP ROLE IF EXISTS regress_user5;
  DROP ROLE IF EXISTS regress_user6;
  
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
  
  RESET client_min_messages;
  
--- 17,23 ----
  DROP ROLE IF EXISTS regress_user5;
  DROP ROLE IF EXISTS regress_user6;
  
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000;
  
  RESET client_min_messages;
  
*************** SELECT lo_unlink(2002);
*** 729,735 ****
  
  \c -
  -- confirm ACL setting
! SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata;
  
  SET SESSION AUTHORIZATION regress_user3;
  
--- 729,735 ----
  
  \c -
  -- confirm ACL setting
! SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
  
  SET SESSION AUTHORIZATION regress_user3;
  
*************** DROP TABLE atestc;
*** 960,966 ****
  DROP TABLE atestp1;
  DROP TABLE atestp2;
  
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
  
  DROP GROUP regress_group1;
  DROP GROUP regress_group2;
--- 960,966 ----
  DROP TABLE atestp1;
  DROP TABLE atestp2;
  
! SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000;
  
  DROP GROUP regress_group1;
  DROP GROUP regress_group2;

Attachment: signature.asc
Description: Digital signature

Reply via email to