[mkgmap-dev] [mkgmap-svn] Commit r4811: fix java.lang.AssertionError while building index from unicode tiles

Thu Nov 18 09:06:06 GMT 2021

Hi Gerd

While looking at the mdr output from Mapinstall and the differences in
the street repeat flags, mainly where there are shield/road-ref
entries, I realised my change to Sort.java for undefined sortOrder went
to far and it disrupts this ordering for Unicode.

Missing or 0 sortOrder (anything before the first "<" in sort/cp*.txt)
means the character is ignored by the key sort and collator.compare(),
so TERTIARY and EQUAL can give different answers.  

Attached is mdrUnicode_v9a.patch that generates a fake sortOrder only
for rows where no sortOrders are defined, instead of for anything in
Unicode with no/0 sortOrder.

With this patch it becomes possible for Unicode to trigger the original
problem again. Attached is mdrUnicode_v9b.patch that has the fix for
this from the previous patch.

Ticker

On Tue, 2021-11-09 at 18:13 +0000, Ticker Berkin wrote:
> Hi Gerd
> 
> Sorry about the delay, I'm away at the moment without access to the 
> sources and other tools, but:
> 
> The same pointer size (1-3 bytes) is used by Mdr5 and Mdr25. I can't 
> remember if this is determined by the actual number of Mdr5 entries
> or 
> bigger than Mdr5, then this setting needs to be investigated.
> 
> To see what happens when Mdr25 uses exact name dedupe and Mdr5 uses 
> TERTIARY collator, the best way I can think of is to remove some 
> characters from the sort/code-page such that Mdr5 will consider some 
> cities to be identical in Mdr5, then see what effect this has on 
> Find>Address.
> 
> With Mdr, I can well understand unwillingness to change anything when
> it 
> seems to be working. My attempts to switch to SECONDARY collator,
> which 
> I think is what has to happen eventually, had confusing results
> 
> Ticker
> 
> 
> On 07/11/2021 09:16, Gerd Petermann wrote:
> > Hi Ticker,
> > 
> > >   there is no logical change in behaviour to Mdr5 with this
> > > patch.
> > yes, I already wrote it's only refactoring.
> > I still try to find some "real OSM" input where the changes to the
> > other files show any difference in the mdr file.
> > 
> > I found this very old svn log entry by Steve:
> > https://www.mkgmap.org.uk/websvn/revision.php?repname=mkgmap&rev=3093
> > which says that "All cases where sorted names are compared with
> > equals() need to be changed to use Collator.compare() with the
> > SrtCollator from Sort."
> > 
> > So, I really wonder why he didn't change all  sources accordingly.
> > Maybe it was only about cities, maybe not.
> > I see only one way to find out: Have an input that has shows
> > differences in the index and find out which version gives better
> > results in the search in MapSource or - if testing is possible - on
> > a device.
> > 
> > Gerd
> > 
> > ________________________________________
> > Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im Auftrag
> > von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> > Gesendet: Samstag, 6. November 2021 18:46
> > An: Development list for mkgmap
> > Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4811: fix
> > java.lang.AssertionError while building index from unicode tiles
> > 
> > Hi Gerd
> > 
> > I don't know if Mdr5 was correct before, but, for European names at
> > least, I presume it has been used quite a lot and no complaints.
> > 
> > If the default strength for a Collator is TERTIARY, there is no
> > logical
> > change in behaviour to Mdr5 with this patch.
> > 
> > Ticker
> > 
> > On Sat, 2021-11-06 at 14:33 +0000, Gerd Petermann wrote:
> > > H Ticker,
> > > 
> > > but how do you know that the code in Mdr5 is right (with or
> > > without
> > > the patch)?
> > > 
> > > Gerd
> > > 
> > > ________________________________________
> > > Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im
> > > Auftrag
> > > von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> > > Gesendet: Samstag, 6. November 2021 15:29
> > > An: Development list for mkgmap
> > > Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4811: fix
> > > java.lang.AssertionError while building index from unicode tiles
> > > 
> > > Hi Gerd
> > > 
> > > I just hate the idea it is obviously wrong and trivially to make
> > > correct. It is unlikely it will ever fail. While it is in this
> > > inconsistent state it inhibits any addressing of the more
> > > complicated
> > > issue about how to tackle the case-differences in city names and
> > > what
> > > MdrCheck is reporting.
> > > 
> > > Ticker
> > > 
> > > On Sat, 2021-11-06 at 14:10 +0000, Gerd Petermann wrote:
> > > > Hi Ticker,
> > > > 
> > > > ok, so let's wait for that crash to get an example. I really
> > > > have
> > > > no
> > > > idea how to force it :(
> > > > According to MdrCheck the index is full of errors for the two
> > > > tiles
> > > > in China, but maybe it's MdrCheck which is wrong.
> > > > 
> > > > Gerd
> > > > 
> > > > ________________________________________
> > > > Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im
> > > > Auftrag
> > > > von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> > > > Gesendet: Samstag, 6. November 2021 12:27
> > > > An: mkgmap-dev at lists.mkgmap.org.uk
> > > > Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4811: fix
> > > > java.lang.AssertionError while building index from unicode
> > > > tiles
> > > > 
> > > > Hi Gerd
> > > > 
> > > > The crash in Carlos's original problem is due to the
> > > > inconsistency
> > > > in
> > > > the dedups between Mdr5/Mdr25.
> > > > 
> > > > This could be triggered with any --code-page where city names
> > > > contain
> > > > characters that exist in this character-set but are not given
> > > > sort
> > > > positions.
> > > > 
> > > > My mistake with mdrUnicode_v1/2.patch was trying to tackle the
> > > > case
> > > > problem at the same time. This is going to be much more
> > > > difficult.
> > > > 
> > > > Ticker
> > > > 
> > > > 
> > > > On Fri, 2021-11-05 at 11:00 +0000, Gerd Petermann wrote:
> > > > > Hi Ticker,
> > > > > 
> > > > > sorry for my reluctance. I simply have no test case that
> > > > > shows an
> > > > > error (search for xyz not working). If you have one please
> > > > > share
> > > > > it
> > > > > so that I can understand the importance of the patch.
> > > > > 
> > > > > I would also be happy if you could create a new branch to
> > > > > test
> > > > > further changes reg. --lower-case.
> > > > > According to MdrCheck we also produce wrong data for mdr 27
> > > > > (cities
> > > > > are not de-duped).
> > > > > Found this also with Arndts *.img data but did not yet try to
> > > > > find
> > > > > out if MdrCheck is right.
> > > > > 
> > > > > Gerd
> > > > > 
> > > > > ________________________________________
> > > > > Von: mkgmap-dev <mkgmap-dev-bounces at lists.mkgmap.org.uk> im
> > > > > Auftrag
> > > > > von Ticker Berkin <rwb-mkgmap at jagit.co.uk>
> > > > > Gesendet: Freitag, 5. November 2021 11:34
> > > > > An: mkgmap-dev at lists.mkgmap.org.uk;
> > > > > mkgmap-svn at lists.mkgmap.org.uk
> > > > > Betreff: Re: [mkgmap-dev] [mkgmap-svn] Commit r4811: fix
> > > > > java.lang.AssertionError while building index from unicode
> > > > > tiles
> > > > > 
> > > > > Hi Gerd
> > > > > 
> > > > > I really don't like the idea of the consistent dedup part of
> > > > > this
> > > > > patch
> > > > > not being applied (Mdr5 & Mdr25).
> > > > > The Mdr7 changes are a slight refactor and a useful comment,
> > > > > but
> > > > > has
> > > > > no
> > > > > logical effect.
> > > > > The Mdr29 changes are an assert to detect inconsistency in
> > > > > Mdr5/25
> > > > > index pointer lengths before these could cause a crash +
> > > > > .equal()/collator.compare() change that could be removed
> > > > > 
> > > > > Ticker
> > > > > 
> > > > > On Fri, 2021-11-05 at 09:02 +0000, svn commit wrote:
> > > > > > Version mkgmap-r4811 was committed by gerd on Fri, 05 Nov
> > > > > > 2021
> > > > > > 
> > > > > > fix java.lang.AssertionError while building index from
> > > > > > unicode
> > > > > > tiles
> > > > > > Changes extracted from mdrUnicode_v8.patch by Ticker Berkin
> > > > > > 
> > > > > > http://www.mkgmap.org.uk/websvn/revision.php?repname=mkgmap&rev=4811
> > > > > > _______________________________________________
> > > > > > mkgmap-svn mailing list
> > > > > > To unsubscribe send an mail to
> > > > > > mkgmap-svn-leave at lists.mkgmap.org.uk
> > > > > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-svn
> > > > > 
> > > > > 
> > > > > _______________________________________________
> > > > > mkgmap-dev mailing list
> > > > > mkgmap-dev at lists.mkgmap.org.uk
> > > > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > > > _______________________________________________
> > > > > mkgmap-dev mailing list
> > > > > mkgmap-dev at lists.mkgmap.org.uk
> > > > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > > 
> > > > 
> > > > _______________________________________________
> > > > mkgmap-dev mailing list
> > > > mkgmap-dev at lists.mkgmap.org.uk
> > > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > > _______________________________________________
> > > > mkgmap-dev mailing list
> > > > mkgmap-dev at lists.mkgmap.org.uk
> > > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > 
> > > 
> > > _______________________________________________
> > > mkgmap-dev mailing list
> > > mkgmap-dev at lists.mkgmap.org.uk
> > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > _______________________________________________
> > > mkgmap-dev mailing list
> > > mkgmap-dev at lists.mkgmap.org.uk
> > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > 
> > 
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev at lists.mkgmap.org.uk
> > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev at lists.mkgmap.org.uk
> > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > 
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mdrUnicode_v9b.patch
Type: text/x-patch
Size: 8404 bytes
Desc: not available
URL: <http://www.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20211118/09341fa1/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mdrUnicode_v9a.patch
Type: text/x-patch
Size: 975 bytes
Desc: not available
URL: <http://www.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20211118/09341fa1/attachment-0003.bin>