Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2cb1272

Browse files
committed
Rethink method for assigning OIDs to the template0 and postgres DBs.
Commit aa01051 assigned fixed OIDs to template0 and postgres in a very ad-hoc way. Notably, instead of teaching Catalog.pm about these OIDs, the unused_oids script was just hacked to not show them as unused. That's problematic since, for example, duplicate_oids wouldn't report any future conflict. Hence, invent a macro DECLARE_OID_DEFINING_MACRO() that can be used to define an OID that is known to Catalog.pm and will participate in duplicate-detection as well as renumbering by renumber_oids.pl. (We don't anticipate renumbering these particular OIDs, but we might as well build out all the Catalog.pm infrastructure while we're here.) Another issue is that aa01051 neglected to touch IsPinnedObject, with the result that it now claimed template0 and postgres are pinned. The right thing to do there seems to be to teach it that no database is pinned, since in fact DROP DATABASE doesn't check for pinned-ness (and at least for these cases, that is an intentional choice). It's not clear whether this wrong answer had any visible effect, but perhaps it could have resulted in erroneous management of dependency entries. In passing, rename the TemplateDbOid macro to Template1DbOid to reduce confusion (likely we should have done that way back when we invented template0, but we didn't), and rename the OID macros for template0 and postgres to have a similar style. There are no changes to postgres.bki here, so no need for a catversion bump. Discussion: https://postgr.es/m/2935358.1650479692@sss.pgh.pa.us
1 parent 586955d commit 2cb1272

File tree

14 files changed

+75
-34
lines changed

14 files changed

+75
-34
lines changed

doc/src/sgml/bki.sgml

+4-3
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,13 @@
180180
[
181181

182182
# A comment could appear here.
183-
{ oid => '1', oid_symbol => 'TemplateDbOid',
183+
{ oid => '1', oid_symbol => 'Template1DbOid',
184184
descr => 'database\'s default template',
185-
datname => 'template1', encoding => 'ENCODING', datistemplate => 't',
185+
datname => 'template1', encoding => 'ENCODING',
186+
datlocprovider => 'LOCALE_PROVIDER', datistemplate => 't',
186187
datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0',
187188
datminmxid => '1', dattablespace => 'pg_default', datcollate => 'LC_COLLATE',
188-
datctype => 'LC_CTYPE', datacl => '_null_' },
189+
datctype => 'LC_CTYPE', daticulocale => 'ICU_LOCALE', datacl => '_null_' },
189190

190191
]
191192
]]></programlisting>

src/backend/access/transam/xlog.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -4540,9 +4540,9 @@ BootStrapXLOG(void)
45404540
checkPoint.nextMulti = FirstMultiXactId;
45414541
checkPoint.nextMultiOffset = 0;
45424542
checkPoint.oldestXid = FirstNormalTransactionId;
4543-
checkPoint.oldestXidDB = TemplateDbOid;
4543+
checkPoint.oldestXidDB = Template1DbOid;
45444544
checkPoint.oldestMulti = FirstMultiXactId;
4545-
checkPoint.oldestMultiDB = TemplateDbOid;
4545+
checkPoint.oldestMultiDB = Template1DbOid;
45464546
checkPoint.oldestCommitTsXid = InvalidTransactionId;
45474547
checkPoint.newestCommitTsXid = InvalidTransactionId;
45484548
checkPoint.time = (pg_time_t) time(NULL);

src/backend/catalog/Catalog.pm

+14
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ sub ParseHeader
4444
$catalog{columns} = [];
4545
$catalog{toasting} = [];
4646
$catalog{indexing} = [];
47+
$catalog{other_oids} = [];
48+
$catalog{foreign_keys} = [];
4749
$catalog{client_code} = [];
4850

4951
open(my $ifh, '<', $input_file) || die "$input_file: $!";
@@ -118,6 +120,14 @@ sub ParseHeader
118120
index_decl => $6
119121
};
120122
}
123+
elsif (/^DECLARE_OID_DEFINING_MACRO\(\s*(\w+),\s*(\d+)\)/)
124+
{
125+
push @{ $catalog{other_oids} },
126+
{
127+
other_name => $1,
128+
other_oid => $2
129+
};
130+
}
121131
elsif (
122132
/^DECLARE_(ARRAY_)?FOREIGN_KEY(_OPT)?\(\s*\(([^)]+)\),\s*(\w+),\s*\(([^)]+)\)\)/
123133
)
@@ -572,6 +582,10 @@ sub FindAllOidsFromHeaders
572582
{
573583
push @oids, $index->{index_oid};
574584
}
585+
foreach my $other (@{ $catalog->{other_oids} })
586+
{
587+
push @oids, $other->{other_oid};
588+
}
575589
}
576590

577591
return \@oids;

src/backend/catalog/catalog.c

