From 91e345f77ff5c64e2ba726ad40c65c378e1ce04d Mon Sep 17 00:00:00 2001 From: Emily Love Watson Date: Fri, 22 Aug 2025 15:57:28 -0500 Subject: [PATCH] Store entrance fields as columns on table --- lib-lua/themes/nominatim/presets.lua | 2 +- lib-sql/functions/utils.sql | 28 ++++++--- lib-sql/tables.sql | 17 +++--- src/nominatim_api/results.py | 25 +++++--- src/nominatim_api/sql/sqlalchemy_schema.py | 9 ++- src/nominatim_api/types.py | 9 +-- src/nominatim_api/v1/format_json.py | 4 +- src/nominatim_api/v1/format_xml.py | 4 +- src/nominatim_db/tools/migration.py | 21 ++++--- test/bdd/features/db/import/entrances.feature | 19 +++--- test/bdd/features/db/update/entrances.feature | 61 ++++++++++++++----- 11 files changed, 127 insertions(+), 72 deletions(-) diff --git a/lib-lua/themes/nominatim/presets.lua b/lib-lua/themes/nominatim/presets.lua index 5aed0f57..3fac52b1 100644 --- a/lib-lua/themes/nominatim/presets.lua +++ b/lib-lua/themes/nominatim/presets.lua @@ -174,7 +174,7 @@ module.MAIN_TAGS_POIS = function (group) fire_hydrant = group}, entrance = {'always', no = group}, - ["routing:entrance"] = {'always', + ["routing:entrance"] = {exclude_when_key_present('entrance'), no = group}, healthcare = {'fallback', yes = group, diff --git a/lib-sql/functions/utils.sql b/lib-sql/functions/utils.sql index 6f9db61f..aea6d296 100644 --- a/lib-sql/functions/utils.sql +++ b/lib-sql/functions/utils.sql @@ -629,19 +629,27 @@ CREATE OR REPLACE FUNCTION place_update_entrances(placeid BIGINT, osmid BIGINT) RETURNS INTEGER AS $$ DECLARE - entrances JSONB; + entrance RECORD; + osm_ids BIGINT[]; BEGIN - SELECT jsonb_agg(jsonb_build_object('osm_id', osm_id, 'type', type, 'lat', ST_Y(geometry), 'lon', ST_X(geometry), 'extratags', extratags)) + osm_ids := '{}'; + FOR entrance in SELECT osm_id, type, geometry, extratags FROM place - WHERE osm_id IN (SELECT unnest(nodes) FROM planet_osm_ways WHERE id=osmid) AND class IN ('routing:entrance', 'entrance') - INTO entrances; - IF entrances IS NOT NULL THEN - INSERT INTO place_entrance (place_id, entrances) - SELECT placeid, entrances - ON CONFLICT (place_id) DO UPDATE - SET entrances = excluded.entrances; + WHERE osm_type = 'N' + AND osm_id IN (SELECT unnest(nodes) FROM planet_osm_ways WHERE id=osmid) + AND class IN ('routing:entrance', 'entrance') + LOOP + osm_ids := array_append(osm_ids, entrance.osm_id); + INSERT INTO placex_entrance (place_id, osm_id, type, location, extratags) + VALUES (placeid, entrance.osm_id, entrance.type, entrance.geometry, entrance.extratags) + ON CONFLICT (place_id, osm_id) DO UPDATE + SET type = excluded.type, location = excluded.location, extratags = excluded.extratags; + END LOOP; + + IF array_length(osm_ids, 1) > 0 THEN + DELETE FROM placex_entrance WHERE place_id=placeid AND NOT osm_id=ANY(osm_ids); ELSE - DELETE FROM place_entrance WHERE place_id=placeid; + DELETE FROM placex_entrance WHERE place_id=placeid; END IF; RETURN NULL; diff --git a/lib-sql/tables.sql b/lib-sql/tables.sql index 4b11e1a2..956009ac 100644 --- a/lib-sql/tables.sql +++ b/lib-sql/tables.sql @@ -245,18 +245,21 @@ CREATE INDEX idx_postcode_geometry ON location_postcode USING GIST (geometry) {{ GRANT SELECT ON location_postcode TO "{{config.DATABASE_WEBUSER}}" ; -- Table to store location of entrance nodes -DROP TABLE IF EXISTS place_entrance; -CREATE TABLE place_entrance ( +DROP TABLE IF EXISTS placex_entrance; +CREATE TABLE placex_entrance ( place_id BIGINT NOT NULL, - entrances JSONB NOT NULL + osm_id BIGINT NOT NULL, + type TEXT NOT NULL, + location GEOMETRY(Point, 4326) NOT NULL, + extratags HSTORE ); -CREATE UNIQUE INDEX idx_place_entrance_place_id ON place_entrance - USING BTREE (place_id) {{db.tablespace.search_index}}; -GRANT SELECT ON place_entrance TO "{{config.DATABASE_WEBUSER}}" ; +CREATE UNIQUE INDEX idx_placex_entrance_place_id_osm_id ON placex_entrance + USING BTREE (place_id, osm_id) {{db.tablespace.search_index}}; +GRANT SELECT ON placex_entrance TO "{{config.DATABASE_WEBUSER}}" ; -- Create an index on the place table for lookups to populate the entrance -- table -CREATE INDEX IF NOT EXISTS idx_place_entrance_lookup ON place +CREATE INDEX IF NOT EXISTS idx_placex_entrance_lookup ON place USING BTREE (osm_id) WHERE class IN ('routing:entrance', 'entrance'); diff --git a/src/nominatim_api/results.py b/src/nominatim_api/results.py index 0b887280..ddb15a2e 100644 --- a/src/nominatim_api/results.py +++ b/src/nominatim_api/results.py @@ -723,18 +723,27 @@ async def complete_linked_places(conn: SearchConnection, result: BaseResult) -> async def complete_entrances_details(conn: SearchConnection, results: List[BaseResultT]) -> None: - """ Retrieve information about tagged entrances for this place. + """ Retrieve information about tagged entrances for the given results. """ - t = conn.t.place_entrance - 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)) + place_ids = (r.place_id for r in results if r.source_table == SourceTable.PLACEX) + + t = conn.t.placex_entrance + sql = sa.select(t.c.place_id, t.c.osm_id, t.c.type, t.c.location, t.c.extratags)\ + .where(t.c.place_id.in_(place_ids)) current_result = None - 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) + for row in await conn.execute(sql): + if current_result is None or row.place_id != current_result.place_id: + current_result = next((r for r in results if r.place_id == row.place_id), None) assert current_result is not None - current_result.entrances = [EntranceDetails(**r) for r in entrances] + if current_result.entrances is None: + current_result.entrances = [] + current_result.entrances.append(EntranceDetails( + osm_id=row.osm_id, + type=row.type, + location=Point.from_wkb(row.location), + extratags=row.extratags, + )) async def complete_keywords(conn: SearchConnection, result: BaseResult) -> None: diff --git a/src/nominatim_api/sql/sqlalchemy_schema.py b/src/nominatim_api/sql/sqlalchemy_schema.py index e7a9ecf3..283f18fb 100644 --- a/src/nominatim_api/sql/sqlalchemy_schema.py +++ b/src/nominatim_api/sql/sqlalchemy_schema.py @@ -128,7 +128,10 @@ class SearchTables: sa.Column('linegeo', Geometry), sa.Column('postcode', sa.Text)) - self.place_entrance = sa.Table( - 'place_entrance', meta, + self.placex_entrance = sa.Table( + 'placex_entrance', meta, sa.Column('place_id', sa.BigInteger), - sa.Column('entrances', KeyValueStore)) + sa.Column('osm_id', sa.BigInteger), + sa.Column('type', sa.Text), + sa.Column('location', Geometry, nullable=False), + sa.Column('extratags', KeyValueStore)) diff --git a/src/nominatim_api/types.py b/src/nominatim_api/types.py index d126d0cc..38f6ed9d 100644 --- a/src/nominatim_api/types.py +++ b/src/nominatim_api/types.py @@ -566,12 +566,9 @@ class EntranceDetails: type: str """ The value of the OSM entrance tag (i.e. yes, main, secondary, etc.). """ - lat: float - """ The latitude of the entrance node. - """ - lon: float - """ The longitude of the entrance node. + location: Point + """ The location of the entrance node. """ extratags: Dict[str, str] - """ The longitude of the entrance node. + """ The other tags associated with the entrance node. """ diff --git a/src/nominatim_api/v1/format_json.py b/src/nominatim_api/v1/format_json.py index de6a648a..3c665ace 100644 --- a/src/nominatim_api/v1/format_json.py +++ b/src/nominatim_api/v1/format_json.py @@ -77,8 +77,8 @@ def write_entrances(out: JsonWriter, entrances: Optional[List[EntranceDetails]]) out.start_object()\ .keyval('osm_id', entrance.osm_id)\ .keyval('type', entrance.type)\ - .keyval('lat', f"{entrance.lat:0.7f}")\ - .keyval('lon', f"{entrance.lon:0.7f}") + .keyval('lat', f"{entrance.location.lat:0.7f}")\ + .keyval('lon', f"{entrance.location.lon:0.7f}") if entrance.extratags: out.keyval('extratags', entrance.extratags) diff --git a/src/nominatim_api/v1/format_xml.py b/src/nominatim_api/v1/format_xml.py index ec5b7255..a28b3ff5 100644 --- a/src/nominatim_api/v1/format_xml.py +++ b/src/nominatim_api/v1/format_xml.py @@ -87,8 +87,8 @@ def _create_entrance(root: ET.Element, entrance: EntranceDetails) -> None: entrance_node = ET.SubElement(root, 'entrance', attrib={ "osm_id": str(entrance.osm_id), "type": entrance.type, - "lat": f"{entrance.lat:0.7f}", - "lon": f"{entrance.lon:0.7f}", + "lat": f"{entrance.location.lat:0.7f}", + "lon": f"{entrance.location.lon:0.7f}", }) if entrance.extratags: for k, v in entrance.extratags.items(): diff --git a/src/nominatim_db/tools/migration.py b/src/nominatim_db/tools/migration.py index 853eff7c..85a0c811 100644 --- a/src/nominatim_db/tools/migration.py +++ b/src/nominatim_db/tools/migration.py @@ -119,24 +119,27 @@ def create_postcode_parent_index(conn: Connection, **_: Any) -> None: @_migration(5, 1, 99, 0) -def create_place_entrance_table(conn: Connection, config: Configuration, **_: Any) -> None: - """ Add the place_entrance table to store entrance nodes +def create_placex_entrance_table(conn: Connection, config: Configuration, **_: Any) -> None: + """ Add the placex_entrance table to store entrance nodes """ sqlp = SQLPreprocessor(conn, config) sqlp.run_string(conn, """ -- Table to store location of entrance nodes - DROP TABLE IF EXISTS place_entrance; - CREATE TABLE place_entrance ( + DROP TABLE IF EXISTS placex_entrance; + CREATE TABLE placex_entrance ( place_id BIGINT NOT NULL, - entrances JSONB NOT NULL + osm_id BIGINT NOT NULL, + type TEXT NOT NULL, + location GEOMETRY(Point, 4326) NOT NULL, + extratags HSTORE ); - CREATE UNIQUE INDEX idx_place_entrance_place_id ON place_entrance - USING BTREE (place_id) {{db.tablespace.search_index}}; - GRANT SELECT ON place_entrance TO "{{config.DATABASE_WEBUSER}}" ; + CREATE UNIQUE INDEX idx_placex_entrance_place_id_osm_id ON placex_entrance + USING BTREE (place_id, osm_id) {{db.tablespace.search_index}}; + GRANT SELECT ON placex_entrance TO "{{config.DATABASE_WEBUSER}}" ; -- Create an index on the place table for lookups to populate the entrance -- table - CREATE INDEX IF NOT EXISTS idx_place_entrance_lookup ON place + CREATE INDEX IF NOT EXISTS idx_placex_entrance_lookup ON place USING BTREE (osm_id) WHERE class IN ('routing:entrance', 'entrance'); """) diff --git a/test/bdd/features/db/import/entrances.feature b/test/bdd/features/db/import/entrances.feature index c3ea30b2..d2a3e3fc 100644 --- a/test/bdd/features/db/import/entrances.feature +++ b/test/bdd/features/db/import/entrances.feature @@ -6,17 +6,18 @@ Feature: Entrance nodes are recorded | 1 | 2 | | 4 | 3 | Given the places - | osm | class | type | geometry | extratags | - | W1 | building | yes | (1,2,3,4,1) | | - | N10 | entrance | main | 1 | 'wheelchair': 'yes' | - | N20 | entrance | yes | 3 | | + | osm | class | type | geometry | extratags | + | W1 | building | yes | (1,2,3,4,1) | | + | N1 | entrance | main | 1 | 'wheelchair': 'yes' | + | N2 | entrance | yes | 3 | | And the ways - | id | nodes | - | 1 | 10,20,30,40 | + | id | nodes | + | 1 | 1,2,3,4,1 | When importing Then placex contains exactly | object | place_id | | W1 | 1 | - Then place_entrance contains exactly - | place_id | entrances | - | 1 | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': {'wheelchair': 'yes'}}, {'lat': 1e-05, 'lon': 1e-05, 'type': 'yes', 'osm_id': 20, 'extratags': {}}] | + Then placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | + | 1 | 1 | main | 1 | {'wheelchair': 'yes'} | + | 1 | 2 | yes | 3 | {} | diff --git a/test/bdd/features/db/update/entrances.feature b/test/bdd/features/db/update/entrances.feature index 8d0091d5..adadee9a 100644 --- a/test/bdd/features/db/update/entrances.feature +++ b/test/bdd/features/db/update/entrances.feature @@ -15,8 +15,8 @@ Feature: Entrance nodes are recorded Then placex contains exactly | object | place_id | | W1 | 1 | - Then place_entrance contains exactly - | place_id | entrances | + Then placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | When updating places | osm | class | type | geometry | | N1 | entrance | main | 1 | @@ -24,9 +24,9 @@ Feature: Entrance nodes are recorded 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': 1, 'extratags': None}] | + Then placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | + | 1 | 1 | main | 1 | - | Scenario: A building with a updated entrance node Given the grid @@ -44,8 +44,8 @@ Feature: Entrance nodes are recorded | object | place_id | | N1 | 1 | | W1 | 2 | - Then place_entrance contains exactly - | place_id | entrances | + Then placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | When updating places | osm | class | type | geometry | | N1 | entrance | main | 1 | @@ -54,9 +54,9 @@ Feature: Entrance nodes are recorded | object | place_id | | N1 | 1 | | W1 | 2 | - And place_entrance contains exactly - | place_id | entrances | - | 2 | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 1, 'extratags': None}] | + And placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | + | 2 | 1 | main | 1 | - | Scenario: A building with a removed entrance Given the grid @@ -73,9 +73,9 @@ Feature: Entrance nodes are recorded 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': 1, 'extratags': None}] | + And placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | + | 1 | 1 | main | 1 | - | When marking for delete N1 And updating places | osm | class | type | geometry | @@ -83,5 +83,36 @@ Feature: Entrance nodes are recorded Then placex contains exactly | object | place_id | | W1 | 1 | - And place_entrance contains exactly - | place_id | entrances | + And placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | + + Scenario: A building with a removed and remaining entrance + Given the grid + | 1 | 2 | + | 4 | 3 | + Given the places + | osm | class | type | geometry | + | N1 | entrance | main | 1 | + | N3 | entrance | yes | 3 | + | W1 | building | yes | (1,2,3,4,1) | + And the ways + | id | nodes | + | 1 | 1, 2, 3, 4, 1 | + When importing + Then placex contains exactly + | object | place_id | + | W1 | 1 | + And placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | + | 1 | 1 | main | 1 | - | + | 1 | 3 | yes | 3 | - | + 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 | + And placex_entrance contains exactly + | place_id | osm_id | type | location!wkt | extratags | + | 1 | 3 | yes | 3 | - | -- 2.39.5