On 4/5/18, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Here are the results of an evening's desultory hacking on v13.
>
> [numeric function oids with overloaded name]
Thank you for the detailed review and for improving the function
references (not to mention the type references I somehow left on the
table). I was also not quite satisfied with just the regproc columns.
> I did not like the hard-wired handling of proargtypes and proallargtypes
> in genbki.pl; it hardly seems impossible that we'll want similar
> conversions for other array-of-OID columns in future. After a bit of
> thought, it seemed like we could allow
>
> oidvector proargtypes BKI_LOOKUP(pg_type);
>
> Oid proallargtypes[1] BKI_DEFAULT(_null_) BKI_LOOKUP(pg_type);
>
> and just teach genbki.pl that if a lookup rule is attached to
> an oidvector or Oid[] column, it means to apply the rule to
> each array element individually.
I think that's a good idea. I went an extra step and extracted the
common logic into a function (attached draft patch to be applied on
top of yours). It treats all lookups as operating on arrays. The
common case is that we pass a single-element array. That may seem
awkward, but I think it's clear. The code is slimmer, and the lines
now fit within 80 characters.
> I also changed genbki.pl so that it'd warn about entries that aren't
> recognized by the lookup rules. This seems like a good idea for
> catching errors, such as (ahem) applying BKI_LOOKUP to a column
> that isn't even an OID.
Yikes, I must have fat-fingered that during the comment reformatting.
Unrelated, I noticed my quoting of defaults that contain back-slashes
was half-baked, so I'll include that fix in the next patchset. I'll
put out a new one in a couple days, to give a chance for further
review and discussion of the defaults. I didn't feel the need to
respond to the other messages, but yours and Andres' points are well
taken.
-John Naylor
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index f6be50a..8d47109 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -343,34 +343,21 @@ EOM
#
# If the column type is oidvector or oid[], we have to replace
# each element of the array as per the lookup rule.
- #
- # If we don't have a unique value to substitute, warn and
- # leave the entry unchanged.
if ($column->{lookup})
{
my $lookup = $lookup_kind{ $column->{lookup} };
+ my @lookupnames;
+ my @lookupoids;
die "unrecognized BKI_LOOKUP type " . $column->{lookup}
if !defined($lookup);
if ($atttype eq 'oidvector')
{
- my @lookupnames = split /\s+/, $bki_values{$attname};
- my @lookupoids;
- foreach my $lookupname (@lookupnames)
- {
- my $lookupoid = $lookup->{ $lookupname };
- if (defined($lookupoid) && $lookupoid ne 'MULTIPLE')
- {
- $lookupname = $lookupoid;
- }
- else
- {
- warn "unresolved OID reference \"$lookupname\" in $catname row " . join(',', values(%bki_values))
- if $lookupname ne '-' && $lookupname ne '0';
- }
- push @lookupoids, $lookupname;
- }
+ @lookupnames = split /\s+/, $bki_values{$attname};
+ @lookupoids = lookup_oids($lookup, $catname,
+ \%bki_values, @lookupnames);
+
$bki_values{$attname} = join(' ', @lookupoids);
}
elsif ($atttype eq 'oid[]')
@@ -378,39 +365,21 @@ EOM
if ($bki_values{$attname} ne '_null_')
{
$bki_values{$attname} =~ s/[{}]//g;
- my @lookupnames = split /,/, $bki_values{$attname};
- my @lookupoids;
- foreach my $lookupname (@lookupnames)
- {
- my $lookupoid = $lookup->{ $lookupname };
- if (defined($lookupoid) && $lookupoid ne 'MULTIPLE')
- {
- $lookupname = $lookupoid;
- }
- else
- {
- warn "unresolved OID reference \"$lookupname\" in $catname row " . join(',', values(%bki_values))
- if $lookupname ne '-' && $lookupname ne '0';
- }
- push @lookupoids, $lookupname;
- }
+ @lookupnames = split /,/, $bki_values{$attname};
+ @lookupoids = lookup_oids($lookup, $catname,
+ \%bki_values, @lookupnames);
+
$bki_values{$attname} =
sprintf "{%s}", join(',', @lookupoids);
}
}
else
{
- my $lookupname = $bki_values{$attname};
- my $lookupoid = $lookup->{ $lookupname };
- if (defined($lookupoid) && $lookupoid ne 'MULTIPLE')
- {
- $bki_values{$attname} = $lookupoid;
- }
- else
- {
- warn "unresolved OID reference \"$lookupname\" in $catname row " . join(',', values(%bki_values))
- if $lookupname ne '-' && $lookupname ne '0';
- }
+ $lookupnames[0] = $bki_values{$attname};
+ @lookupoids = lookup_oids($lookup, $catname,
+ \%bki_values, @lookupnames);
+
+ $bki_values{$attname} = $lookupoids[0];
}
}
}
@@ -759,6 +728,32 @@ sub morph_row_for_schemapg
}
}
+# Perform OID lookups on an array of OID names.
+# If we don't have a unique value to substitute, warn and
+# leave the entry unchanged.
+sub lookup_oids
+{
+ my ($lookup, $catname, $bki_values, @lookupnames) = @_;
+
+ my @lookupoids;
+ foreach my $lookupname (@lookupnames)
+ {
+ my $lookupoid = $lookup->{$lookupname};
+ if (defined($lookupoid) and $lookupoid ne 'MULTIPLE')
+ {
+ push @lookupoids, $lookupoid;
+ }
+ else
+ {
+ push @lookupoids, $lookupname;
+ warn "unresolved OID reference \"$lookupname\" in $catname row "
+ . join(',', values(%$bki_values))
+ if $lookupname ne '-' && $lookupname ne '0';
+ }
+ }
+ return @lookupoids;
+}
+
# Determine canonical pg_type OID #define symbol from the type name.
sub form_pg_type_symbol
{
