integer conversion issue in RelationsMapStash / RelationsMapIndex

VeerleVanbelleghem-TomTom commented 1 week ago

What version of libosmium are you using?

latest (checked out main)

What operating system and compiler are you using?

ubuntu:24.10

What did you do exactly?

Using get ids to filter: "osmium getid --add-referenced ... r15937701"

Example of input:

<relation id="15937701" version="1" timestamp="2024-08-29T00:00:00Z" uid="1" changeset="1">
    <member type="node" ref="5" role="label"/>
    <member type="way" ref="6" role="outer"/>
    <tag k="e" v="f"/>
  </relation>
  <relation id="3819290742" version="1" timestamp="2024-08-29T00:00:00Z" uid="1" changeset="1">
    <member type="node" ref="1" role="label"/>
    <member type="way" ref="2" role="outer"/>
    <tag k="c" v="d"/>
  </relation>
  <relation id="4310904997" version="1" timestamp="2024-08-29T00:00:00Z" uid="1" changeset="1">
    <member type="node" ref="2" role="label"/>
    <member type="way" ref="4" role="outer"/>
    <member type="relation" ref="3819290742" role="rel"/>
    <tag k="a" v="b"/>
  </relation>

Output:

  <relation id="15937701" version="1" timestamp="2024-08-29T00:00:00Z" uid="1" changeset="1">
    <member type="node" ref="5" role="label"/>
    <member type="way" ref="6" role="outer"/>
    <tag k="e" v="f"/>
  </relation>
  <relation id="3819290742" version="1" timestamp="2024-08-29T00:00:00Z" uid="1" changeset="1">
    <member type="node" ref="1" role="label"/>
    <member type="way" ref="2" role="outer"/>
    <tag k="c" v="d"/>
  </relation>

What did happen instead?

Relation with id="15937701" should be in the output (it is in the filter), but relation with id "3819290742" should not. The reason why it is in, is that it is a member of relation with id "4310904997". In RelationsMapStash: the id 4310904997 has an overflow, it is converted from u64 to u32: from 4310904997 to 15937701

What did you do to try analyzing the problem?

I changed all appearances of uint32_t to uint64_t in relations_map.hpp and it solves my problem. I didn't look at the impact on the rest of the code.

joto commented 4 days ago

Osmium was created for working with OSM data and is optimized for that. There are no relation ids in OSM that need 64 bit (and there wont be for a long time), so that's why the code doesn't handle this case. Using 32 bit ids here saves a lot of memory, so that's what we do.

This is not the first time issues like this have come up here, and often from TomTom employees. I am not against adding support for non-OSM data to Osmium, but there are tradeoffs involved and we might have to write the code in a way that it works in those cases while still optimizing for the more common case. All of that needs effort and time and somebody has to pay for that. I have said that several times already. So this is unlikely to get any work done unless somebody is willing to step up and pay for development.

/cc @salimbaidoun-tomtom

VeerleVanbelleghem-TomTom commented 2 days ago

I am really sorry, I thought I was being help full (since there other commits about large numbers and since there is wrong casting in the code, when ids are large). I will close the issue. There is no need to solve the problem.

VeerleVanbelleghem-TomTom commented 2 days ago

Closed this, see my comment.

osmcode / libosmium