Re: Radix tree for character conversion

On Tue, Feb 28, 2017 at 5:34 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> At Tue, 28 Feb 2017 15:20:06 +0900, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote in <CAB7nPqR49krGP6qaaKaL2v3HCnn+dnzv8Dq_ySGbDSr6b_ywrw(at)mail(dot)gmail(dot)com>
>> +conv.o: conv.c char_converter.c
>> This also can go away.
>
> Touching char_converter.c will be ignored if it is removed. Did
> you mistake it for map_checker?

That was not what I meant: as pg_mb_radix_conv() is only used in
conv.c, it may be better to just remove completely char_converter.c.

> And the code-comment pointed in the comment by the previous mail
> is rewritten as Robert's suggestion.

Fine for me.

-distclean: clean
+distclean:
rm -f $(TEXTS)

-maintainer-clean: distclean
- rm -f $(MAPS)
-
+maintainer-clean:
+ rm -f $(TEXTS) $(MAPS)
Well, I would have assumed that this should not change..

The last version of the patch looks in rather good shape to me, we are
also sure that the sanity checks on the old maps and the new maps
match per the previous runs with map_checker. One thing that still
need some extra opinions is what to do with the old maps:
1) Just remove them, replacing the old maps by the new radix tree maps.
2) Keep them around in the backend code, even if they are useless.
3) Use a GUC to be able to switch from one to the other, giving a
fallback method in case of emergency.
4) Use an extension module to store the old maps with as well the
previous build code, so as sanity checks can still be performed on the
new maps.

I would vote for 2), to reduce long term maintenance burdens and after
seeing all the sanity checks that have been done in previous versions.
--
Michael