From 36cf0eb922d4c03e73a3a3a5505e46d5cee061b9 Mon Sep 17 00:00:00 2001
From: Sarah Hoffmann <lonvia@denofr.de>
Date: Mon, 14 Nov 2022 13:57:26 +0100
Subject: [PATCH] reorganize handling of place type changes

Always replace existing entries in place, never delete them because
a direct delete will cause conflicts.
---
 lib-sql/functions/place_triggers.sql   | 37 ++++++++++++++------
 test/bdd/osm2pgsql/update/tags.feature | 47 ++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/lib-sql/functions/place_triggers.sql b/lib-sql/functions/place_triggers.sql
index 5b2642e7..ca6ba690 100644
--- a/lib-sql/functions/place_triggers.sql
+++ b/lib-sql/functions/place_triggers.sql
@@ -47,8 +47,6 @@ BEGIN
 
   {% if debug %}RAISE WARNING 'Existing: %',existing.osm_id;{% endif %}
 
-  -- Handle a place changing type by removing the old data.
-  -- (This trigger is executed BEFORE INSERT of the NEW tuple.)
   IF existing.osm_type IS NULL THEN
     DELETE FROM place where osm_type = NEW.osm_type and osm_id = NEW.osm_id and class = NEW.class;
   END IF;
@@ -192,15 +190,11 @@ BEGIN
     END IF;
     {% endif %}
 
-    IF existing.osm_type IS NOT NULL THEN
-      -- Pathological case caused by the triggerless copy into place during initial import
-      -- force delete even for large areas, it will be reinserted later
-      UPDATE place SET geometry = ST_SetSRID(ST_Point(0,0), 4326)
-        WHERE osm_type = NEW.osm_type and osm_id = NEW.osm_id
-              and class = NEW.class and type = NEW.type;
-      DELETE FROM place
-        WHERE osm_type = NEW.osm_type and osm_id = NEW.osm_id
-              and class = NEW.class and type = NEW.type;
+    IF existingplacex.osm_type is not NULL THEN
+      -- Mark any existing place for delete in the placex table
+      UPDATE placex SET indexed_status = 100
+        WHERE placex.osm_type = NEW.osm_type and placex.osm_id = NEW.osm_id
+              and placex.class = 'boundary' and placex.type = 'administrative';
     END IF;
 
     -- Process it as a new insertion
@@ -211,6 +205,27 @@ BEGIN
 
     {% if debug %}RAISE WARNING 'insert done % % % % %',NEW.osm_type,NEW.osm_id,NEW.class,NEW.type,NEW.name;{% endif %}
 
+    IF existing.osm_type is not NULL THEN
+      -- If there is already an entry in place, just update that, if necessary.
+      IF coalesce(existing.name, ''::hstore) != coalesce(NEW.name, ''::hstore)
+         or coalesce(existing.address, ''::hstore) != coalesce(NEW.address, ''::hstore)
+         or coalesce(existing.extratags, ''::hstore) != coalesce(NEW.extratags, ''::hstore)
+         or coalesce(existing.admin_level, 15) != coalesce(NEW.admin_level, 15)
+         or existing.geometry::text != NEW.geometry::text
+      THEN
+        UPDATE place
+          SET name = NEW.name,
+              address = NEW.address,
+              extratags = NEW.extratags,
+              admin_level = NEW.admin_level,
+              geometry = NEW.geometry
+          WHERE osm_type = NEW.osm_type and osm_id = NEW.osm_id
+                and class = NEW.class and type = NEW.type;
+      END IF;
+
+      RETURN NULL;
+    END IF;
+
     RETURN NEW;
   END IF;
 
diff --git a/test/bdd/osm2pgsql/update/tags.feature b/test/bdd/osm2pgsql/update/tags.feature
index b94f2564..f617c38f 100644
--- a/test/bdd/osm2pgsql/update/tags.feature
+++ b/test/bdd/osm2pgsql/update/tags.feature
@@ -364,3 +364,50 @@ Feature: Tag evaluation
           | object     | indexed_status |
           | W1:tourism | 2              |
           | W1:highway | 1              |
+
+
+    Scenario: Replay on administrative boundary
+        When loading osm data
+          """
+          n10 x34.0 y-4.23
+          n11 x34.1 y-4.23
+          n12 x34.2 y-4.13
+          w10 Tboundary=administrative,waterway=river,name=Border,admin_level=2 Nn12,n11,n10
+          """
+        Then place contains exactly
+          | object       |
+          | W10:waterway |
+          | W10:boundary |
+
+        When updating osm data
+          """
+          w10 Tboundary=administrative,waterway=river,name=Border,admin_level=2 Nn12,n11,n10
+          """
+        Then place contains exactly
+          | object       |
+          | W10:waterway |
+          | W10:boundary |
+        And placex contains exactly
+          | object       |
+          | W10:waterway |
+
+
+    Scenario: Change admin_level on administrative boundary
+        When loading osm data
+          """
+          n10 x34.0 y-4.23
+          n11 x34.1 y-4.23
+          n12 x34.2 y-4.13
+          w10 Tboundary=administrative,name=Border,admin_level=2 Nn12,n11,n10
+          """
+        Then place contains exactly
+          | object       | admin_level |
+          | W10:boundary | 2           |
+
+        When updating osm data
+          """
+          w10 Tboundary=administrative,name=Border,admin_level=4 Nn12,n11,n10
+          """
+        Then place contains exactly
+          | object       | admin_level |
+          | W10:boundary | 4           |
-- 
2.39.5