From: Sarah Hoffmann Date: Wed, 28 Feb 2024 13:25:16 +0000 (+0100) Subject: Merge pull request #3342 from mtmail/tyops X-Git-Tag: v4.4.0~6 X-Git-Url: https://git.openstreetmap.org./nominatim.git/commitdiff_plain/247065ff6f6f096c609729080b83896235aedfc8?hp=7205491b8495e48c62b28373d1746e77d475582b Merge pull request #3342 from mtmail/tyops Correct some typos --- diff --git a/lib-sql/functions/importance.sql b/lib-sql/functions/importance.sql index 6c089d82..9b2fb773 100644 --- a/lib-sql/functions/importance.sql +++ b/lib-sql/functions/importance.sql @@ -130,7 +130,7 @@ BEGIN -- Still nothing? Fall back to a default. IF result.importance is null THEN - result.importance := 0.75001 - (rank_search::float / 40); + result.importance := 0.40001 - (rank_search::float / 75); END IF; {% if 'secondary_importance' in db.tables %} diff --git a/lib-sql/functions/place_triggers.sql b/lib-sql/functions/place_triggers.sql index f3b6ab2b..5b4a8441 100644 --- a/lib-sql/functions/place_triggers.sql +++ b/lib-sql/functions/place_triggers.sql @@ -296,7 +296,9 @@ BEGIN extratags = NEW.extratags, admin_level = NEW.admin_level, indexed_status = 2, - geometry = NEW.geometry + geometry = CASE WHEN existingplacex.rank_address = 0 + THEN simplify_large_polygons(NEW.geometry) + ELSE NEW.geometry END WHERE place_id = existingplacex.place_id; -- Invalidate linked places: they potentially get a new name and addresses. diff --git a/lib-sql/functions/placex_triggers.sql b/lib-sql/functions/placex_triggers.sql index 530bf541..386140f4 100644 --- a/lib-sql/functions/placex_triggers.sql +++ b/lib-sql/functions/placex_triggers.sql @@ -119,12 +119,14 @@ CREATE OR REPLACE FUNCTION find_associated_street(poi_osm_type CHAR(1), AS $$ DECLARE location RECORD; + member JSONB; parent RECORD; result BIGINT; distance FLOAT; new_distance FLOAT; waygeom GEOMETRY; BEGIN +{% if db.middle_db_format == '1' %} FOR location IN SELECT members FROM planet_osm_rels WHERE parts @> ARRAY[poi_osm_id] @@ -161,6 +163,40 @@ BEGIN END LOOP; END LOOP; +{% else %} + FOR member IN + SELECT value FROM planet_osm_rels r, LATERAL jsonb_array_elements(members) + WHERE planet_osm_member_ids(members, poi_osm_type::char(1)) && ARRAY[poi_osm_id] + and tags->>'type' = 'associatedStreet' + and value->>'role' = 'street' + LOOP + FOR parent IN + SELECT place_id, geometry + FROM placex + WHERE osm_type = (member->>'type')::char(1) + and osm_id = (member->>'ref')::bigint + and name is not null + and rank_search between 26 and 27 + LOOP + -- Find the closest 'street' member. + -- Avoid distance computation for the frequent case where there is + -- only one street member. + IF waygeom is null THEN + result := parent.place_id; + waygeom := parent.geometry; + ELSE + distance := coalesce(distance, ST_Distance(waygeom, bbox)); + new_distance := ST_Distance(parent.geometry, bbox); + IF new_distance < distance THEN + distance := new_distance; + result := parent.place_id; + waygeom := parent.geometry; + END IF; + END IF; + END LOOP; + END LOOP; +{% endif %} + RETURN result; END; $$ @@ -257,7 +293,11 @@ CREATE OR REPLACE FUNCTION find_linked_place(bnd placex) RETURNS placex AS $$ DECLARE +{% if db.middle_db_format == '1' %} relation_members TEXT[]; +{% else %} + relation_members JSONB; +{% endif %} rel_member RECORD; linked_placex placex%ROWTYPE; bnd_name TEXT; @@ -678,6 +718,12 @@ BEGIN NEW.country_code := NULL; END IF; + -- Simplify polygons with a very large memory footprint when they + -- do not take part in address computation. + IF NEW.rank_address = 0 THEN + NEW.geometry := simplify_large_polygons(NEW.geometry); + END IF; + END IF; {% if debug %}RAISE WARNING 'placex_insert:END: % % % %',NEW.osm_type,NEW.osm_id,NEW.class,NEW.type;{% endif %} @@ -749,7 +795,11 @@ CREATE OR REPLACE FUNCTION placex_update() DECLARE i INTEGER; location RECORD; +{% if db.middle_db_format == '1' %} relation_members TEXT[]; +{% else %} + relation_member JSONB; +{% endif %} geom GEOMETRY; parent_address_level SMALLINT; @@ -971,6 +1021,7 @@ BEGIN -- waterway ways are linked when they are part of a relation and have the same class/type IF NEW.osm_type = 'R' and NEW.class = 'waterway' THEN +{% if db.middle_db_format == '1' %} FOR relation_members IN select members from planet_osm_rels r where r.id = NEW.osm_id and r.parts != array[]::bigint[] LOOP FOR i IN 1..array_upper(relation_members, 1) BY 2 LOOP @@ -989,6 +1040,29 @@ BEGIN END IF; END LOOP; END LOOP; +{% else %} + FOR relation_member IN + SELECT value FROM planet_osm_rels r, LATERAL jsonb_array_elements(r.members) + WHERE r.id = NEW.osm_id + LOOP + IF relation_member->>'role' IN ('', 'main_stream', 'side_stream') + and relation_member->>'type' = 'W' + THEN + {% if debug %}RAISE WARNING 'waterway parent %, child %', NEW.osm_id, relation_member;{% endif %} + FOR linked_node_id IN + SELECT place_id FROM placex + WHERE osm_type = 'W' and osm_id = (relation_member->>'ref')::bigint + and class = NEW.class and type in ('river', 'stream', 'canal', 'drain', 'ditch') + and (relation_member->>'role' != 'side_stream' or NEW.name->'name' = name->'name') + LOOP + UPDATE placex SET linked_place_id = NEW.place_id WHERE place_id = linked_node_id; + {% if 'search_name' in db.tables %} + DELETE FROM search_name WHERE place_id = linked_node_id; + {% endif %} + END LOOP; + END IF; + END LOOP; +{% endif %} {% if debug %}RAISE WARNING 'Waterway processed';{% endif %} END IF; diff --git a/lib-sql/functions/utils.sql b/lib-sql/functions/utils.sql index ff2f037d..f8b365c5 100644 --- a/lib-sql/functions/utils.sql +++ b/lib-sql/functions/utils.sql @@ -73,6 +73,26 @@ END; $$ LANGUAGE plpgsql IMMUTABLE; + +CREATE OR REPLACE FUNCTION get_rel_node_members(members JSONB, memberLabels TEXT[]) + RETURNS SETOF BIGINT + AS $$ +DECLARE + member JSONB; +BEGIN + FOR member IN SELECT * FROM jsonb_array_elements(members) + LOOP + IF member->>'type' = 'N' and member->>'role' = ANY(memberLabels) THEN + RETURN NEXT (member->>'ref')::bigint; + END IF; + END LOOP; + + RETURN; +END; +$$ +LANGUAGE plpgsql IMMUTABLE; + + -- Copy 'name' to or from the default language. -- -- \param country_code Country code of the object being named. @@ -416,6 +436,20 @@ END; $$ LANGUAGE plpgsql IMMUTABLE; +CREATE OR REPLACE FUNCTION simplify_large_polygons(geometry GEOMETRY) + RETURNS GEOMETRY + AS $$ +BEGIN + IF ST_GeometryType(geometry) in ('ST_Polygon','ST_MultiPolygon') + and ST_MemSize(geometry) > 3000000 + THEN + geometry := ST_SimplifyPreserveTopology(geometry, 0.0001); + END IF; + RETURN geometry; +END; +$$ +LANGUAGE plpgsql IMMUTABLE; + CREATE OR REPLACE FUNCTION place_force_delete(placeid BIGINT) RETURNS BOOLEAN diff --git a/lib-sql/tables.sql b/lib-sql/tables.sql index 17216b50..eafed6d8 100644 --- a/lib-sql/tables.sql +++ b/lib-sql/tables.sql @@ -298,7 +298,15 @@ CREATE TABLE IF NOT EXISTS wikipedia_redirect ( -- osm2pgsql does not create indexes on the middle tables for Nominatim -- Add one for lookup of associated street relations. -CREATE INDEX planet_osm_rels_parts_associated_idx ON planet_osm_rels USING gin(parts) WHERE tags @> ARRAY['associatedStreet']; +{% if db.middle_db_format == '1' %} +CREATE INDEX planet_osm_rels_parts_associated_idx ON planet_osm_rels USING gin(parts) + {{db.tablespace.address_index}} + WHERE tags @> ARRAY['associatedStreet']; +{% else %} +CREATE INDEX planet_osm_rels_relation_members_idx ON planet_osm_rels USING gin(planet_osm_member_ids(members, 'R'::character(1))) + WITH (fastupdate=off) + {{db.tablespace.address_index}}; +{% endif %} -- Needed for lookups if a node is part of an interpolation. CREATE INDEX IF NOT EXISTS idx_place_interpolations diff --git a/nominatim/api/results.py b/nominatim/api/results.py index 8e83d523..d66da6c6 100644 --- a/nominatim/api/results.py +++ b/nominatim/api/results.py @@ -233,7 +233,7 @@ class BaseResult: of the value or an artificial value computed from the place's search rank. """ - return self.importance or (0.7500001 - (self.rank_search/40.0)) + return self.importance or (0.40001 - (self.rank_search/75.0)) def localize(self, locales: Locales) -> None: diff --git a/nominatim/api/search/db_search_fields.py b/nominatim/api/search/db_search_fields.py index aa3a2dad..cd571775 100644 --- a/nominatim/api/search/db_search_fields.py +++ b/nominatim/api/search/db_search_fields.py @@ -199,8 +199,7 @@ class SearchData: categories: Dict[Tuple[str, str], float] = {} min_penalty = 1000.0 for t in tokens: - if t.penalty < min_penalty: - min_penalty = t.penalty + min_penalty = min(min_penalty, t.penalty) cat = t.get_category() if t.penalty < categories.get(cat, 1000.0): categories[cat] = t.penalty diff --git a/nominatim/api/search/db_searches.py b/nominatim/api/search/db_searches.py index 742f4a70..be883953 100644 --- a/nominatim/api/search/db_searches.py +++ b/nominatim/api/search/db_searches.py @@ -700,7 +700,7 @@ class PlaceSearch(AbstractSearch): or (details.viewbox is not None and details.viewbox.area < 0.5): sql = sql.order_by( penalty - sa.case((tsearch.c.importance > 0, tsearch.c.importance), - else_=0.75001-(sa.cast(tsearch.c.search_rank, sa.Float())/40))) + else_=0.40001-(sa.cast(tsearch.c.search_rank, sa.Float())/75))) sql = sql.add_columns(t.c.importance) diff --git a/nominatim/db/sql_preprocessor.py b/nominatim/db/sql_preprocessor.py index 839f682d..af5bc335 100644 --- a/nominatim/db/sql_preprocessor.py +++ b/nominatim/db/sql_preprocessor.py @@ -7,7 +7,7 @@ """ Preprocessing of SQL files. """ -from typing import Set, Dict, Any +from typing import Set, Dict, Any, cast import jinja2 from nominatim.db.connection import Connection @@ -28,13 +28,24 @@ def _get_partitions(conn: Connection) -> Set[int]: def _get_tables(conn: Connection) -> Set[str]: """ Return the set of tables currently in use. - Only includes non-partitioned """ with conn.cursor() as cur: cur.execute("SELECT tablename FROM pg_tables WHERE schemaname = 'public'") return set((row[0] for row in list(cur))) +def _get_middle_db_format(conn: Connection, tables: Set[str]) -> str: + """ Returns the version of the slim middle tables. + """ + if 'osm2pgsql_properties' not in tables: + return '1' + + with conn.cursor() as cur: + cur.execute("SELECT value FROM osm2pgsql_properties WHERE property = 'db_format'") + row = cur.fetchone() + + return cast(str, row[0]) if row is not None else '1' + def _setup_tablespace_sql(config: Configuration) -> Dict[str, str]: """ Returns a dict with tablespace expressions for the different tablespace @@ -84,6 +95,7 @@ class SQLPreprocessor: db_info['tables'] = _get_tables(conn) db_info['reverse_only'] = 'search_name' not in db_info['tables'] db_info['tablespace'] = _setup_tablespace_sql(config) + db_info['middle_db_format'] = _get_middle_db_format(conn, db_info['tables']) self.env.globals['config'] = config self.env.globals['db'] = db_info diff --git a/nominatim/tools/exec_utils.py b/nominatim/tools/exec_utils.py index c742e3e0..db89c389 100644 --- a/nominatim/tools/exec_utils.py +++ b/nominatim/tools/exec_utils.py @@ -31,7 +31,7 @@ def run_osm2pgsql(options: Mapping[str, Any]) -> None: """ env = get_pg_env(options['dsn']) cmd = [str(options['osm2pgsql']), - '--hstore', '--latlon', '--slim', + '--slim', '--log-progress', 'true', '--number-processes', '1' if options['append'] else str(options['threads']), '--cache', str(options['osm2pgsql_cache']), @@ -43,7 +43,7 @@ def run_osm2pgsql(options: Mapping[str, Any]) -> None: os.environ.get('LUAPATH', ';'))) cmd.extend(('--output', 'flex')) else: - cmd.extend(('--output', 'gazetteer')) + cmd.extend(('--output', 'gazetteer', '--hstore', '--latlon')) cmd.append('--append' if options['append'] else '--create') diff --git a/osm2pgsql b/osm2pgsql index 415de9ab..cf66989f 160000 --- a/osm2pgsql +++ b/osm2pgsql @@ -1 +1 @@ -Subproject commit 415de9abdf2d003a5c0a0abe8e8fc139acacc2b5 +Subproject commit cf66989fa2a47aa406f25c0be79e3b4e146284ee diff --git a/settings/import-extratags.lua b/settings/import-extratags.lua index 204bd1c8..830345c6 100644 --- a/settings/import-extratags.lua +++ b/settings/import-extratags.lua @@ -33,7 +33,8 @@ flex.set_main_tags{ craft = 'always', junction = 'fallback', landuse = 'fallback', - leisure = 'always', + leisure = {'always', + nature_reserve = 'fallback'}, office = 'always', mountain_pass = 'always', shop = 'always', @@ -56,11 +57,12 @@ flex.set_prefilters{delete_keys = {'note', 'note:*', 'source', '*source', 'attri natural = {'yes', 'no', 'coastline'}, highway = {'no', 'turning_circle', 'mini_roundabout', 'noexit', 'crossing', 'give_way', 'stop'}, - railway = {'level_crossing', 'no', 'rail'}, + railway = {'level_crossing', 'no', 'rail', 'switch', + 'abandoned', 'signal', 'buffer_stop', 'razed'}, man_made = {'survey_point', 'cutline'}, aerialway = {'pylon', 'no'}, aeroway = {'no'}, - amenity = {'no'}, + amenity = {'no', 'parking_space', 'parking_entrance'}, club = {'no'}, craft = {'no'}, leisure = {'no'}, @@ -72,7 +74,7 @@ flex.set_prefilters{delete_keys = {'note', 'note:*', 'source', '*source', 'attri tunnel = {'no'}, waterway = {'riverbank'}, building = {'no'}, - boundary = {'place'}}, + boundary = {'place', 'land_area'}}, extra_keys = {'*:prefix', '*:suffix', 'name:prefix:*', 'name:suffix:*', 'name:etymology', 'name:signed', 'name:botanical', 'wikidata', '*:wikidata', diff --git a/settings/import-full.lua b/settings/import-full.lua index 1dc317a9..5b1ab060 100644 --- a/settings/import-full.lua +++ b/settings/import-full.lua @@ -33,7 +33,8 @@ flex.set_main_tags{ craft = 'always', junction = 'fallback', landuse = 'fallback', - leisure = 'always', + leisure = {'always', + nature_reserve = 'fallback'}, office = 'always', mountain_pass = 'always', shop = 'always', @@ -60,11 +61,12 @@ flex.set_prefilters{delete_keys = {'note', 'note:*', 'source', '*source', 'attri natural = {'yes', 'no', 'coastline'}, highway = {'no', 'turning_circle', 'mini_roundabout', 'noexit', 'crossing', 'give_way', 'stop'}, - railway = {'level_crossing', 'no', 'rail'}, + railway = {'level_crossing', 'no', 'rail', 'switch', + 'abandoned', 'signal', 'buffer_stop', 'razed'}, man_made = {'survey_point', 'cutline'}, aerialway = {'pylon', 'no'}, aeroway = {'no'}, - amenity = {'no'}, + amenity = {'no', 'parking_space', 'parking_entrance'}, club = {'no'}, craft = {'no'}, leisure = {'no'}, @@ -76,7 +78,7 @@ flex.set_prefilters{delete_keys = {'note', 'note:*', 'source', '*source', 'attri tunnel = {'no'}, waterway = {'riverbank'}, building = {'no'}, - boundary = {'place'}}, + boundary = {'place', 'land_area'}}, extra_keys = {'wikidata', 'wikipedia', 'wikipedia:*'} } diff --git a/test/bdd/steps/steps_db_ops.py b/test/bdd/steps/steps_db_ops.py index 493b40cc..c30ee894 100644 --- a/test/bdd/steps/steps_db_ops.py +++ b/test/bdd/steps/steps_db_ops.py @@ -52,33 +52,52 @@ def add_data_to_planet_relations(context): for tests on data that looks up members. """ with context.db.cursor() as cur: - for r in context.table: - last_node = 0 - last_way = 0 - parts = [] - if r['members']: - members = [] - for m in r['members'].split(','): - mid = NominatimID(m) - if mid.typ == 'N': - parts.insert(last_node, int(mid.oid)) - last_node += 1 - last_way += 1 - elif mid.typ == 'W': - parts.insert(last_way, int(mid.oid)) - last_way += 1 - else: - parts.append(int(mid.oid)) - - members.extend((mid.typ.lower() + mid.oid, mid.cls or '')) - else: - members = None - - tags = chain.from_iterable([(h[5:], r[h]) for h in r.headings if h.startswith("tags+")]) - - cur.execute("""INSERT INTO planet_osm_rels (id, way_off, rel_off, parts, members, tags) - VALUES (%s, %s, %s, %s, %s, %s)""", - (r['id'], last_node, last_way, parts, members, list(tags))) + cur.execute("SELECT value FROM osm2pgsql_properties WHERE property = 'db_format'") + row = cur.fetchone() + if row is None or row[0] == '1': + for r in context.table: + last_node = 0 + last_way = 0 + parts = [] + if r['members']: + members = [] + for m in r['members'].split(','): + mid = NominatimID(m) + if mid.typ == 'N': + parts.insert(last_node, int(mid.oid)) + last_node += 1 + last_way += 1 + elif mid.typ == 'W': + parts.insert(last_way, int(mid.oid)) + last_way += 1 + else: + parts.append(int(mid.oid)) + + members.extend((mid.typ.lower() + mid.oid, mid.cls or '')) + else: + members = None + + tags = chain.from_iterable([(h[5:], r[h]) for h in r.headings if h.startswith("tags+")]) + + cur.execute("""INSERT INTO planet_osm_rels (id, way_off, rel_off, parts, members, tags) + VALUES (%s, %s, %s, %s, %s, %s)""", + (r['id'], last_node, last_way, parts, members, list(tags))) + else: + for r in context.table: + if r['members']: + members = [] + for m in r['members'].split(','): + mid = NominatimID(m) + members.append({'ref': mid.oid, 'role': mid.cls or '', 'type': mid.typ}) + else: + members = [] + + tags = {h[5:]: r[h] for h in r.headings if h.startswith("tags+")} + + cur.execute("""INSERT INTO planet_osm_rels (id, tags, members) + VALUES (%s, %s, %s)""", + (r['id'], psycopg2.extras.Json(tags), + psycopg2.extras.Json(members))) @given("the ways") def add_data_to_planet_ways(context): @@ -86,12 +105,19 @@ def add_data_to_planet_ways(context): tests on that that looks up node ids in this table. """ with context.db.cursor() as cur: + cur.execute("SELECT value FROM osm2pgsql_properties WHERE property = 'db_format'") + row = cur.fetchone() + json_tags = row is not None and row[0] != '1' for r in context.table: - tags = chain.from_iterable([(h[5:], r[h]) for h in r.headings if h.startswith("tags+")]) + if json_tags: + tags = psycopg2.extras.Json({h[5:]: r[h] for h in r.headings if h.startswith("tags+")}) + else: + tags = list(chain.from_iterable([(h[5:], r[h]) + for h in r.headings if h.startswith("tags+")])) nodes = [ int(x.strip()) for x in r['nodes'].split(',') ] cur.execute("INSERT INTO planet_osm_ways (id, nodes, tags) VALUES (%s, %s, %s)", - (r['id'], nodes, list(tags))) + (r['id'], nodes, tags)) ################################ WHEN ################################## diff --git a/test/python/api/test_result_formatting_v1.py b/test/python/api/test_result_formatting_v1.py index 0c54667e..0ff834a4 100644 --- a/test/python/api/test_result_formatting_v1.py +++ b/test/python/api/test_result_formatting_v1.py @@ -77,7 +77,7 @@ def test_search_details_minimal(): 'admin_level': 15, 'names': {}, 'localname': '', - 'calculated_importance': pytest.approx(0.0000001), + 'calculated_importance': pytest.approx(0.00001), 'rank_address': 30, 'rank_search': 30, 'isarea': False, diff --git a/test/python/api/test_results.py b/test/python/api/test_results.py index 2a279028..54a54549 100644 --- a/test/python/api/test_results.py +++ b/test/python/api/test_results.py @@ -37,7 +37,7 @@ def test_minimal_detailed_result(): assert res.lon == 23.1 assert res.lat == 0.5 - assert res.calculated_importance() == pytest.approx(0.0000001) + assert res.calculated_importance() == pytest.approx(0.00001) def test_detailed_result_custom_importance(): res = DetailedResult(SourceTable.PLACEX,