On 4/26/18, Tom Lane <t...@sss.pgh.pa.us> wrote:
> (The prosrc-from-proname case, in isolation, could be handled about
> as well by adding a hardwired rule like we have for pronargs.)
If we think we might go in the direction of more special-case
behavior, the attached patch more fully documents what we already do,
and does some minor refactoring to make future additions more
straightforward. I think this would even be good to do for v11.
> Robert Haas <robertmh...@gmail.com> writes:
>> -1 for trying to automate this. It varies between fooin and foo_in,
>> and it'll be annoying to have to remember which one happens
>> automatically and which one needs an override.
>
> Yeah, that was my first reaction to that example as well. If it
> weren't so nearly fifty-fifty then we might have more traction there;
> but it's pretty close, and actually the foo_in cases outnumber fooin
> if I counted correctly. (Array entries should be ignored for this
> purpose; maybe we'll autogenerate them someday.)
Hmm, that wouldn't be too hard. Add a new metadata field called
'array_type_oid', then if it finds such an OID, teach genbki.pl to
construct a tuple for the corresponding array type by consulting
something like
char typcategory BKI_ARRAY(A);
char typstorage BKI_ARRAY(x);
...etc
in the header file, plus copying typalign from the element type. I'll
whip this up sometime and add it to the next commitfest.
On 4/26/18, Mark Dilger <hornschnor...@gmail.com> wrote:
> /* cast method */
> - char castmethod;
> + char castmethod BKI_DEFAULT("/${castfunc} == 0 ? 'b' :
> 'f'/e");
> } FormData_pg_cast;
I don't have a strong opinion about your simple substitution
mechanism, but I find the above a bit harder to reason about. It also
has the added disadvantage that there is whitespace in the BKI
annotation, so it can no longer be parsed with the current setup. That
could be overcome with additional complexity, of course, but then
you're back in the position of advocating that for a single use case.
-John Naylor
From 0bec30078080730a8f1adf0e54060d60d6fc3015 Mon Sep 17 00:00:00 2001
From: John Naylor <jcnay...@gmail.com>
Date: Sat, 28 Apr 2018 15:54:14 +0700
Subject: [PATCH 1/1] Clarify special-case bootstrap values.
Take special-case computation of out of the loop over all columns.
This makes the code a bit more clear. It doesn't make much of a
difference now, but if many columns are computed, it becomes
difficult to read.
Also add more comments.
---
src/backend/catalog/Catalog.pm | 22 ++++++++++++++--------
src/include/catalog/pg_proc.dat | 3 +++
src/include/catalog/reformat_dat_file.pl | 13 ++++++-------
3 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 67d1719..a6f555c 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -292,6 +292,20 @@ sub AddDefaultValues
my ($row, $schema, $catname) = @_;
my @missing_fields;
+ # Compute special-case field values.
+ # Note: If you add new cases here, you must also teach
+ # strip_default_values() in include/catalog/reformat_dat_file.pl
+ # to delete them.
+ if ($catname eq 'pg_proc')
+ {
+ # pg_proc.pronargs can be derived from proargtypes.
+ if (defined $row->{proargtypes})
+ {
+ my @proargtypes = split /\s+/, $row->{proargtypes};
+ $row->{pronargs} = scalar(@proargtypes);
+ }
+ }
+
foreach my $column (@$schema)
{
my $attname = $column->{name};
@@ -305,14 +319,6 @@ sub AddDefaultValues
{
$row->{$attname} = $column->{default};
}
- elsif ($catname eq 'pg_proc'
- && $attname eq 'pronargs'
- && defined($row->{proargtypes}))
- {
- # pg_proc.pronargs can be derived from proargtypes.
- my @proargtypes = split /\s+/, $row->{proargtypes};
- $row->{$attname} = scalar(@proargtypes);
- }
else
{
# Failed to find a value.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index f643f56..be37fb0 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -19,6 +19,9 @@
# duplicate the operator's comment.) initdb will supply suitable default
# comments for functions referenced by pg_operator.
+# Note: pronargs is computed when this file is read, so does not need to be
+# specified here. See AddDefaultValues() in Catalog.pm.
+
# Try to follow the style of existing functions' comments.
# Some recommended conventions:
diff --git a/src/include/catalog/reformat_dat_file.pl b/src/include/catalog/reformat_dat_file.pl
index 8ebbec6..c807d3a 100644
--- a/src/include/catalog/reformat_dat_file.pl
+++ b/src/include/catalog/reformat_dat_file.pl
@@ -194,14 +194,13 @@ sub strip_default_values
{
delete $row->{$attname};
}
+ }
- # Also delete pg_proc.pronargs, since that can be recomputed.
- if ( $catname eq 'pg_proc'
- && $attname eq 'pronargs'
- && defined($row->{proargtypes}))
- {
- delete $row->{$attname};
- }
+ # Delete computed values. See AddDefaultValues() in Catalog.pm.
+ # Note: This must be done after deleting values matching defaults.
+ if ($catname eq 'pg_proc')
+ {
+ delete $row->{pronargs} if defined $row->{proargtypes};
}
}
--
2.7.4