+9-5
Original file line numberDiff line numberDiff line change
@@ -339,16 +339,20 @@ IsPinnedObject(Oid classId, Oid objectId)
339339
* robustness.
340340
*/
341341

342-
/* template1 is not pinned */
343-
if (classId == DatabaseRelationId &&
344-
objectId == TemplateDbOid)
345-
return false;
346-
347342
/* the public namespace is not pinned */
348343
if (classId == NamespaceRelationId &&
349344
objectId == PG_PUBLIC_NAMESPACE)
350345
return false;
351346

347+
/*
348+
* Databases are never pinned. It might seem that it'd be prudent to pin
349+
* at least template0; but we do this intentionally so that template0 and
350+
* template1 can be rebuilt from each other, thus letting them serve as
351+
* mutual backups (as long as you've not modified template1, anyway).
352+
*/
353+
if (classId == DatabaseRelationId)
354+
return false;
355+
352356
/*
353357
* All other initdb-created objects are pinned. This is overkill (the
354358
* system doesn't really depend on having every last weird datatype, for

src/backend/catalog/genbki.pl

+7-1
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@
472472
$catalog->{rowtype_oid_macro}, $catalog->{rowtype_oid}
473473
if $catalog->{rowtype_oid_macro};
474474
475-
# Likewise for macros for toast and index OIDs
475+
# Likewise for macros for toast, index, and other OIDs
476476
foreach my $toast (@{ $catalog->{toasting} })
477477
{
478478
printf $def "#define %s %s\n",
@@ -488,6 +488,12 @@
488488
$index->{index_oid_macro}, $index->{index_oid}
489489
if $index->{index_oid_macro};
490490
}
491+
foreach my $other (@{ $catalog->{other_oids} })
492+
{
493+
printf $def "#define %s %s\n",
494+
$other->{other_name}, $other->{other_oid}
495+
if $other->{other_name};
496+
}
491497
492498
print $def "\n";
493499

src/backend/utils/init/postinit.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
908908
*/
909909
if (bootstrap)
910910
{
911-
MyDatabaseId = TemplateDbOid;
911+
MyDatabaseId = Template1DbOid;
912912
MyDatabaseTableSpace = DEFAULTTABLESPACE_OID;
913913
}
914914
else if (in_dbname != NULL)

src/bin/initdb/initdb.c

+5-4
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@
5959
#include "sys/mman.h"
6060
#endif
6161

