Select all entrances for results in one query

lib-sql/functions/place_triggers.sql

@@ -338,6 +338,11 @@ BEGIN
     END IF;
   END IF;
 
+  -- When an existing way is updated, recalculate entrances
+  IF existingplacex.osm_type = 'W' and (existingplacex.rank_search > 27 or existingplacex.class IN ('landuse', 'leisure')) THEN
+    PERFORM place_update_entrances(existingplacex.place_id, existingplacex.osm_id);
+  END IF;
+
   -- Abort the insertion (we modified the existing place instead)
   RETURN NULL;
 END;
@@ -365,35 +370,3 @@ BEGIN
   RETURN NULL;
 END;
 $$ LANGUAGE plpgsql;
-
-CREATE OR REPLACE FUNCTION place_after_insert()
-  RETURNS TRIGGER
-  AS $$
-BEGIN
-  {% if debug %}
-    RAISE WARNING 'place_after_insert: % % % % %',NEW.osm_type,NEW.osm_id,NEW.class,NEW.type,st_area(NEW.geometry);
-  {% endif %}
-
-  IF NEW.class IN ('routing:entrance', 'entrance') THEN
-    PERFORM place_update_entrances_for_node(NEW.osm_id);
-  END IF;
-
-  RETURN NEW;
-END;
-$$ LANGUAGE plpgsql;
-
-CREATE OR REPLACE FUNCTION place_after_delete()
-  RETURNS TRIGGER
-  AS $$
-BEGIN
-  {% if debug %}
-    RAISE WARNING 'place_after_delete: % % % % %',OLD.osm_type,OLD.osm_id,OLD.class,OLD.type,st_area(OLD.geometry);
-  {% endif %}
-
-  IF OLD.class IN ('routing:entrance', 'entrance') THEN
-    PERFORM place_update_entrances_for_node(OLD.osm_id);
-  END IF;
-
-  RETURN NULL;
-END;
-$$ LANGUAGE plpgsql;

lib-sql/functions/placex_triggers.sql

@@ -818,8 +818,6 @@ DECLARE
   nameaddress_vector INTEGER[];
   addr_nameaddress_vector INTEGER[];
 
-  entrances JSONB;
-
   linked_place BIGINT;
 
   linked_node_id BIGINT;

lib-sql/functions/utils.sql

@@ -625,24 +625,6 @@ END;
 $$ LANGUAGE plpgsql;
 
 
-CREATE OR REPLACE FUNCTION place_update_entrances_for_node(osmid BIGINT)
-  RETURNS INTEGER
-  AS $$
-DECLARE
-  entrance_way RECORD;
-BEGIN
-  FOR entrance_way IN
-    SELECT osm_id, place_id FROM planet_osm_ways JOIN placex ON placex.osm_id = planet_osm_ways.id WHERE osmid=ANY(nodes)
-  LOOP
-    PERFORM place_update_entrances(entrance_way.place_id, entrance_way.osm_id);
-  END LOOP;
-
-  RETURN NULL;
-END;
-$$
-LANGUAGE plpgsql;
-
-
 CREATE OR REPLACE FUNCTION place_update_entrances(placeid BIGINT, osmid BIGINT)
   RETURNS INTEGER
   AS $$

lib-sql/table-triggers.sql

@@ -24,10 +24,6 @@ CREATE TRIGGER place_before_delete BEFORE DELETE ON place
     FOR EACH ROW EXECUTE PROCEDURE place_delete();
 CREATE TRIGGER place_before_insert BEFORE INSERT ON place
     FOR EACH ROW EXECUTE PROCEDURE place_insert();
-CREATE TRIGGER place_after_insert AFTER INSERT ON place
-    FOR EACH ROW EXECUTE PROCEDURE place_after_insert();
-CREATE TRIGGER place_after_delete AFTER DELETE ON place
-    FOR EACH ROW EXECUTE PROCEDURE place_after_delete();
 
 CREATE TRIGGER location_postcode_before_update BEFORE UPDATE ON location_postcode
     FOR EACH ROW EXECUTE PROCEDURE postcode_update();

src/nominatim_api/results.py

@@ -470,8 +470,7 @@ async def add_result_details(conn: SearchConnection, results: List[BaseResultT],
                 await complete_parented_places(conn, result)
         if details.entrances:
             log().comment('Query entrances details')
-            for result in results:
+            await complete_entrances_details(conn, results)
-                await complete_entrances_details(conn, result)
         if details.keywords:
             log().comment('Query keywords')
             for result in results:
@@ -723,17 +722,19 @@ async def complete_linked_places(conn: SearchConnection, result: BaseResult) ->
         result.linked_rows.append(_result_row_to_address_row(row))
 
 
-async def complete_entrances_details(conn: SearchConnection, result: BaseResult) -> None:
+async def complete_entrances_details(conn: SearchConnection, results: List[BaseResultT]) -> None:
     """ Retrieve information about tagged entrances for this place.
     """
-    if result.source_table != SourceTable.PLACEX:
-        return
-
     t = conn.t.place_entrance
-    sql = sa.select(t.c.entrances).where(t.c.place_id == result.place_id)
+    place_ids = (r.place_id for r in results)
+    sql = sa.select(t.c.place_id, t.c.entrances).where(t.c.place_id.in_(place_ids))
 
-    for results in await conn.execute(sql):
+    current_result = None
-        result.entrances = [EntranceDetails(**r) for r in results[0]]
+    for place_id, entrances in await conn.execute(sql):
+        if current_result is None or place_id != current_result.place_id:
+            current_result = next((r for r in results if r.place_id == place_id), None)
+            assert current_result is not None
+        current_result.entrances = [EntranceDetails(**r) for r in entrances]
 
 
 async def complete_keywords(conn: SearchConnection, result: BaseResult) -> None:

test/bdd/conftest.py

@@ -247,6 +247,23 @@ def check_result_for_field_absence(nominatim_result, attributes):
     assert all(a not in nominatim_result.result for a in attributes)
 
 
+@then(step_parse(
+    r'the result contains array field (?P<field>\S+) where element (?P<num>\d+) contains'),
+      converters={'num': int})
+def check_result_array_field_for_attributes(nominatim_result, datatable, field, num):
+    assert nominatim_result.is_simple()
+
+    if datatable[0] == ['param', 'value']:
+        pairs = datatable[1:]
+    else:
+        pairs = zip(datatable[0], datatable[1])
+
+    prefix = f"{field}+{num}+"
+
+    for k, v in pairs:
+        assert ResultAttr(nominatim_result.result, prefix + k) == v
+
+
 @then(step_parse('the result set contains(?P<exact> exactly)?'))
 def check_result_list_match(nominatim_result, datatable, exact):
     assert not nominatim_result.is_simple()

test/bdd/features/api/details/params.feature

@@ -36,7 +36,9 @@ Feature: Object details
           | W       | 429210603 | 1         |
         Then a HTTP 200 is returned
         And the result is valid json
-        And the result has attributes entrances
+        And the result contains array field entrances where element 0 contains
+          | osm_id     | type | lat        | lon       |
+          | 6580031131 | yes  | 47.2489382 | 9.5284033 |
 
     Scenario: Details with linkedplaces
         When sending v1/details

test/bdd/features/db/update/entrances.feature

@@ -9,8 +9,8 @@ Feature: Entrance nodes are recorded
           | osm  | class    | type  | geometry    |
           | W1   | building | yes   | (1,2,3,4,1) |
         And the ways
-          | id | nodes       |
+          | id | nodes         |
-          | 1  | 10,20,30,40 |
+          | 1  | 1, 2, 3, 4, 1 |
         When importing
         Then placex contains exactly
          | object | place_id |
@@ -18,63 +18,68 @@ Feature: Entrance nodes are recorded
         Then place_entrance contains exactly
          | place_id | entrances |
         When updating places
-          | osm  | class    | type  | geometry    |
+          | osm | class    | type  | geometry    |
-          | N10  | entrance | main  | 1           |
+          | N1  | entrance | main  | 1           |
+          | W1  | building | yes   | (1,2,3,4,1) |
         Then placex contains exactly
          | object | place_id |
          | W1     | 1        |
         And place_entrance contains exactly
          | place_id | entrances |
-         | 1        | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': None}] |
+         | 1        | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 1, 'extratags': None}] |
 
     Scenario: A building with a updated entrance node
         Given the grid
           | 1 | 2 |
           | 4 | 3 |
         Given the places
