At Wed, 1 Mar 2017 14:34:23 +0900, Michael Paquier <michael.paqu...@gmail.com> wrote in <CAB7nPqQ_4n+FDWi5Xiueo68i=fTmdg1Wx+y6XWWX=8rahkr...@mail.gmail.com> > On Tue, Feb 28, 2017 at 5:34 PM, Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote: > > At Tue, 28 Feb 2017 15:20:06 +0900, Michael Paquier > > <michael.paqu...@gmail.com> wrote in > > <cab7npqr49krgp6qaakal2v3hcnn+dnzv8dq_ysgbdsr6b_y...@mail.gmail.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.
Ouch! You're right. Sorry for my short-sight. char_converter.c is removed and related description in Makefile is removed. > > 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.. I should have reverted there but actually the patch somehow does the different thing.. Surely reverted it this time. > 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. Agreed. > 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. I don't vote 3 and 4. And I did 1 in the last patch. About 2, any change in the authority files rarely but possiblly happens. Even in the case the plain map files are no longer can be generated. (but can with a bit tweak of convutils.pm) If the radix-tree file generator is under a suspicion, the "plain" map file generator (and the map_checker) or some other means to sanity check might be required. That being said, when something occurs in radix files, we can find it in the radix file and can find the corresponding lines in the aurhority file. The remaining problem is the case where some substantial change in authority files doesn't affect radix files. We can detect such mistake by detecting changes in authority files. So I propose the 5th option. 5) Just remove plain map files and all related code. Addition to that, Makefile stores hash digest of authority files in Unicode/authoriy_hashes.txt or something that is managed by git. This digest may differ among platforms (typically from cr/nl difference) but we can assume *nix for the usage. I will send the next version after this discussion is settled. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers