improvements in Unicode tables generation code

At Tue, 22 Jun 2021 09:20:16 +0200, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in

I have accumulated a few patches to improve the output of the scripts
in src/backend/utils/mb/Unicode/ to be less non-standard-looking and
fix a few other minor things in that area.

v1-0001-Make-Unicode-makefile-more-parallel-safe.patch

The makefile rule that calls UCS_to_most.pl was written incorrectly
for parallel make. The script writes all output files in one go, but
the rule as written would call the command once for each output file
in parallel.

I was annoyed by that behavior but haven't found how to stop that. It
looks to work. (But I haven't run it for me for the reason at the end
of this mail.)

v1-0002-Make-UCS_to_most.pl-process-encodings-in-sorted-o.patch

This mainly just helps eyeball the output while debugging the previous
patch.

v1-0003-Remove-some-whitespace-in-generated-C-output.patch

Improve a small formatting issue in the output.

These look just fine.

v1-0004-Simplify-code-generation-code.patch

This simplifies the code a bit, which helps with the next patch.

This simplifies the code in exchange of allowing a comma after the
last element of array literals. I'm fine with it as long as we allow
that style in the tree.

v1-0005-Fix-indentation-in-generated-output.patch

This changes the indentation in the output from two spaces to a tab.

I haven't included the actual output changes in the last patch,
because they would be huge, but the idea should be clear.

All together, these make the output look closer to how pgindent would
make it.

I agree to the fix.

Mmm. (although, somewhat unrelated to this patch set) I tried this but
I found that www.unicode.org doesn't respond (for at least these
several days). I'm not sure what is happening here.

wget -O 8859-2.TXT --no-use-server-timestamps https://www.unicode.org/Public/MAPPINGS/ISO8859/8859-2.TXT
--2021-06-22 17:09:34-- https://www.unicode.org/Public/MAPPINGS/ISO8859/8859-2.TXT
Resolving www.unicode.org (www.unicode.org)... 66.34.208.12
Connecting to www.unicode.org (www.unicode.org)|66.34.208.12|:443...

(timeouts)

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 22/06/2021 10:20, Peter Eisentraut wrote:

I have accumulated a few patches to improve the output of the scripts in
src/backend/utils/mb/Unicode/ to be less non-standard-looking and fix a
few other minor things in that area.

v1-0001-Make-Unicode-makefile-more-parallel-safe.patch

The makefile rule that calls UCS_to_most.pl was written incorrectly for
parallel make. The script writes all output files in one go, but the
rule as written would call the command once for each output file in
parallel.

This could use a comment. At a quick glance, I don't understand what all
the $(wordlist ...) magic does.

Perhaps we should change the script or Makefile so that it doesn't
create all the maps in one go?

v1-0002-Make-UCS_to_most.pl-process-encodings-in-sorted-o.patch

This mainly just helps eyeball the output while debugging the previous
patch.

+1

v1-0003-Remove-some-whitespace-in-generated-C-output.patch

Improve a small formatting issue in the output.

I'm surprised the added \n in the perl code didn't result in extra
newlines in the outputs.

v1-0004-Simplify-code-generation-code.patch

This simplifies the code a bit, which helps with the next patch.

If we do that, let's add the trailing commas to the other arrays too,
not just the combined maps.

No objection, but how does this help the next patch?