-          | osm  | class    | type  | geometry    |
+          | osm | class    | type  | geometry    |
-          | W1   | building | yes   | (1,2,3,4,1) |
+          | N1  | barrier  | gate  | 1           |
-          | N10  | barrier  | gate  | 1           |
+          | W1  | building | yes   | (1,2,3,4,1) |
         And the ways
-          | id | nodes       |
+          | id | nodes         |
-          | 1  | 10,20,30,40 |
+          | 1  | 1, 2, 3, 4, 1 |
         When importing
         Then placex contains exactly
          | object | place_id |
-         | W1     | 1        |
+         | N1     | 1        |
-         | N10    | 2        |
+         | W1     | 2        |
         Then place_entrance contains exactly
          | place_id | entrances |
         When updating places
-          | osm  | class    | type  | geometry    |
+          | osm | class    | type  | geometry    |
-          | N10  | entrance | main  | 1           |
+          | N1  | entrance | main  | 1           |
+          | W1  | building | yes   | (1,2,3,4,1) |
         Then placex contains exactly
          | object | place_id |
-         | W1     | 1        |
+         | N1     | 1        |
-         | N10    | 2        |
+         | W1     | 2        |
         And place_entrance contains exactly
          | place_id | entrances |
-         | 1        | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': None}] |
+         | 2        | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 1, 'extratags': None}] |
 
     Scenario: A building with a removed entrance
         Given the grid
           | 1 | 2 |
           | 4 | 3 |
         Given the places
-          | osm  | class    | type  | geometry    |
+          | osm | class    | type  | geometry    |
-          | W1   | building | yes   | (1,2,3,4,1) |
+          | N1  | entrance | main  | 1           |
-          | N10  | entrance | main  | 1           |
+          | W1  | building | yes   | (1,2,3,4,1) |
         And the ways
-          | id | nodes       |
+          | id | nodes         |
-          | 1  | 10,20,30,40 |
+          | 1  | 1, 2, 3, 4, 1 |
         When importing
         Then placex contains exactly
          | object | place_id |
          | W1     | 1        |
         And place_entrance contains exactly
          | place_id | entrances |
-         | 1        | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': None}] |
+         | 1        | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 1, 'extratags': None}] |
-        When marking for delete N10
+        When marking for delete N1
+        And updating places
+          | osm | class    | type  | geometry  |
+          | W1  | building | yes   | (2,3,4,2) |
         Then placex contains exactly
          | object | place_id |
          | W1     | 1        |

test/bdd/utils/checks.py

@@ -7,6 +7,7 @@
 """
 Helper functions to compare expected values.
 """
+import collections.abc
 import json
 import re
 import math
@@ -102,8 +103,13 @@ class ResultAttr:
             self.subobj = self.obj
             for sub in self.key.split('+'):
                 done += f"[{sub}]"
-                assert sub in self.subobj, \
+                if isinstance(self.subobj, collections.abc.Sequence) and sub.isdigit():
-                    f"Missing attribute {done}. Full object:\n{_pretty(self.obj)}"
+                    sub = int(sub)
+                    assert sub < len(self.subobj), \
+                        f"Out of bound index {done}. Full object:\n{_pretty(self.obj)}"
+                else:
+                    assert sub in self.subobj, \
+                        f"Missing attribute {done}. Full object:\n{_pretty(self.obj)}"
                 self.subobj = self.subobj[sub]
 
     def __eq__(self, other):