Re: Radix tree for character conversion

Hello, thank you for polishing this.

At Wed, 9 Nov 2016 02:19:01 +0100, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote in <80F34F25-BF6D-4BCD-9C38-42ED10D3F453(at)yesql(dot)se>
> > On 08 Nov 2016, at 17:37, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> >
> > On 10/31/16 12:11 PM, Daniel Gustafsson wrote:
> >> I took a small stab at doing some cleaning of the Perl scripts, mainly around
> >> using the more modern (well, modern as in +15 years old) form for open(..),
> >> avoiding global filehandles for passing scalar references and enforcing use
> >> strict. Some smaller typos and fixes were also included. It seems my Perl has
> >> become a bit rusty so I hope the changes make sense. The produced files are
> >> identical with these patches applied, they are merely doing cleaning as opposed
> >> to bugfixing.
> >>
> >> The attached patches are against the 0001-0006 patches from Heikki and you in
> >> this series of emails, the separation is intended to make them easier to read.
> >
> > Cool. See also here:
> > https://www.postgresql.org/message-id/55E52225.4040305%40gmx.net

> Nice, not having hacked much Perl in quite a while I had all but forgotten
> about perlcritic.

I tried it on CentOS7. Installation failed saying that
Module::Build is too old. It is yum-inatlled so removed it and
installed it with CPAN. Again failed with many 'Could not create
MYMETA files'. Then tried to install CPAN::Meta and it failed
saying that CPAN::Meta::YAML is too *new*. That sucks.

So your patch is greately helpfull. Thank you.

| -my @mapnames = map { s/\.map//; $_ } values %plainmaps;
| +my @mapnames = map { my $m = $_; $m =~ s/\.map//; $m } values %plainmaps;

It surprised me to know that perlcritic does such things.

> Running it on the current version of the patchset yields mostly warnings on
> string values used in the require “convutils.pm” statement. There were however
> two more interesting reports: one more open() call not using the three
> parameter form and an instance of map which alters the input value.

Sorry for overlooking it.

> The latter
> is not causing an issue since we don’t use the input list past the map but
> fixing it seems like good form.

Agreed.

> Attached is a patch that addresses the perlcritic reports (running without any
> special options).

Thanks. The attached patch contains the patch by perlcritic.

0001,2,3 are Heikki's patch that are not modified since it is
first proposed. It's a bit too big so I don't attach them to this
mail (again).

https://www.postgresql.org/message-id/08e7892a-d55c-eefe-76e6-7910bc8dd1f3@iki.fi

0004 is radix-tree stuff, applies on top of the three patches
above.

There's a hidden fifth patch which of 20MB in size. But it is
generated by running make in the Unicode directory.

[$(TOP)]$ ./configure ...
[$(TOP)]$ make
[Unicode]$ make
[Unicode]$ make distclean
[Unicode]$ git add .
[Unicode]$ commit
=== COMMITE MESSSAGE
Replace map files with radix tree files.

These encodings no longer uses the former map files and uses new radix
tree files. All existing authority files in this directory are removed.
===

regards,