62-
#include "access/transam.h"
6362
#include "access/xlog_internal.h"
6463
#include "catalog/pg_authid_d.h"
6564
#include "catalog/pg_class_d.h" /* pgrminclude ignore */
6665
#include "catalog/pg_collation_d.h"
66+
#include "catalog/pg_database_d.h" /* pgrminclude ignore */
6767
#include "common/file_perm.h"
6868
#include "common/file_utils.h"
6969
#include "common/logging.h"
@@ -1812,8 +1812,8 @@ make_template0(FILE *cmdfd)
18121812
* be a little bit slower and make the new cluster a little bit bigger.
18131813
*/
18141814
static const char *const template0_setup[] = {
1815-
"CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false OID = "
1816-
CppAsString2(Template0ObjectId)
1815+
"CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false"
1816+
" OID = " CppAsString2(Template0DbOid)
18171817
" STRATEGY = file_copy;\n\n",
18181818

18191819
/*
@@ -1862,7 +1862,8 @@ make_postgres(FILE *cmdfd)
18621862
* OID to postgres and select the file_copy strategy.
18631863
*/
18641864
static const char *const postgres_setup[] = {
1865-
"CREATE DATABASE postgres OID = " CppAsString2(PostgresObjectId) " STRATEGY = file_copy;\n\n",
1865+
"CREATE DATABASE postgres OID = " CppAsString2(PostgresDbOid)
1866+
" STRATEGY = file_copy;\n\n",
18661867
"COMMENT ON DATABASE postgres IS 'default administrative connection database';\n\n",
18671868
NULL
18681869
};

src/bin/pg_dump/pg_dump.c

+5-4
Original file line numberDiff line numberDiff line change
@@ -2901,10 +2901,11 @@ dumpDatabase(Archive *fout)
29012901
qdatname = pg_strdup(fmtId(datname));
29022902

29032903
/*
2904-
* Prepare the CREATE DATABASE command. We must specify encoding, locale,
2905-
* and tablespace since those can't be altered later. Other DB properties
2906-
* are left to the DATABASE PROPERTIES entry, so that they can be applied
2907-
* after reconnecting to the target DB.
2904+
* Prepare the CREATE DATABASE command. We must specify OID (if we want
2905+
* to preserve that), as well as the encoding, locale, and tablespace
2906+
* since those can't be altered later. Other DB properties are left to
2907+
* the DATABASE PROPERTIES entry, so that they can be applied after
2908+
* reconnecting to the target DB.
29082909
*/
29092910
if (dopt->binary_upgrade)
29102911
{

src/include/access/transam.h

-4
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,6 @@ FullTransactionIdAdvance(FullTransactionId *dest)
196196
#define FirstUnpinnedObjectId 12000
197197
#define FirstNormalObjectId 16384
198198

199-
/* OIDs of Template0 and Postgres database are fixed */
200-
#define Template0ObjectId 4
201-
#define PostgresObjectId 5
202-
203199
/*
204200
* VariableCache is a data structure in shared memory that is used to track
205201
* OID and XID assignment state. For largely historical reasons, there is

src/include/catalog/genbki.h

+8
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,14 @@
8484
#define DECLARE_UNIQUE_INDEX(name,oid,oidmacro,decl) extern int no_such_variable
8585
#define DECLARE_UNIQUE_INDEX_PKEY(name,oid,oidmacro,decl) extern int no_such_variable
8686

87+
/*
88+
* These lines inform genbki.pl about manually-assigned OIDs that do not
89+
* correspond to any entry in the catalog *.dat files, but should be subject
90+
* to uniqueness verification and renumber_oids.pl renumbering. A C macro
91+
* to #define the given name is emitted into the corresponding *_d.h file.
92+
*/
93+
#define DECLARE_OID_DEFINING_MACRO(name,oid) extern int no_such_variable
94+
8795
/*
8896
* These lines are processed by genbki.pl to create a table for use
8997
* by the pg_get_catalog_foreign_keys() function. We do not have any

src/include/catalog/pg_database.dat

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
[
1414

15-
{ oid => '1', oid_symbol => 'TemplateDbOid',
15+
{ oid => '1', oid_symbol => 'Template1DbOid',
1616
descr => 'default template for new databases',
1717
datname => 'template1', encoding => 'ENCODING', datlocprovider => 'LOCALE_PROVIDER', datistemplate => 't',
1818
datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0',

src/include/catalog/pg_database.h

+9
Original file line numberDiff line numberDiff line change
@@ -91,4 +91,13 @@ DECLARE_TOAST_WITH_MACRO(pg_database, 4177, 4178, PgDatabaseToastTable, PgDataba
9191
DECLARE_UNIQUE_INDEX(pg_database_datname_index, 2671, DatabaseNameIndexId, on pg_database using btree(datname name_ops));
9292
DECLARE_UNIQUE_INDEX_PKEY(pg_database_oid_index, 2672, DatabaseOidIndexId, on pg_database using btree(oid oid_ops));
9393

94+
/*
95+
* pg_database.dat contains an entry for template1, but not for the template0
96+
* or postgres databases, because those are created later in initdb.
97+
* However, we still want to manually assign the OIDs for template0 and
98+
* postgres, so declare those here.
99+
*/
100+
DECLARE_OID_DEFINING_MACRO(Template0DbOid, 4);
101+
DECLARE_OID_DEFINING_MACRO(PostgresDbOid, 5);
102+
94103
#endif /* PG_DATABASE_H */

src/include/catalog/renumber_oids.pl

+10
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,16 @@
170170
$changed = 1;
171171
}
172172
}
173+
elsif (/^(DECLARE_OID_DEFINING_MACRO\(\s*\w+,\s*)(\d+)\)/)
174+
{
175+
if (exists $maphash{$2})
176+
{
177+
my $repl = $1 . $maphash{$2} . ")";
178+
$line =~
179+
s/^DECLARE_OID_DEFINING_MACRO\(\s*\w+,\s*\d+\)/$repl/;
180+
$changed = 1;
181+
}
182+
}
173183
elsif ($line =~ m/^CATALOG\((\w+),(\d+),(\w+)\)/)
174184
{
175185
if (exists $maphash{$2})

src/include/catalog/unused_oids

-9
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,6 @@ my @input_files = glob("pg_*.h");
3232

3333
my $oids = Catalog::FindAllOidsFromHeaders(@input_files);
3434

35-
# Push the template0 and postgres database OIDs.
36-
my $Template0ObjectId =
37-
Catalog::FindDefinedSymbol('access/transam.h', '..', 'Template0ObjectId');
38-
push @{$oids}, $Template0ObjectId;
39-
40-
my $PostgresObjectId =
41-
Catalog::FindDefinedSymbol('access/transam.h', '..', 'PostgresObjectId');
42-
push @{$oids}, $PostgresObjectId;
43-
4435
# Also push FirstGenbkiObjectId to serve as a terminator for the last gap.
4536
my $FirstGenbkiObjectId =
4637
Catalog::FindDefinedSymbol('access/transam.h', '..', 'FirstGenbkiObjectId');

0 commit comments

Comments
 (0)