Getting Gen_fmgrtab.pl to generate macros for all pg_proc entries - Mailing list pgsql-hackers

Over in [1] I noted a need for fmgroids.h macros for pg_proc entries
that overload a particular C function.  The F_XXX macros seem to be
designed on the assumption that the only thing they're needed for
is to call the function, so you don't really care which SQL alias
you use to get there.  That's unhelpful though when you are concerned
with the SQL-level function more than the C function.  I seem to recall
that we've run into this before, but up to now have been able to work
around it.

A narrow solution would be to revert the policy we just established
(in commit 36b931214) against allowing oid_symbol entries in
pg_proc.dat, and just manually label such entries when we need to refer
to them in the C code.  I don't especially care for that though, for
the reasons cited in 36b931214 --- it'll lead to a lot of random
variation in how people refer to pg_proc entries, rather than having
a uniform convention.

I experimented with generating the fmgroids.h macros by using, not
the prosrc field, but the concatenation of the proname and proargtypes
fields.  That's guaranteed unique within pg_proc, but it would have
resulted in several hundred required changes in our source tree,
because of common references to F_OIDEQ and suchlike.  But I discovered
that we could alter the rule to be "append proargtypes only if proname
is not unique", and that eliminates the pain almost completely: only
two existing code references need to change.

As an additional benefit, we can generate macros for all the bootstrap
pg_proc entries, not only ones that are for C functions; that fixes
another issue noted in [1].

Hence, I propose the attached.  Any objections?

            regards, tom lane

[1] https://www.postgresql.org/message-id/298489.1604188643%40sss.pgh.pa.us

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index e7d814651b..85ef873caa 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -779,7 +779,7 @@ contain_volatile_functions_not_nextval(Node *clause)
 static bool
 contain_volatile_functions_not_nextval_checker(Oid func_id, void *context)
 {
-    return (func_id != F_NEXTVAL_OID &&
+    return (func_id != F_NEXTVAL &&
             func_volatile(func_id) == PROVOLATILE_VOLATILE);
 }

diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index b7c7b4c8fa..a3f7307263 100644
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -64,22 +64,26 @@ foreach my $datfile (@ARGV)

 # Collect certain fields from pg_proc.dat.
 my @fmgr = ();
+my %proname_counts;

 foreach my $row (@{ $catalog_data{pg_proc} })
 {
     my %bki_values = %$row;

-    # Select out just the rows for internal-language procedures.
-    next if $bki_values{prolang} ne 'internal';
-
     push @fmgr,
       {
         oid    => $bki_values{oid},
+        name   => $bki_values{proname},
+        args   => $bki_values{proargtypes},
         strict => $bki_values{proisstrict},
         retset => $bki_values{proretset},
         nargs  => $bki_values{pronargs},
+        lang   => $bki_values{prolang},
         prosrc => $bki_values{prosrc},
       };
+
+    # Count so that we can detect overloaded pronames.
+    $proname_counts{ $bki_values{proname} }++;
 }

 # Emit headers for both files
@@ -122,13 +126,10 @@ print $ofh <<OFH;
 /*
  *    Constant macros for the OIDs of entries in pg_proc.
  *
- *    NOTE: macros are named after the prosrc value, ie the actual C name
- *    of the implementing function, not the proname which may be overloaded.
- *    For example, we want to be able to assign different macro names to both
- *    char_text() and name_text() even though these both appear with proname
- *    'text'.  If the same C function appears in more than one pg_proc entry,
- *    its equivalent macro will be defined with the lowest OID among those
- *    entries.
+ *    F_XXX macros are named after the proname field; if that is not unique,
+ *    we append the proargtypes field, replacing spaces with underscores.
+ *    For example, we have F_OIDEQ because that proname is unique, but
+ *    F_POW_FLOAT8_FLOAT8 (among others) because that proname is not.
  */
 OFH

@@ -186,14 +187,20 @@ print $tfh <<TFH;

 TFH

-# Emit #define's and extern's -- only one per prosrc value
+# Emit fmgroids.h and fmgrprotos.h entries in OID order.
 my %seenit;
 foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
 {
-    next if $seenit{ $s->{prosrc} };
-    $seenit{ $s->{prosrc} } = 1;
-    print $ofh "#define F_" . uc $s->{prosrc} . " $s->{oid}\n";
-    print $pfh "extern Datum $s->{prosrc}(PG_FUNCTION_ARGS);\n";
+    my $sqlname = $s->{name};
+    $sqlname .= "_" . $s->{args} if ($proname_counts{ $s->{name} } > 1);
+    $sqlname =~ tr/ /_/;
+    print $ofh "#define F_" . uc $sqlname . " $s->{oid}\n";
+    # We want only one extern per internal-language function
+    if ($s->{lang} eq 'internal' && !$seenit{ $s->{prosrc} })
+    {
+        $seenit{ $s->{prosrc} } = 1;
+        print $pfh "extern Datum $s->{prosrc}(PG_FUNCTION_ARGS);\n";
+    }
 }

 # Create the fmgr_builtins table, collect data for fmgr_builtin_oid_index
@@ -206,22 +213,16 @@ my $last_builtin_oid = 0;
 my $fmgr_count       = 0;
 foreach my $s (sort { $a->{oid} <=> $b->{oid} } @fmgr)
 {
+    next if $s->{lang} ne 'internal';
+
+    print $tfh ",\n" if ($fmgr_count > 0);
     print $tfh
       "  { $s->{oid}, $s->{nargs}, $bmap{$s->{strict}}, $bmap{$s->{retset}}, \"$s->{prosrc}\", $s->{prosrc} }";

     $fmgr_builtin_oid_index[ $s->{oid} ] = $fmgr_count++;
     $last_builtin_oid = $s->{oid};
-
-    if ($fmgr_count <= $#fmgr)
-    {
-        print $tfh ",\n";
-    }
-    else
-    {
-        print $tfh "\n";
-    }
 }
-print $tfh "};\n";
+print $tfh "\n};\n";

 printf $tfh qq|
 const int fmgr_nbuiltins = (sizeof(fmgr_builtins) / sizeof(FmgrBuiltin));
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 6c656586e8..7446193252 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10144,7 +10144,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
                         RangeTblFunction *rtfunc = (RangeTblFunction *) lfirst(lc);

                         if (!IsA(rtfunc->funcexpr, FuncExpr) ||
-                            ((FuncExpr *) rtfunc->funcexpr)->funcid != F_ARRAY_UNNEST ||
+                            ((FuncExpr *) rtfunc->funcexpr)->funcid != F_UNNEST_ANYARRAY ||
                             rtfunc->funccolnames != NIL)
                         {
                             all_unnest = false;