Re: Radix tree for character conversion

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: daniel(at)yesql(dot)se
Cc: hlinnaka(at)iki(dot)fi, robertmhaas(at)gmail(dot)com, tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, ishii(at)sraoss(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Radix tree for character conversion
Date: 2016-11-04 07:34:33
Message-ID: 20161104.163433.48171709.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thank you for looling this.

At Mon, 31 Oct 2016 17:11:17 +0100, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote in <3FC648B5-2B7F-4585-9615-207A44B730A9(at)yesql(dot)se>
> > On 27 Oct 2016, at 09:23, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > Perl scripts are to be messy, I believe. Anyway the duplicate
> > check as been built into the sub print_radix_trees. Maybe the
> > same check is needed by some plain map files but it would be just
> > duplication for the maps having radix tree.
>
> 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.

I'm not sure how the discussion about this goes, these patches
makes me think about coding style of Perl.

The distinction between executable script and library is by
intention with an obscure basis. Existing scripts don't get less
modification, but library uses more restricted scopes to get rid
of the troubles caused by using global scopes. But I don't have a
clear preference on that. The TAP test scripts takes OO notations
but I'm not sure convutils.pl also be better to take the same
notation. It would be rarely edited hereafter and won't gets
grown any more.

As far as I see the obvious bug fixes in the patchset are the
following,

- 0007: load_maptable fogets to close input file.
- 0010: commment for load_maptables is wrong.
- 0011: hash reference is incorrectly dereferenced

All other fixes other than the above three seem to be styling or
syntax-generation issues and I don't know whether any
recommendation exists...

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-11-04 08:02:07 Re: WAL consistency check facility
Previous Message amul sul 2016-11-04 06:52:15 Re: Exclude pg_largeobject form pg_dump