Postgres Pro
Downloads
Home   >   mailing lists

Thread: renumber_oids.pl needs some updates

renumber_oids.pl needs some updates

From
Tom Lane
Date:
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

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_settingusing 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(parnametext_ops)); 
 DECLARE_UNIQUE_INDEX_PKEY(pg_parameter_acl_oid_index, 8928, ParameterAclOidIndexId, on pg_parameter_acl using
btree(oidoid_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_originusing btree(roident oid_ops)); 
 DECLARE_UNIQUE_INDEX(pg_replication_origin_roname_index, 6002, ReplicationOriginNameIndex, on pg_replication_origin
usingbtree(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(objoidoid_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(objoidoid_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(oidoid_ops)); 
 DECLARE_UNIQUE_INDEX(pg_subscription_subname_index, 6115, SubscriptionNameIndexId, on pg_subscription using
btree(subdbidoid_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

From
Peter Eisentraut
Date:
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.

Re: renumber_oids.pl needs some updates

From
Tom Lane
Date:
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