Skip to content

Commit ff992c0

Browse files
sfrostsfrost
committed
pg_upgrade: Fix large object COMMENTS, SECURITY LABELS
When performing a pg_upgrade, we copy the files behind pg_largeobject and pg_largeobject_metadata, allowing us to avoid having to dump out and reload the actual data for large objects and their ACLs. Unfortunately, that isn't all of the information which can be associated with large objects. Currently, we also support COMMENTs and SECURITY LABELs with large objects and these were being silently dropped during a pg_upgrade as pg_dump would skip everything having to do with a large object and pg_upgrade only copied the tables mentioned to the new cluster. As the file copies happen after the catalog dump and reload, we can't simply include the COMMENTs and SECURITY LABELs in pg_dump's binary-mode output but we also have to include the actual large object definition as well. With the definition, comments, and security labels in the pg_dump output and the file copies performed by pg_upgrade, all of the data and metadata associated with large objects is able to be successfully pulled forward across a pg_upgrade. In 9.6 and master, we can simply adjust the dump bitmask to indicate which components we don't want. In 9.5 and earlier, we have to put explciit checks in in dumpBlob() and dumpBlobs() to not include the ACL or the data when in binary-upgrade mode. Adjustments made to the privileges regression test to allow another test (large_object.sql) to be added which explicitly leaves a large object with a comment in place to provide coverage of that case with pg_upgrade. Back-patch to all supported branches. Discussion: https://postgr.es/m/20170221162655.GE9812@tamriel.snowman.net
1 parent a8df75b commit ff992c0

10 files changed

+80
-16
lines changed

‎src/bin/pg_dump/pg_backup.h

+1
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ typedef struct _restoreOptions
120120
int enable_row_security;
121121
int sequence_data; /* dump sequence data even in schema-only mode */
122122
int include_subscriptions;
123+
int binary_upgrade;
123124
} RestoreOptions;
124125

125126
typedef struct _dumpOptions

‎src/bin/pg_dump/pg_backup_archiver.c

+9-1
Original file line numberDiff line numberDiff line change
@@ -2874,7 +2874,15 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
28742874
/* Mask it if we only want schema */
28752875
if (ropt->schemaOnly)
28762876
{
2877-
if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") == 0))
2877+
/*
2878+
* In binary-upgrade mode, even with schema-only set, we do not mask
2879+
* out large objects. Only large object definitions, comments and
2880+
* other information should be generated in binary-upgrade mode (not
2881+
* the actual data).
2882+
*/
2883+
if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") == 0) &&
2884+
!(ropt->binary_upgrade && strcmp(te->desc, "BLOB") == 0) &&
2885+
!(ropt->binary_upgrade && strncmp(te->tag, "LARGE OBJECT ", 13) == 0))
28782886
res = res & REQ_SCHEMA;
28792887
}
28802888

‎src/bin/pg_dump/pg_dump.c

+28-3
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,15 @@ main(int argc, char **argv)
772772
if (dopt.schemaOnly && dopt.sequence_data)
773773
getTableData(&dopt, tblinfo, numTables, dopt.oids, RELKIND_SEQUENCE);
774774

775-
if (dopt.outputBlobs)
775+
/*
776+
* In binary-upgrade mode, we do not have to worry about the actual blob
777+
* data or the associated metadata that resides in the pg_largeobject and
778+
* pg_largeobject_metadata tables, respectivly.
779+
*
780+
* However, we do need to collect blob information as there may be
781+
* comments or other information on blobs that we do need to dump out.
782+
*/
783+
if (dopt.outputBlobs || dopt.binary_upgrade)
776784
getBlobs(fout);
777785

778786
/*
@@ -852,6 +860,7 @@ main(int argc, char **argv)
852860
ropt->enable_row_security = dopt.enable_row_security;
853861
ropt->sequence_data = dopt.sequence_data;
854862
ropt->include_subscriptions = dopt.include_subscriptions;
863+
ropt->binary_upgrade = dopt.binary_upgrade;
855864

856865
if (compressLevel == -1)
857866
ropt->compression = 0;
@@ -2900,6 +2909,20 @@ getBlobs(Archive *fout)
29002909
PQgetisnull(res, i, i_initlomacl) &&
29012910
PQgetisnull(res, i, i_initrlomacl))
29022911
binfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL;
2912+
2913+
/*
2914+
* In binary-upgrade mode for blobs, we do *not* dump out the data or
2915+
* the ACLs, should any exist. The data and ACL (if any) will be
2916+
* copied by pg_upgrade, which simply copies the pg_largeobject and
2917+
* pg_largeobject_metadata tables.
2918+
*
2919+
* We *do* dump out the definition of the blob because we need that to
2920+
* make the restoration of the comments, and anything else, work since
2921+
* pg_upgrade copies the files behind pg_largeobject and
2922+
* pg_largeobject_metadata after the dump is restored.
2923+
*/
2924+
if (dopt->binary_upgrade)
2925+
binfo[i].dobj.dump &= ~(DUMP_COMPONENT_DATA | DUMP_COMPONENT_ACL);
29032926
}
29042927

29052928
/*
@@ -8828,7 +8851,8 @@ dumpComment(Archive *fout, const char *target,
88288851
}
88298852
else
88308853
{
8831-
if (dopt->schemaOnly)
8854+
/* We do dump blob comments in binary-upgrade mode */
8855+
if (dopt->schemaOnly && !dopt->binary_upgrade)
88328856
return;
88338857
}
88348858

@@ -14223,7 +14247,8 @@ dumpSecLabel(Archive *fout, const char *target,
1422314247
}
1422414248
else
1422514249
{
14226-
if (dopt->schemaOnly)
14250+
/* We do dump blob security labels in binary-upgrade mode */
14251+
if (dopt->schemaOnly && !dopt->binary_upgrade)
1422714252
return;
1422814253
}
1422914254

‎src/bin/pg_dump/t/002_pg_dump.pl

