Re: generating bootstrap entries for array types

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> ilmari(at)ilmari(dot)org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>> ilmari(at)ilmari(dot)org (Dagfinn Ilmari Mannsåker) writes:
>>> Doing this in genbki.pl makes DBD::Pg lose its array type information,
>>> since it uses Catalog::ParseData() to get it
>>> (https://github.com/bucardo/dbdpg/blob/master/types.c#L439). May I
>>> suggest moving gen_array_types() to Catalog.pm and calling it from
>>> ParseData() if the input file is pg_type.dat, so that it always returns
>>> complete data?
>
>> Attached is a proposed revision of this patch that does the above. It
>> passes check-world, reformat_dat_file.pl still works, and DBD::Pg is
>> still able to get the array type information.
>
> I find the way you did that (making the call dependent on
> $preserve_formatting) to be a mighty ugly kluge. It causes ParseData
> to know far more than it should about what callers are likely to do with
> the data.
>
> I'm inclined to think the right way is to do the expansion always,
> and teach reformat_dat_file.pl to drop autogenerated array types
> on the way out, making this work more like the existing facilities
> for default/computed fields.
>
> The easiest way to make that happen seems to be to invent another, purely
> internal metadata field, along the lines of "is_autogenerated_entry".
> Fooling with that now ...

Something like this, on top of the v2 patch?

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 12b142928a..7286c5b673 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -297,7 +297,7 @@ sub ParseData

# Generate array types unless we're just reformatting the file
Catalog::GenArrayTypes($schema, $data)
- if $catname eq 'pg_type' and !$preserve_formatting;
+ if $catname eq 'pg_type';
return $data;
}

@@ -485,6 +485,7 @@ sub GenArrayTypes

foreach my $elem_type (@$types)
{
+ next if ref $elem_type ne 'HASH';
next if !exists $elem_type->{array_type_oid};
my %array_type;

@@ -493,6 +494,9 @@ sub GenArrayTypes
$array_type{typname} = '_' . $elem_type->{typname};
$array_type{typelem} = $elem_type->{typname};

+ # Stops this entry being output when reformatting the .dat files
+ $array_type{is_auto_generated_entry} = 1;
+
# Arrays require INT alignment, unless the element type requires
# DOUBLE alignment.
$array_type{typalign} = $elem_type->{typalign} eq 'd' ? 'd' : 'i';
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 053d1ee00d..3859d46043 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -396,7 +396,8 @@ EOM
|| $key eq "oid_symbol"
|| $key eq "descr"
|| $key eq "line_number"
- || $key eq "array_type_oid";
+ || $key eq "array_type_oid"
+ || $key eq "is_auto_generated_entry";
die sprintf "unrecognized field name \"%s\" in %s.dat line %s\n",
$key, $catname, $bki_values{line_number}
if (!exists($attnames{$key}));
diff --git a/src/include/catalog/reformat_dat_file.pl b/src/include/catalog/reformat_dat_file.pl
index 24d9d18637..78c0f3e103 100755
--- a/src/include/catalog/reformat_dat_file.pl
+++ b/src/include/catalog/reformat_dat_file.pl
@@ -132,6 +132,7 @@ foreach my $catname (@catnames)
if (ref $data eq 'HASH')
{
my %values = %$data;
+ next if $values{is_auto_generated_entry};

############################################################
# At this point we have the full tuple in memory as a hash

- ilmari
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law