Re: inconsistency and inefficiency in setup_conversion()

On 5/17/18, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> Hi John,
>
> This failed for my patch testing robot on Windows, with this message[1]:
...
> I see that you changed src/backend/catalog/Makefile to pass the new -I
> switch to genbki.pl. I think for Windows you might need to add it to
> the line in src/tools/msvc/Solution.pm that invokes genbki.pl via
> system()?

Yes, you're quite right. Thanks for the report. I've attached an
updated v2 patchset, with some additional revisions:

I wrote:

> -I have not done performance testing of initdb yet. I'll do so at a
> later date unless someone is excited enough to beat me to it.

Tom Lane reported [1] that setup_conversion() took ~70ms out of
1300ms. With this patch, on hardware with a similar total runtime, the
total is ~65ms faster, as one would expect based on the number of new
entries.

> -I piggy-backed on the OID lookup machinery for the encoding lookup,
> but haven't changed all the comments that refer only to catalogs and
> OIDs.

I considered changing the SGML documentation referring to OID
references [2], since this patch invalidates some details, but in the
end I thought that would make the docs harder to follow for the sake
of a small corner case. Instead, I added some comments around the
encoding lookups to alert the reader we're repurposing the OID lookup
machinery. I can revisit this later if desired.

> -With the 88 pg_proc entries with prolang=13 along with the 50 or so
> with prolang=14, it might be worth it to create a language lookup.
> This patch does not do so, however.

In version 2, patch 0001 adds a pg_language lookup. It turns out doing
so has a side benefit of simplifying Gen_fmgr.pl and its Makefile,
too.

--
[1] https://www.postgresql.org/message-id/7408.1525812528%40sss.pgh.pa.us
[2] https://www.postgresql.org/docs/devel/static/system-catalog-initial-data.html#SYSTEM-CATALOG-OID-REFERENCES

-John Naylor