If we want to avoid the stray commas (and I think they are a little
ugly, but that's a matter of taste), we could adopt the approach that
print_radix_table() uses to avoid the comma. That seems simpler than
what print_from_utf8_combined_map and print_to_utf8_combined_map are doing.

v1-0005-Fix-indentation-in-generated-output.patch

This changes the indentation in the output from two spaces to a tab.

I haven't included the actual output changes in the last patch, because
they would be huge, but the idea should be clear.

All together, these make the output look closer to how pgindent would
make it.

Thanks!

- Heikki

At Tue, 22 Jun 2021 11:20:46 +0300, Heikki Linnakangas <hlinnaka@iki.fi> wrote in

On 22/06/2021 10:20, Peter Eisentraut wrote:

v1-0004-Simplify-code-generation-code.patch
This simplifies the code a bit, which helps with the next patch.

If we do that, let's add the trailing commas to the other arrays too,
not just the combined maps.

No objection, but how does this help the next patch?

If we want to avoid the stray commas (and I think they are a little
ugly, but that's a matter of taste), we could adopt the approach that
print_radix_table() uses to avoid the comma. That seems simpler than
what print_from_utf8_combined_map and print_to_utf8_combined_map are
doing.

+1 for adopting the same method with print_radix_table *if* we do want
to avoid the stray commans.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 22.06.21 10:20, Heikki Linnakangas wrote:

On 22/06/2021 10:20, Peter Eisentraut wrote:

I have accumulated a few patches to improve the output of the scripts in
src/backend/utils/mb/Unicode/ to be less non-standard-looking and fix a
few other minor things in that area.

v1-0001-Make-Unicode-makefile-more-parallel-safe.patch

The makefile rule that calls UCS_to_most.pl was written incorrectly for
parallel make.  The script writes all output files in one go, but the
rule as written would call the command once for each output file in
parallel.

This could use a comment. At a quick glance, I don't understand what all
the $(wordlist ...) magic does.

Perhaps we should change the script or Makefile so that it doesn't
create all the maps in one go?

I agree, either comment it better or just write one file at a time.
I'll take another look at that.

v1-0003-Remove-some-whitespace-in-generated-C-output.patch

Improve a small formatting issue in the output.

I'm surprised the added \n in the perl code didn't result in extra
newlines in the outputs.

True, I'll have to check that again. I suspect that \n actually belongs
to patch 0004.

v1-0004-Simplify-code-generation-code.patch

This simplifies the code a bit, which helps with the next patch.

If we do that, let's add the trailing commas to the other arrays too,
not just the combined maps.

No objection, but how does this help the next patch?

Mainly it just moves things around so that each print normally starts at
the beginning of a line and concludes with a \n.

On 23.06.21 10:55, Peter Eisentraut wrote:

v1-0001-Make-Unicode-makefile-more-parallel-safe.patch

The makefile rule that calls UCS_to_most.pl was written incorrectly for
parallel make.  The script writes all output files in one go, but the
rule as written would call the command once for each output file in
parallel.

This could use a comment. At a quick glance, I don't understand what
all the $(wordlist ...) magic does.

Perhaps we should change the script or Makefile so that it doesn't
create all the maps in one go?

I agree, either comment it better or just write one file at a time. I'll
take another look at that.

Here is a patch that does it one file (pair) at a time. The other rules
besides UCS_to_most.pl actually had the same problem, since they produce
two output files, so running in parallel called each script twice. In
this patch, all of that is heavily refactored and works correctly now.
Note that UCS_to_most.pl already accepted a command-line argument to
specify which encoding to work with.

On 20.07.21 13:57, Peter Eisentraut wrote:

Perhaps we should change the script or Makefile so that it doesn't
create all the maps in one go?

I agree, either comment it better or just write one file at a time.
I'll take another look at that.

Here is a patch that does it one file (pair) at a time.  The other rules
besides UCS_to_most.pl actually had the same problem, since they produce
two output files, so running in parallel called each script twice.  In
this patch, all of that is heavily refactored and works correctly now.
Note that UCS_to_most.pl already accepted a command-line argument to
specify which encoding to work with.

Here is an updated patch with a thinko fix that made the previous patch
not actually work.

On 28.09.21 10:25, Peter Eisentraut wrote:

On 20.07.21 13:57, Peter Eisentraut wrote:

Perhaps we should change the script or Makefile so that it doesn't
create all the maps in one go?

I agree, either comment it better or just write one file at a time.
I'll take another look at that.

Here is a patch that does it one file (pair) at a time.  The other
rules besides UCS_to_most.pl actually had the same problem, since they
produce two output files, so running in parallel called each script
twice.  In this patch, all of that is heavily refactored and works
correctly now. Note that UCS_to_most.pl already accepted a
command-line argument to specify which encoding to work with.

Here is an updated patch with a thinko fix that made the previous patch
not actually work.

I have committed this one and closed the CF entry.