Re: generating bootstrap entries for array types - Mailing list pgsql-hackers

Tom Lane <tgl@sss.pgh.pa.us> writes:

> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>> ilmari@ilmari.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