Import a Placemark's geometry as one feature per type, not one per part

[njakobsen]

Importers::KML emits one feature per <Polygon>, <LineString> or <Point> element. Software that models an object out of many faces puts every face in one Placemark's <MultiGeometry>, so a Placemark becomes hundreds of rows. On one production record, 2,661 Placemarks became 136,769 features — a third of that deployment's entire features table, at 4.7 vertices per feature.

The row count is what costs. Each feature takes a savepoint and an ST_GeomFromKML round trip to build, then a validation query, an ST_Force2D and an insert to save.

ST_GeomFromKML accepts a whole <MultiGeometry> in one call, and returns one multipart geometry. That is the entire mechanism this issue is about.

Measurements

One file, 33.8 MB, 76,190 geometries in 1,233 Placemarks. Geometry conversion only:

Approach	Features	Conversion
Today, one call per part	76,190	~456s (5.99 ms/call measured over 300 calls, × 76,190)
ogr2ogr on the cleaned document	1,233	1.13s, plus 0.27s to parse, strip and serialise
ST_GeomFromKML per Placemark	1,233	1.23s
ST_GeomFromKML per (Placemark, geometry type)	1,233	1.86s

Across the whole record, grouping takes 136,769 features to 2,661, a 51x reduction, and takes the per-feature save cost down with it.

The conversion figures above were measured in isolation and are reliable. An end-to-end harness over all four files of that record is not quoted here: holding six figures of parsed features in one process put it under enough GC pressure that identical parse work timed 29.70s in one pass and 48.84s in another, which says more about the harness than about either approach.

This issue previously proposed reading KML through GDAL. The diagnosis was right and the remedy was wrong: the 60x row reduction was doing the work and GDAL was incidental to it. Staying in Ruby matches ogr2ogr on speed without shelling out. What that route would and would not have bought is kept as a note at the end.

There is consequently no separate interim work worth doing. Batching ST_GeomFromKML as relief ahead of a larger migration was considered; the grouping change is the fix, not a stand-in for one.

Recommended shape

Group per (Placemark, geometry type) rather than per Placemark.

Grouping per Placemark alone returns GEOMETRYCOLLECTION when a Placemark mixes types, and the feature_type generated column maps that to 'polygon', so a mixed Placemark would be mistyped. Grouping by type instead returns ST_Point, ST_LineString, ST_MultiPolygon and ST_MultiLineString, and no collections.

This is insurance rather than a fix for an observed problem: 0 of the 1,233 Placemarks in that file mix types. It costs 0.6s of the 1.86s, which is worth paying to remove the failure mode rather than to correct a live one.

each_record already iterates Placemarks and reads their geometry with placemark.css(...) after #66, so this change is to what is handed to ST_GeomFromKML, not to the traversal.

What grouping does to the geometry

For legitimate multipart geometry, grouping is lossless. Disjoint parts parse as a valid MultiPolygon, are never repaired, and lose no vertices:

two disjoint parcels    valid=true   vertices 10 -> 10   MultiPolygon
three disjoint parcels  valid=true   vertices 15 -> 15   MultiPolygon
two overlapping         valid=false  vertices 10 ->  9   Polygon

Only overlapping parts come back invalid, and the files that produce them are CAD models, where the facets of a 3D structure overlap in plan view. An OGC MULTIPOLYGON requires its parts to have disjoint interiors, so a grouped mesh fails validation with Self-intersection or Too few points in geometry component.

That is not fatal, because has_spatial_features sets make_valid: true by default, so AbstractFeature#validate_geometry repairs an invalid geometry with ST_Buffer(geog, 0) and re-checks it. Both paths store every feature. What differs is what repair does to each. Measured over 80 polygon-bearing Placemarks from a 33.8 MB CAD-derived KML:

Path	Features	Repaired	Invalid after repair	Vertices	After repair
Per part (today)	8,443	452	0	39,311	37,051 (94.3% kept)
Grouped	80	80	0	39,311	5,265 (13.4% kept)

Repairing an individual triangle mostly leaves it alone. Repairing a grouped mesh dissolves it into its outline, and roughly seven of every eight vertices go with it.

This is contingent, not inherent. It happens only because CAD models are imported as geometry in the first place. #69 proposes detecting them in the existing Nokogiri pre-pass and reporting them the way a map image is reported, rather than importing their facets. With that in place, grouping never meets an overlapping part, and this change is what it appears to be: the same shapes in fewer rows, with no geometry change at all.

The table above is therefore the fallback case — what grouping does if it lands without detection. It is not an argument against grouping so much as a reason to sequence #69 first.

Record-level area is unaffected either way. features_area derives from ST_Area(ST_Union(...)), and the union of the parts and the union of the grouped features agree to 0.0000% (15,417.31 m² both ways) on that sample. Per-feature area does change, since today overlapping triangles double-count it: the sum of per-feature areas is 15,767.24 m² across parts against 15,434.61 m² grouped. Anything reading the union is safe; anything reading or summing per-feature area is not.

The decision: this changes what a feature is

Every record on every deployment re-imports, and its features come back renumbered and regrouped. Feature ids move, so the MVT tile caches and proximity caches keyed on them are invalidated. Anything a client has recorded against a feature id no longer resolves.

That is identical whichever engine does the grouping, and it is the decision this issue exists to force. It needs a deliberate rollout — an ordering, a re-import window, and a view on what happens to cached derivatives — not a gem bump.

That decision is @njakobsen's and Ryan's, and the measurements above do not settle it.

Worth weighing on the other side: 136,769 rows describing a drill rig's faces is not a mapped area anyone asked for, and every one of them is carried in perpetuity by every query that touches that record.

features_hash is not at risk. Importers::Base#cache_key is Digest::MD5.hexdigest(@data), over the source bytes rather than the emitted features, so regrouping cannot invalidate it.

Note: what reading through GDAL would have bought, and why it was dropped

Kept because it is tested, and because it should stop ogr2ogr being re-proposed for KML.

Against it: reprojection is not needed, as KML is always EPSG:4326; KMZ reading is something the importer already does by unzipping; and format breadth is an argument for the shapefile and ESRI JSON importers, which already shell out, not for KML. It costs a tempfile and a serialisation pass, and it loses ExtendedData — a <SimpleData name="pdfmaps_photos"> inside <SchemaData> did not appear in OGR's properties at all in testing, which would take importable_image_paths and the image handlers with it. Staying in Ruby keeps our own metadata extraction, including the nested CDATA table parsing OGR has no equivalent for. That is why ExtendedData is no longer on the blocker list.

The GroundOverlay findings below are moot for the grouping path, since the existing Nokogiri pre-pass removes overlays before anything else runs. They apply if the shapefile or GeoJSON side ever wants similar treatment.

Filtering GDAL's output on the icon attribute does not work: a GroundOverlay with no <Icon>, or an empty one, produces a feature where icon is absent from the layer schema rather than null, indistinguishable by attribute from a real Placemark polygon. --config LIBKML_READ_GROUND_OVERLAY FALSE suppresses them properly, but suppressed overlays cannot then be named, and it does not apply to PhotoOverlay or ScreenOverlay. Features emitted, against three synthetic fixtures:

Fixture	ogr2ogr	LIBKML_READ_GROUND_OVERLAY FALSE	Strip in XML, then ogr2ogr	Names available
1 Placemark + 1 GroundOverlay with <Icon>	2	1	1	yes
2 GroundOverlays, one with no <Icon>, one empty	2	0	0	yes
1 Placemark + PhotoOverlay + ScreenOverlay	1	1	1	yes

The third row shows the driver never surfaces PhotoOverlay or ScreenOverlay as features at all, so the config option is beside the point for them while the warning still has to name them. Stripping in XML first was the answer, and it is what the importer already does.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv

[njakobsen]

Rewritten. The issue previously proposed reading KML through GDAL; it now proposes grouping a Placemark's geometry into one ST_GeomFromKML call per geometry type, and the title changed with it.

What prompted it: @njakobsen asked what ogr2ogr was actually doing for us that Ruby could not, given the importer already parses the whole document. Testing that, ST_GeomFromKML turns out to accept a whole <MultiGeometry> in one call, which matches ogr2ogr on speed for this file (1.23s against 1.13s) at the same granularity, without a tempfile, a serialisation pass, or the loss of ExtendedData.

The original diagnosis stands unchanged: row inflation is the cost. Only the remedy was wrong.

Three corrections to what the issue said before:

• The Placemark count for that record was given as 2,413. It is 2,661, which reconciles exactly with the 136,769 features in production.
• ExtendedData is no longer a blocker. It was only ever at risk because OGR drops it, and the importer no longer leaves Ruby.
• Layer naming is no longer a consideration, since nothing writes an intermediate file.

Unchanged, and still the reason the issue is open: grouping changes what a feature is, so every record re-imports with renumbered features and the caches keyed on feature ids are invalidated. That needs a rollout decision.

The GDAL and GroundOverlay measurements are kept at the end of the issue rather than deleted. They are tested, and their remaining value is stopping ogr2ogr being re-proposed for KML.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv

[njakobsen]

Correcting a figure I circulated before it reached this issue, and adding what re-measurement showed.

What I claimed. A prototype of the grouping built 2,661 features and saved only 1,449, and a validity check put grouped geometries at 0 valid against 80 invalid. I read that as grouping producing geometry the model would reject.

What was wrong with it. has_spatial_features sets make_valid: true by default (has_spatial_features.rb:7), which threads through update_features! to each importer and onto every feature, so validate_geometry repairs an invalid geometry with ST_Buffer(geog, 0) and re-checks rather than rejecting it. My harness constructed Feature objects directly and never set it, so nothing was repaired. The rejection rate was an artifact of the harness, and the validity comparison was measuring pre-repair validity when production repairs on import. Re-measured with repair on both paths, both store 100% of their features.

What stands, and is the actual finding. Repair does very different things to the two shapes. Over 80 polygon-bearing Placemarks:

Path	Features	Repaired	Invalid after	Vertices	After repair
Per part (today)	8,443	452	0	39,311	37,051 (94.3% kept)
Grouped	80	80	0	39,311	5,265 (13.4% kept)

Repairing a triangle mostly leaves it alone. Repairing a grouped mesh dissolves it into its outline. So grouping changes the stored geometry from a mesh to a footprint, which is a product decision rather than a performance one, and the issue now leads with it.

One reassurance that came out of the re-measurement. Record-level area is unaffected: features_area derives from ST_Area(ST_Union(...)), and the union of the parts and the union of the grouped features agree to 0.0000% on that sample. Per-feature area does change, since today overlapping triangles double-count it.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv

[njakobsen]

Reframed the geometry section against #69.

The mesh-versus-footprint question this issue was carrying turns out to be contingent rather than inherent. Grouping is lossless for legitimate multipart geometry — disjoint parts parse as a valid MultiPolygon, are never repaired, and lose no vertices. Only overlapping parts are invalid, and the only files producing them are CAD models, whose facets overlap in plan view.

So the footprint outcome is what grouping does if it lands without detection. #69 proposes detecting those files in the pre-pass and reporting them the way a map image is reported. With that first, this change is the same shapes in fewer rows and no geometry change.

The repair table is kept rather than deleted, reframed as the fallback case, so the consequence of sequencing them the other way round stays visible.

This issue now carries one decision: the re-import rollout. The question of what a CAD upload should become moved to #69, where the submitter-facing side of it belongs.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv