[Tile-serving] [osm2pgsql-dev/osm2pgsql] Add grouped-linemerge generalization (PR #2482)

[Sarah Hoffmann]

@lonvia commented on this pull request.

The mix of tile expiry and recursive query is interesting. I actually think that the solution is simpler because of your use of tile-based expiry instead of way-based. I did an implementation with way-based expiry for [waymarkedtrails](https://github.com/waymarkedtrails/osgende/blob/master/osgende/lines/segments.py) and only got it to work by tracking the IDs of the source ways for each merged way. That would make this implementation significantly more complex.

ST_LineMerge has a few quirks which might lead to unexpected results but that is something that can be easily addressed later.

Performance stands an falls with the complexity of the merged ways, which in turn depends on the source table and the grouping. I'd like to run the import query on a planet to see how long it takes. That should give us a ballpark figure how expensive merging is.

> +    // endpoint equality, not ST_Intersects, to avoid deleting unrelated
+    // same-group lines that cross a component. A second pass over the changed
+    // region cleans up outputs of components that disappeared entirely (all
+    // their lines deleted), which leave no reached nodes behind; that pass is
+    // self-correcting because anything it removes either vanished or has a
+    // surviving line in the region and is regenerated below.
+    timer(m_timer_delete).start();
+    auto deleted = dbexec(R"(
+DELETE FROM {dest} d
+ USING _glm_nodes n
+ WHERE {group_join_dn}
+   AND ( (ST_X(ST_StartPoint(d."{geom_column}")) = n.x
+      AND ST_Y(ST_StartPoint(d."{geom_column}")) = n.y)
+     OR (ST_X(ST_EndPoint(d."{geom_column}")) = n.x
+      AND ST_Y(ST_EndPoint(d."{geom_column}")) = n.y) )
+)");

Is this delete really necessary? Are there any cases of updating that wouldn't be covered by the DELETE below in line335?

> +    }
+}
+
+void gen_grouped_linemerge_t::process_create()
+{
+    if (get_params().get_bool("create_indexes", true)) {
+        log_gen("Creating endpoint indexes on source table...");
+        dbexec(
+            R"(CREATE INDEX IF NOT EXISTS "{idx_startpt}" ON {src} USING btree)"
+            R"( (ST_X(ST_StartPoint("{geom_column}")),)"
+            R"( ST_Y(ST_StartPoint("{geom_column}"))) {index_predicate})");
+        dbexec(
+            R"(CREATE INDEX IF NOT EXISTS "{idx_endpt}" ON {src} USING btree)"
+            R"( (ST_X(ST_EndPoint("{geom_column}")),)"
+            R"( ST_Y(ST_EndPoint("{geom_column}"))) {index_predicate})");
+    }

I understand your reasoning for an extra index here but I think this still could be a geometry index on the points, i.e.

```
CREATE INDEX ... USING gist(ST_StartPoint(geometry))
```

The you can use ST_Intersect on the start and end points below making the code a lot more readable.

> +    CASE WHEN ST_X(ST_StartPoint(l."{geom_column}")) = n.x
+          AND ST_Y(ST_StartPoint(l."{geom_column}")) = n.y
+         THEN ST_X(ST_EndPoint(l."{geom_column}"))
+         ELSE ST_X(ST_StartPoint(l."{geom_column}")) END,
+    CASE WHEN ST_X(ST_StartPoint(l."{geom_column}")) = n.x
+          AND ST_Y(ST_StartPoint(l."{geom_column}")) = n.y
+         THEN ST_Y(ST_EndPoint(l."{geom_column}"))
+         ELSE ST_Y(ST_StartPoint(l."{geom_column}")) END
+    FROM nodes n
+    JOIN {src} l
+      ON {group_join}
+     AND ( (ST_X(ST_StartPoint(l."{geom_column}")) = n.x
+        AND ST_Y(ST_StartPoint(l."{geom_column}")) = n.y)
+       OR (ST_X(ST_EndPoint(l."{geom_column}")) = n.x
+        AND ST_Y(ST_EndPoint(l."{geom_column}")) = n.y) )
+     AND {where}

One way to make this recursive CTE cheaper is to join here only with nodes added in the last recursion. You need to be very, very careful though not to create endless loops when doing that. So maybe an optimization for later rather than now.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/osm2pgsql-dev/osm2pgsql/pull/2482#pullrequestreview-4437753493
You are receiving this because you are subscribed to this thread.

Message ID: <osm2pgsql-dev/osm2pgsql/pull/2482/review/4437753493 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20260605/be24b1c6/attachment-0001.htm>