Skip to content

MergeInputReader: take object type into account when applying changes - #321

Closed
Wahaj18 wants to merge 1 commit into
osmcode:masterfrom
Wahaj18:fix-318-merge-same-id
Closed

Wahaj18 wants to merge 1 commit into
osmcode:masterfrom
Wahaj18:fix-318-merge-same-id

Conversation

@Wahaj18

@Wahaj18 Wahaj18 commented Oct 12, 2026

Copy link
Copy Markdown

MergeInputReader.apply_to_reader() only compared object IDs when picking the newest version of each object, so an object whose ID matched the previous object of a different type was dropped. In the example from #318, relation/1 from the change file disappeared because way/1 came right before it.

The check now looks at both type and ID, the same way the simplify branch of apply() already does and the way osmium-tool's apply-changes was fixed for the same bug.

Added a regression test with a node, way and relation that all have ID 1. It fails on master (the relation is missing from the output) and passes with the change. Full test suite passes locally.

Fixes #318.

OSM IDs are only unique per object type, so a changed relation with the
same ID as a way or node in the input was dropped by apply_to_reader().

Fixes osmcode#318.
@lonvia

lonvia commented Oct 12, 2026

Copy link
Copy Markdown
Member

We don't take AI mass PRs.

@lonvia lonvia closed this Oct 12, 2026
@Wahaj18
Wahaj18 deleted the fix-318-merge-same-id branch October 12, 2026 07:09
@clarisma

Copy link
Copy Markdown
Contributor

Same fix as osmcode/osmium-tool@f8ba3b3.
I like the concise unit test.
I'd take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

MergeInputReader drops objects when IDs collide across OSM types

3 participants