pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedrenumber_oids.pl needs some updates
3+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2022-04-21 40eba064b2 Use DECLARE_TOAST_WITH_MACRO() to simplify toast-table declarations.
* renumber_oids.pl needs some updates
@ 2022-04-20 20:45 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2022-04-20 20:45 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org; +Cc: John Naylor <john.naylor@enterprisedb.com>
I did a test run of renumber_oids.pl to see if there would be any
problems when the time comes (pretty soon!) to run it for v15.
Depressingly enough, I found two problems:
1. When commit dfb75e478 invented DECLARE_UNIQUE_INDEX_PKEY,
it neglected to teach renumber_oids.pl about it. I'm surprised
we did not notice this last year.
2. renumber_oids.pl failed miserably on pg_parameter_acl.h:
@@ -48,11 +48,11 @@ CATALOG(pg_parameter_acl,8924,ParameterAclRelationId) BKI_SHARED_RELATION
*/
typedef FormData_pg_parameter_acl *Form_pg_parameter_acl;
-DECLARE_TOAST(pg_parameter_acl, 8925, 8926);
+DECLARE_TOAST(pg_parameter_acl, 6244, 6245);
#define PgParameterAclToastTable 8925
#define PgParameterAclToastIndex 8926
because of course it didn't know it should update the
PgParameterAclToastTable and PgParameterAclToastIndex macro definitions.
(We have this same coding pattern elsewhere, but I guess that
renumber_oids.pl has never previously been asked to renumber a shared
catalog.)
I think the right way to fix #2 is to put the responsibility for
generating the #define's into genbki.pl, instead of this mistake-prone
approach of duplicating the OID constants in the source code.
The attached proposed patch invents a variant macro
DECLARE_TOAST_WITH_MACRO for the relatively small number of cases
where we need such OID macros. A different idea could be to require
all the catalog headers to define C macros for their toast tables
and change DECLARE_TOAST to a five-argument macro across the board.
However, that would require touching a bunch more places and inventing
a bunch more macro names, and it didn't really seem useful.
Thoughts?
regards, tom lane
Attachments:
generate-toast-table-oid-macros-1.patch (text/x-diff, 11.0K) download
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index dbc3f87e28..0275795dea 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -94,6 +94,17 @@ sub ParseHeader
push @{ $catalog{toasting} },
{ parent_table => $1, toast_oid => $2, toast_index_oid => $3 };
}
+ elsif (/^DECLARE_TOAST_WITH_MACRO\(\s*(\w+),\s*(\d+),\s*(\d+),\s*(\w+),\s*(\w+)\)/)
+ {
+ push @{ $catalog{toasting} },
+ {
+ parent_table => $1,
+ toast_oid => $2,
+ toast_index_oid => $3,
+ toast_oid_macro => $4,
+ toast_index_oid_macro => $5
+ };
+ }
elsif (
/^DECLARE_(UNIQUE_)?INDEX(_PKEY)?\(\s*(\w+),\s*(\d+),\s*(\w+),\s*(.+)\)/)
{
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 2dfcdc5dad..2d02b02267 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -472,6 +472,16 @@ EOM
$catalog->{rowtype_oid_macro}, $catalog->{rowtype_oid}
if $catalog->{rowtype_oid_macro};
+ # Likewise for macros for toast and index OIDs
+ foreach my $toast (@{ $catalog->{toasting} })
+ {
+ printf $def "#define %s %s\n",
+ $toast->{toast_oid_macro}, $toast->{toast_oid}
+ if $toast->{toast_oid_macro};
+ printf $def "#define %s %s\n",
+ $toast->{toast_index_oid_macro}, $toast->{toast_index_oid}
+ if $toast->{toast_index_oid_macro};
+ }
foreach my $index (@{ $catalog->{indexing} })
{
printf $def "#define %s %s\n",
diff --git a/src/include/catalog/genbki.h b/src/include/catalog/genbki.h
index 9fb0d2c83d..4ecd76f4be 100644
--- a/src/include/catalog/genbki.h
+++ b/src/include/catalog/genbki.h
@@ -55,9 +55,13 @@
* need stable OIDs for shared relations, and that includes toast tables
* of shared relations.
*
- * The macro definition is just to keep the C compiler from spitting up.
+ * The DECLARE_TOAST_WITH_MACRO variant is used when C macros are needed
+ * for the toast table/index OIDs (usually only for shared catalogs).
+ *
+ * The macro definitions are just to keep the C compiler from spitting up.
*/
#define DECLARE_TOAST(name,toastoid,indexoid) extern int no_such_variable
+#define DECLARE_TOAST_WITH_MACRO(name,toastoid,indexoid,toastoidmacro,indexoidmacro) extern int no_such_variable
/*
* These lines are processed by genbki.pl to create the statements
diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
index 4b65e39a1f..3512601c80 100644
--- a/src/include/catalog/pg_authid.h
+++ b/src/include/catalog/pg_authid.h
@@ -55,9 +55,7 @@ CATALOG(pg_authid,1260,AuthIdRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(284
*/
typedef FormData_pg_authid *Form_pg_authid;
-DECLARE_TOAST(pg_authid, 4175, 4176);
-#define PgAuthidToastTable 4175
-#define PgAuthidToastIndex 4176
+DECLARE_TOAST_WITH_MACRO(pg_authid, 4175, 4176, PgAuthidToastTable, PgAuthidToastIndex);
DECLARE_UNIQUE_INDEX(pg_authid_rolname_index, 2676, AuthIdRolnameIndexId, on pg_authid using btree(rolname name_ops));
DECLARE_UNIQUE_INDEX_PKEY(pg_authid_oid_index, 2677, AuthIdOidIndexId, on pg_authid using btree(oid oid_ops));
diff --git a/src/include/catalog/pg_database.h b/src/include/catalog/pg_database.h
index a9f4a8071f..e10e91c0af 100644
--- a/src/include/catalog/pg_database.h
+++ b/src/include/catalog/pg_database.h
@@ -86,9 +86,7 @@ CATALOG(pg_database,1262,DatabaseRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID
*/
typedef FormData_pg_database *Form_pg_database;
-DECLARE_TOAST(pg_database, 4177, 4178);
-#define PgDatabaseToastTable 4177
-#define PgDatabaseToastIndex 4178
+DECLARE_TOAST_WITH_MACRO(pg_database, 4177, 4178, PgDatabaseToastTable, PgDatabaseToastIndex);
DECLARE_UNIQUE_INDEX(pg_database_datname_index, 2671, DatabaseNameIndexId, on pg_database using btree(datname name_ops));
DECLARE_UNIQUE_INDEX_PKEY(pg_database_oid_index, 2672, DatabaseOidIndexId, on pg_database using btree(oid oid_ops));
diff --git a/src/include/catalog/pg_db_role_setting.h b/src/include/catalog/pg_db_role_setting.h
index ae5cc4365e..45d478e9e7 100644
--- a/src/include/catalog/pg_db_role_setting.h
+++ b/src/include/catalog/pg_db_role_setting.h
@@ -46,9 +46,7 @@ CATALOG(pg_db_role_setting,2964,DbRoleSettingRelationId) BKI_SHARED_RELATION
typedef FormData_pg_db_role_setting * Form_pg_db_role_setting;
-DECLARE_TOAST(pg_db_role_setting, 2966, 2967);
-#define PgDbRoleSettingToastTable 2966
-#define PgDbRoleSettingToastIndex 2967
+DECLARE_TOAST_WITH_MACRO(pg_db_role_setting, 2966, 2967, PgDbRoleSettingToastTable, PgDbRoleSettingToastIndex);
DECLARE_UNIQUE_INDEX_PKEY(pg_db_role_setting_databaseid_rol_index, 2965, DbRoleSettingDatidRolidIndexId, on pg_db_role_setting using btree(setdatabase oid_ops, setrole oid_ops));
diff --git a/src/include/catalog/pg_parameter_acl.h b/src/include/catalog/pg_parameter_acl.h
index 8316391e51..263079c9e1 100644
--- a/src/include/catalog/pg_parameter_acl.h
+++ b/src/include/catalog/pg_parameter_acl.h
@@ -48,9 +48,7 @@ CATALOG(pg_parameter_acl,8924,ParameterAclRelationId) BKI_SHARED_RELATION
*/
typedef FormData_pg_parameter_acl *Form_pg_parameter_acl;
-DECLARE_TOAST(pg_parameter_acl, 8925, 8926);
-#define PgParameterAclToastTable 8925
-#define PgParameterAclToastIndex 8926
+DECLARE_TOAST_WITH_MACRO(pg_parameter_acl, 8925, 8926, PgParameterAclToastTable, PgParameterAclToastIndex);
DECLARE_UNIQUE_INDEX(pg_parameter_acl_parname_index, 8927, ParameterAclParnameIndexId, on pg_parameter_acl using btree(parname text_ops));
DECLARE_UNIQUE_INDEX_PKEY(pg_parameter_acl_oid_index, 8928, ParameterAclOidIndexId, on pg_parameter_acl using btree(oid oid_ops));
diff --git a/src/include/catalog/pg_replication_origin.h b/src/include/catalog/pg_replication_origin.h
index 5dcf1dafe3..3b11cd21cb 100644
--- a/src/include/catalog/pg_replication_origin.h
+++ b/src/include/catalog/pg_replication_origin.h
@@ -54,9 +54,7 @@ CATALOG(pg_replication_origin,6000,ReplicationOriginRelationId) BKI_SHARED_RELAT
typedef FormData_pg_replication_origin *Form_pg_replication_origin;
-DECLARE_TOAST(pg_replication_origin, 4181, 4182);
-#define PgReplicationOriginToastTable 4181
-#define PgReplicationOriginToastIndex 4182
+DECLARE_TOAST_WITH_MACRO(pg_replication_origin, 4181, 4182, PgReplicationOriginToastTable, PgReplicationOriginToastIndex);
DECLARE_UNIQUE_INDEX_PKEY(pg_replication_origin_roiident_index, 6001, ReplicationOriginIdentIndex, on pg_replication_origin using btree(roident oid_ops));
DECLARE_UNIQUE_INDEX(pg_replication_origin_roname_index, 6002, ReplicationOriginNameIndex, on pg_replication_origin using btree(roname text_ops));
diff --git a/src/include/catalog/pg_shdescription.h b/src/include/catalog/pg_shdescription.h
index 1e354b29c2..da2b9f6fdf 100644
--- a/src/include/catalog/pg_shdescription.h
+++ b/src/include/catalog/pg_shdescription.h
@@ -55,9 +55,7 @@ CATALOG(pg_shdescription,2396,SharedDescriptionRelationId) BKI_SHARED_RELATION
*/
typedef FormData_pg_shdescription * Form_pg_shdescription;
-DECLARE_TOAST(pg_shdescription, 2846, 2847);
-#define PgShdescriptionToastTable 2846
-#define PgShdescriptionToastIndex 2847
+DECLARE_TOAST_WITH_MACRO(pg_shdescription, 2846, 2847, PgShdescriptionToastTable, PgShdescriptionToastIndex);
DECLARE_UNIQUE_INDEX_PKEY(pg_shdescription_o_c_index, 2397, SharedDescriptionObjIndexId, on pg_shdescription using btree(objoid oid_ops, classoid oid_ops));
diff --git a/src/include/catalog/pg_shseclabel.h b/src/include/catalog/pg_shseclabel.h
index 6f6bd9da2f..fc1b97f46f 100644
--- a/src/include/catalog/pg_shseclabel.h
+++ b/src/include/catalog/pg_shseclabel.h
@@ -39,9 +39,7 @@ CATALOG(pg_shseclabel,3592,SharedSecLabelRelationId) BKI_SHARED_RELATION BKI_ROW
typedef FormData_pg_shseclabel * Form_pg_shseclabel;
-DECLARE_TOAST(pg_shseclabel, 4060, 4061);
-#define PgShseclabelToastTable 4060
-#define PgShseclabelToastIndex 4061
+DECLARE_TOAST_WITH_MACRO(pg_shseclabel, 4060, 4061, PgShseclabelToastTable, PgShseclabelToastIndex);
DECLARE_UNIQUE_INDEX_PKEY(pg_shseclabel_object_index, 3593, SharedSecLabelObjectIndexId, on pg_shseclabel using btree(objoid oid_ops, classoid oid_ops, provider text_ops));
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index f006a92612..d1260f590c 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -92,9 +92,7 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId) BKI_SHARED_RELATION BKI_ROW
typedef FormData_pg_subscription *Form_pg_subscription;
-DECLARE_TOAST(pg_subscription, 4183, 4184);
-#define PgSubscriptionToastTable 4183
-#define PgSubscriptionToastIndex 4184
+DECLARE_TOAST_WITH_MACRO(pg_subscription, 4183, 4184, PgSubscriptionToastTable, PgSubscriptionToastIndex);
DECLARE_UNIQUE_INDEX_PKEY(pg_subscription_oid_index, 6114, SubscriptionObjectIndexId, on pg_subscription using btree(oid oid_ops));
DECLARE_UNIQUE_INDEX(pg_subscription_subname_index, 6115, SubscriptionNameIndexId, on pg_subscription using btree(subdbid oid_ops, subname name_ops));
diff --git a/src/include/catalog/pg_tablespace.h b/src/include/catalog/pg_tablespace.h
index 58c1cfa605..572eeeb8a6 100644
--- a/src/include/catalog/pg_tablespace.h
+++ b/src/include/catalog/pg_tablespace.h
@@ -47,9 +47,7 @@ CATALOG(pg_tablespace,1213,TableSpaceRelationId) BKI_SHARED_RELATION
*/
typedef FormData_pg_tablespace *Form_pg_tablespace;
-DECLARE_TOAST(pg_tablespace, 4185, 4186);
-#define PgTablespaceToastTable 4185
-#define PgTablespaceToastIndex 4186
+DECLARE_TOAST_WITH_MACRO(pg_tablespace, 4185, 4186, PgTablespaceToastTable, PgTablespaceToastIndex);
DECLARE_UNIQUE_INDEX_PKEY(pg_tablespace_oid_index, 2697, TablespaceOidIndexId, on pg_tablespace using btree(oid oid_ops));
DECLARE_UNIQUE_INDEX(pg_tablespace_spcname_index, 2698, TablespaceNameIndexId, on pg_tablespace using btree(spcname name_ops));
diff --git a/src/include/catalog/renumber_oids.pl b/src/include/catalog/renumber_oids.pl
index d959c2de0f..7de13da4bd 100755
--- a/src/include/catalog/renumber_oids.pl
+++ b/src/include/catalog/renumber_oids.pl
@@ -140,14 +140,33 @@ foreach my $input_file (@header_files)
$changed = 1;
}
}
+ elsif ($line =~ m/^(DECLARE_TOAST_WITH_MACRO\(\s*\w+,\s*)(\d+)(,\s*)(\d+)(,\s*\w+,\s*\w+)\)/)
+ {
+ my $oid2 = $2;
+ my $oid4 = $4;
+ if (exists $maphash{$oid2})
+ {
+ $oid2 = $maphash{$oid2};
+ my $repl = $1 . $oid2 . $3 . $oid4 . $5 . ")";
+ $line =~ s/^DECLARE_TOAST_WITH_MACRO\(\s*\w+,\s*\d+,\s*\d+,\s*\w+,\s*\w+\)/$repl/;
+ $changed = 1;
+ }
+ if (exists $maphash{$oid4})
+ {
+ $oid4 = $maphash{$oid4};
+ my $repl = $1 . $oid2 . $3 . $oid4 . $5 . ")";
+ $line =~ s/^DECLARE_TOAST_WITH_MACRO\(\s*\w+,\s*\d+,\s*\d+,\s*\w+,\s*\w+\)/$repl/;
+ $changed = 1;
+ }
+ }
elsif (
- $line =~ m/^(DECLARE_(UNIQUE_)?INDEX\(\s*\w+,\s*)(\d+)(,\s*.+)\)/)
+ $line =~ m/^(DECLARE_(UNIQUE_)?INDEX(_PKEY)?\(\s*\w+,\s*)(\d+)(,\s*.+)\)/)
{
- if (exists $maphash{$3})
+ if (exists $maphash{$4})
{
- my $repl = $1 . $maphash{$3} . $4 . ")";
+ my $repl = $1 . $maphash{$4} . $5 . ")";
$line =~
- s/^DECLARE_(UNIQUE_)?INDEX\(\s*\w+,\s*\d+,\s*.+\)/$repl/;
+ s/^DECLARE_(UNIQUE_)?INDEX(_PKEY)?\(\s*\w+,\s*\d+,\s*.+\)/$repl/;
$changed = 1;
}
}
* Re: renumber_oids.pl needs some updates
@ 2022-04-20 20:56 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 3+ messages in thread
From: Peter Eisentraut @ 2022-04-20 20:56 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org; +Cc: John Naylor <john.naylor@enterprisedb.com>
On 20.04.22 22:45, Tom Lane wrote:
> I think the right way to fix #2 is to put the responsibility for
> generating the #define's into genbki.pl, instead of this mistake-prone
> approach of duplicating the OID constants in the source code.
>
> The attached proposed patch invents a variant macro
> DECLARE_TOAST_WITH_MACRO for the relatively small number of cases
> where we need such OID macros.
This makes sense.
A more elaborate (future) project would be to have genbki.pl generate
all of IsSharedRelation(), which is the only place these toast-table-OID
macros are used, AFAICT.
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: renumber_oids.pl needs some updates
@ 2022-04-20 21:10 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2022-04-20 21:10 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; +Cc: pgsql-hackers@lists.postgresql.org, John Naylor <john.naylor@enterprisedb.com>
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 20.04.22 22:45, Tom Lane wrote:
>> The attached proposed patch invents a variant macro
>> DECLARE_TOAST_WITH_MACRO for the relatively small number of cases
>> where we need such OID macros.
> This makes sense.
> A more elaborate (future) project would be to have genbki.pl generate
> all of IsSharedRelation(), which is the only place these toast-table-OID
> macros are used, AFAICT.
Perhaps. We invent shared catalogs at a slow enough rate that I'm
not sure the effort would ever pay for itself in person-hours,
but maybe making such invention a trifle less error-prone is
worth something.
I'd still want to keep this form of DECLARE_TOAST, in case someone
comes up with a different reason to want macro names for toast OIDs.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-04-20 21:10 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-04-20 20:45 renumber_oids.pl needs some updates Tom Lane <tgl@sss.pgh.pa.us>
2022-04-20 20:56 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-04-20 21:10 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox