Fix incorrect resizing of node cache in geometry processor #322
Conversation
|
opened #323 to cover the left-over issues |
|
Passes No performance change on a 331 MB PBF pgsql backend with or without |
geometry-processor.cpp
Outdated
There was a problem hiding this comment.
Hmm, the return value of nodes_get_list() is ignored here, which is not good. The function returns the number of nodes actually copied in. If the geometry of some nodes is missing (think OSM data extract) the return value may be smaller than node_count. So there really is a second resize() necessary after the call that uses the return value or the vector still might be too large.
There was a problem hiding this comment.
How's this for a test: First way in file is nodes 1,2,3; second is 4,5; third is 6,7,8 with node 7 missing. I think that catches both.
There was a problem hiding this comment.
testing with this XML reveals that the multi-backend doesn't work at all with incomplete extracts (i.e. extracts where not all nodes referenced by ways are present)
<osm version="0.6">
<node id="1" lat="49.00" lon="-123.00"/>
<node id="2" lat="49.00" lon="-123.01"/>
<node id="3" lat="49.01" lon="-123.02"/>
<node id="4" lat="49.02" lon="-123.00"/>
<node id="5" lat="49.02" lon="-123.01"/>
<node id="6" lat="49.03" lon="-123.00"/>
<node id="8" lat="49.03" lon="-123.02"/>
<way id="1">
<nd ref="1"/>
<nd ref="2"/>
<nd ref="3"/>
</way>
<way id="2">
<nd ref="4"/>
<nd ref="5"/>
</way>
<way id="3">
<nd ref="6"/>
<nd ref="7"/>
</way>
</osm>
It errors on a copy statement trying to copy SRID=900913;LINESTRING (-13692297.3599999994039536 6279953.2999999998137355, nan nan, -13694523.7500... as a geometry.
The same file works with -O pgsql --slim --hstore
So a good catch.
|
📦 |
There was a bug where node_cache was not properly resized when the current way was smaller than previous ways or a way was missing nodes (i.e. incomplete extract). This fixes osm2pgsql-dev#274 and adds a testcase. This entire section of code should be rewritten to use std::vector everywhere.
There was a problem hiding this comment.
std::vector is linear with erased items, so this should be fast if node_cache.size() = node_count
|
Updated, still passes |
Fix incorrect resizing of node cache in geometry processor
3 participants
This is the quick fix to #274, but includes a testcase.
With the testcase, fixing it properly would make a reasonable place for someone to start with osm2pgsql.