+10-4
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,17 @@
3939
binary_upgrade => {
4040
dump_cmd => [
4141
'pg_dump',
42-
"--file=$tempdir/binary_upgrade.sql",
42+
'--format=custom',
43+
"--file=$tempdir/binary_upgrade.dump",
4344
'--schema-only',
4445
'--binary-upgrade',
4546
'-d', 'postgres', # alternative way to specify database
46-
], },
47+
],
48+
restore_cmd => [
49+
'pg_restore', '-Fc',
50+
'--verbose',
51+
"--file=$tempdir/binary_upgrade.sql",
52+
"$tempdir/binary_upgrade.dump", ], },
4753
clean => {
4854
dump_cmd => [
4955
'pg_dump',
@@ -334,6 +340,7 @@
334340
all_runs => 1,
335341
regexp => qr/^ALTER LARGE OBJECT \d+ OWNER TO .*;/m,
336342
like => {
343+
binary_upgrade => 1,
337344
clean => 1,
338345
clean_if_exists => 1,
339346
column_inserts => 1,
@@ -348,7 +355,6 @@
348355
section_pre_data => 1,
349356
test_schema_plus_blobs => 1, },
350357
unlike => {
351-
binary_upgrade => 1,
352358
no_blobs => 1,
353359
no_owner => 1,
354360
only_dump_test_schema => 1,
@@ -666,6 +672,7 @@
666672
'SELECT pg_catalog.lo_from_bytea(0, \'\\x310a320a330a340a350a360a370a380a390a\');',
667673
regexp => qr/^SELECT pg_catalog\.lo_create\('\d+'\);/m,
668674
like => {
675+
binary_upgrade => 1,
669676
clean => 1,
670677
clean_if_exists => 1,
671678
column_inserts => 1,
@@ -681,7 +688,6 @@
681688
section_pre_data => 1,
682689
test_schema_plus_blobs => 1, },
683690
unlike => {
684-
binary_upgrade => 1,
685691
no_blobs => 1,
686692
only_dump_test_schema => 1,
687693
only_dump_test_table => 1,

‎src/test/regress/expected/large_object.out

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
-- This is more-or-less DROP IF EXISTS LARGE OBJECT 3001;
2+
WITH unlink AS (SELECT lo_unlink(loid) FROM pg_largeobject WHERE loid = 3001) SELECT 1;
3+
?column?
4+
----------
5+
1
6+
(1 row)
7+
8+
-- Test creation of a large object and leave it for testing pg_upgrade
9+
SELECT lo_create(3001);
10+
lo_create
11+
-----------
12+
3001
13+
(1 row)
14+
15+
COMMENT ON LARGE OBJECT 3001 IS 'testing comments';

‎src/test/regress/expected/privileges.out

+4-4
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ DROP ROLE IF EXISTS regress_user3;
1212
DROP ROLE IF EXISTS regress_user4;
1313
DROP ROLE IF EXISTS regress_user5;
1414
DROP ROLE IF EXISTS regress_user6;
15-
SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
15+
SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
1616
lo_unlink
1717
-----------
1818
(0 rows)
@@ -1173,11 +1173,11 @@ SELECT lo_unlink(2002);
11731173

11741174
\c -
11751175
-- confirm ACL setting
1176-
SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata;
1176+
SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
11771177
oid | ownername | lomacl
11781178
------+---------------+------------------------------------------------------------------------------------------------
1179-
1002 | regress_user1 |
11801179
1001 | regress_user1 | {regress_user1=rw/regress_user1,=rw/regress_user1}
1180+
1002 | regress_user1 |
11811181
1003 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=r/regress_user1}
11821182
1004 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=rw/regress_user1}
11831183
1005 | regress_user1 | {regress_user1=rw/regress_user1,regress_user2=r*w/regress_user1,regress_user3=r/regress_user2}
@@ -1546,7 +1546,7 @@ DROP TABLE atest6;
15461546
DROP TABLE atestc;
15471547
DROP TABLE atestp1;
15481548
DROP TABLE atestp2;
1549-
SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
1549+
SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
15501550
lo_unlink
15511551
-----------
15521552
1

‎src/test/regress/parallel_schedule

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ test: select_into select_distinct select_distinct_on select_implicit select_havi
8484
# ----------
8585
# Another group of parallel tests
8686
# ----------
87-
test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator
87+
test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator large_object
8888

8989
# ----------
9090
# Another group of parallel tests

‎src/test/regress/serial_schedule

+1
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ test: object_address
116116
test: tablesample
117117
test: groupingsets
118118
test: drop_operator
119+
test: large_object
119120
test: alter_generic
120121
test: alter_operator
121122
test: misc

‎src/test/regress/sql/large_object.sql

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
2+
-- This is more-or-less DROP IF EXISTS LARGE OBJECT 3001;
3+
WITH unlink AS (SELECT lo_unlink(loid) FROM pg_largeobject WHERE loid = 3001) SELECT 1;
4+
5+
-- Test creation of a large object and leave it for testing pg_upgrade
6+
SELECT lo_create(3001);
7+
8+
COMMENT ON LARGE OBJECT 3001 IS 'testing comments';

‎src/test/regress/sql/privileges.sql

+3-3
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ DROP ROLE IF EXISTS regress_user4;
1717
DROP ROLE IF EXISTS regress_user5;
1818
DROP ROLE IF EXISTS regress_user6;
1919

20-
SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
20+
SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
2121

2222
RESET client_min_messages;
2323

@@ -729,7 +729,7 @@ SELECT lo_unlink(2002);
729729

730730
\c -
731731
-- confirm ACL setting
732-
SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata;
732+
SELECT oid, pg_get_userbyid(lomowner) ownername, lomacl FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
733733

734734
SET SESSION AUTHORIZATION regress_user3;
735735

@@ -960,7 +960,7 @@ DROP TABLE atestc;
960960
DROP TABLE atestp1;
961961
DROP TABLE atestp2;
962962

963-
SELECT lo_unlink(oid) FROM pg_largeobject_metadata;
963+
SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid;
964964

965965
DROP GROUP regress_group1;
966966
DROP GROUP regress_group2;

0 commit comments

Comments
 (0)