From c0d8b95f67ef3302541b41a89487097f7a00d54b Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Mon, 24 Jan 2022 16:26:28 +0100 Subject: [PATCH 01/13] update interpolations instead of deleting and recreating --- lib-sql/functions/place_triggers.sql | 30 ++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/lib-sql/functions/place_triggers.sql b/lib-sql/functions/place_triggers.sql index 4e316990..7ea08840 100644 --- a/lib-sql/functions/place_triggers.sql +++ b/lib-sql/functions/place_triggers.sql @@ -13,7 +13,7 @@ DECLARE country RECORD; existing RECORD; existingplacex RECORD; - existingline RECORD; + existingline BIGINT[]; result BOOLEAN; BEGIN {% if debug %} @@ -58,19 +58,29 @@ BEGIN and NEW.osm_type='W' and ST_GeometryType(NEW.geometry) = 'ST_LineString' THEN -- Get the existing entry from the interpolation table. - SELECT * INTO existingline + SELECT array_agg(place_id) INTO existingline FROM location_property_osmline WHERE osm_id = NEW.osm_id; - -- Update the interpolation table: - -- delete all old interpolation lines with same osm_id - -- and insert the new one(s) (they can be split up, if they have > 2 nodes) - IF existingline.osm_id IS NOT NULL THEN - DELETE FROM location_property_osmline where osm_id = NEW.osm_id; + IF existingline IS NULL or array_length(existingline, 1) = 0 THEN + INSERT INTO location_property_osmline (osm_id, address, linegeo) + VALUES (NEW.osm_id, NEW.address, NEW.geometry); + ELSE + -- Update the interpolation table: + -- The first entry gets the original data, all other entries + -- are removed and will be recreated on indexing. + -- (An interpolation can be split up, if it has more than 2 address nodes) + UPDATE location_property_osmline + SET address = NEW.address, + linegeo = NEW.geometry, + startnumber = null, + indexed_status = 1 + WHERE place_id = existingline[1]; + IF array_length(existingline, 1) > 1 THEN + DELETE FROM location_property_osmline + WHERE place_id = any(existingline[2:]); + END IF; END IF; - INSERT INTO location_property_osmline (osm_id, address, linegeo) - VALUES (NEW.osm_id, NEW.address, NEW.geometry); - -- Now invalidate all address nodes on the line. -- They get their parent from the interpolation. UPDATE placex p SET indexed_status = 2 From 638ed15ada6c62d34bf5ac82e99255611b90c7d8 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Tue, 25 Jan 2022 10:14:05 +0100 Subject: [PATCH 02/13] improve handling von updates on nodes in interpolations Use the same update mechanism as for updates on the interpolations themselves. Updates must solely happen in place_insert as this is the place where actual changes of the data happen. --- lib-sql/functions/interpolation.sql | 40 ++++++++++++------ lib-sql/functions/place_triggers.sql | 58 +++++++++++---------------- lib-sql/functions/placex_triggers.sql | 5 +-- lib-sql/indices.sql | 4 ++ 4 files changed, 56 insertions(+), 51 deletions(-) diff --git a/lib-sql/functions/interpolation.sql b/lib-sql/functions/interpolation.sql index a8ef3771..098db26c 100644 --- a/lib-sql/functions/interpolation.sql +++ b/lib-sql/functions/interpolation.sql @@ -82,24 +82,38 @@ $$ LANGUAGE plpgsql STABLE; -CREATE OR REPLACE FUNCTION osmline_reinsert(node_id BIGINT, geom GEOMETRY) - RETURNS BOOLEAN +CREATE OR REPLACE FUNCTION reinsert_interpolation(way_id BIGINT, addr HSTORE, + geom GEOMETRY) + RETURNS INT AS $$ DECLARE - existingline RECORD; + existing BIGINT[]; BEGIN - SELECT w.id FROM planet_osm_ways w, location_property_osmline p - WHERE p.linegeo && geom and p.osm_id = w.id and p.indexed_status = 0 - and node_id = any(w.nodes) INTO existingline; + -- Get the existing entry from the interpolation table. + SELECT array_agg(place_id) INTO existing + FROM location_property_osmline WHERE osm_id = way_id; - IF existingline.id is not NULL THEN - DELETE FROM location_property_osmline WHERE osm_id = existingline.id; - INSERT INTO location_property_osmline (osm_id, address, linegeo) - SELECT osm_id, address, geometry FROM place - WHERE osm_type = 'W' and osm_id = existingline.id; - END IF; + IF existing IS NULL or array_length(existing, 1) = 0 THEN + INSERT INTO location_property_osmline (osm_id, address, linegeo) + VALUES (way_id, addr, geom); + ELSE + -- Update the interpolation table: + -- The first entry gets the original data, all other entries + -- are removed and will be recreated on indexing. + -- (An interpolation can be split up, if it has more than 2 address nodes) + UPDATE location_property_osmline + SET address = addr, + linegeo = geom, + startnumber = null, + indexed_status = 1 + WHERE place_id = existing[1]; + IF array_length(existing, 1) > 1 THEN + DELETE FROM location_property_osmline + WHERE place_id = any(existing[2:]); + END IF; + END IF; - RETURN true; + RETURN 1; END; $$ LANGUAGE plpgsql; diff --git a/lib-sql/functions/place_triggers.sql b/lib-sql/functions/place_triggers.sql index 7ea08840..a472d26d 100644 --- a/lib-sql/functions/place_triggers.sql +++ b/lib-sql/functions/place_triggers.sql @@ -14,7 +14,7 @@ DECLARE existing RECORD; existingplacex RECORD; existingline BIGINT[]; - result BOOLEAN; + interpol RECORD; BEGIN {% if debug %} RAISE WARNING 'place_insert: % % % % %',NEW.osm_type,NEW.osm_id,NEW.class,NEW.type,st_area(NEW.geometry); @@ -57,29 +57,7 @@ BEGIN IF NEW.class='place' and NEW.type='houses' and NEW.osm_type='W' and ST_GeometryType(NEW.geometry) = 'ST_LineString' THEN - -- Get the existing entry from the interpolation table. - SELECT array_agg(place_id) INTO existingline - FROM location_property_osmline WHERE osm_id = NEW.osm_id; - - IF existingline IS NULL or array_length(existingline, 1) = 0 THEN - INSERT INTO location_property_osmline (osm_id, address, linegeo) - VALUES (NEW.osm_id, NEW.address, NEW.geometry); - ELSE - -- Update the interpolation table: - -- The first entry gets the original data, all other entries - -- are removed and will be recreated on indexing. - -- (An interpolation can be split up, if it has more than 2 address nodes) - UPDATE location_property_osmline - SET address = NEW.address, - linegeo = NEW.geometry, - startnumber = null, - indexed_status = 1 - WHERE place_id = existingline[1]; - IF array_length(existingline, 1) > 1 THEN - DELETE FROM location_property_osmline - WHERE place_id = any(existingline[2:]); - END IF; - END IF; + PERFORM reinsert_interpolation(NEW.osm_id, NEW.address, NEW.geometry); -- Now invalidate all address nodes on the line. -- They get their parent from the interpolation. @@ -166,6 +144,28 @@ BEGIN RETURN null; END IF; + -- If an address node is part of a interpolation line and changes or is + -- newly inserted (happens when the node already existed but now gets address + -- information), then mark the interpolation line for reparenting. + -- (Already here, because interpolation lines are reindexed before nodes, + -- so in the second call it would be too late.) + IF NEW.osm_type='N' + and coalesce(existing.address, ''::hstore) != coalesce(NEW.address, ''::hstore) + THEN + FOR interpol IN + SELECT DISTINCT osm_id, address, geometry FROM place, planet_osm_ways w + WHERE NEW.geometry && place.geometry + and place.osm_type = 'W' + and exists (SELECT * FROM location_property_osmline + WHERE osm_id = place.osm_id + and indexed_status in (0, 2)) + and w.id = place.osm_id and NEW.osm_id = any (w.nodes) + LOOP + PERFORM reinsert_interpolation(interpol.osm_id, interpol.address, + interpol.geometry); + END LOOP; + END IF; + -- Get the existing placex entry. SELECT * INTO existingplacex FROM placex @@ -288,16 +288,6 @@ BEGIN geometry = NEW.geometry WHERE place_id = existingplacex.place_id; - -- If an address node which is part of a interpolation line changes - -- mark this line for reparenting. - -- (Already here, because interpolation lines are reindexed before nodes, - -- so in the second call it would be too late.) - IF NEW.osm_type='N' - and coalesce(existing.address, ''::hstore) != coalesce(NEW.address, ''::hstore) - THEN - result:= osmline_reinsert(NEW.osm_id, NEW.geometry); - END IF; - -- Invalidate linked places: they potentially get a new name and addresses. IF existingplacex.linked_place_id is not NULL THEN UPDATE placex x diff --git a/lib-sql/functions/placex_triggers.sql b/lib-sql/functions/placex_triggers.sql index 1b96f87e..8ad8a336 100644 --- a/lib-sql/functions/placex_triggers.sql +++ b/lib-sql/functions/placex_triggers.sql @@ -626,10 +626,7 @@ BEGIN {% if not disable_diff_updates %} -- The following is not needed until doing diff updates, and slows the main index process down - IF NEW.osm_type = 'N' and NEW.rank_search > 28 THEN - -- might be part of an interpolation - result := osmline_reinsert(NEW.osm_id, NEW.geometry); - ELSEIF NEW.rank_address > 0 THEN + IF NEW.rank_address > 0 THEN IF (ST_GeometryType(NEW.geometry) in ('ST_Polygon','ST_MultiPolygon') AND ST_IsValid(NEW.geometry)) THEN -- Performance: We just can't handle re-indexing for country level changes IF st_area(NEW.geometry) < 1 THEN diff --git a/lib-sql/indices.sql b/lib-sql/indices.sql index bbcaf43d..fed34524 100644 --- a/lib-sql/indices.sql +++ b/lib-sql/indices.sql @@ -48,6 +48,10 @@ CREATE INDEX IF NOT EXISTS idx_postcode_postcode CREATE UNIQUE INDEX IF NOT EXISTS idx_place_osm_unique ON place USING btree(osm_id, osm_type, class, type) {{db.tablespace.address_index}}; + + CREATE INDEX IF NOT EXISTS idx_place_interpolations + ON place USING gist(geometry) {{db.tablespace.address_index}} + WHERE osm_type = 'W' and address ? 'interpolation'; {% endif %} -- Indices only needed for search. From 9f64c34f1a579cca7aa144365e5519fce9e47391 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Tue, 25 Jan 2022 11:24:13 +0100 Subject: [PATCH 03/13] optimize indexes for interpolation lines Do not index 'inactive' rows (with startnumber is null) where possible. --- lib-sql/functions/placex_triggers.sql | 7 +++---- lib-sql/indices.sql | 11 ++++++++--- lib-sql/tables.sql | 3 ++- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib-sql/functions/placex_triggers.sql b/lib-sql/functions/placex_triggers.sql index 8ad8a336..5db76b7b 100644 --- a/lib-sql/functions/placex_triggers.sql +++ b/lib-sql/functions/placex_triggers.sql @@ -135,9 +135,8 @@ BEGIN FOR location IN SELECT q.parent_place_id FROM location_property_osmline q, planet_osm_ways x - WHERE q.linegeo && bbox and x.id = q.osm_id - and poi_osm_id = any(x.nodes) - LIMIT 1 + WHERE q.linegeo && bbox and startnumber is not null + and x.id = q.osm_id and poi_osm_id = any(x.nodes) LOOP {% if debug %}RAISE WARNING 'Get parent from interpolation: %', location.parent_place_id;{% endif %} RETURN location.parent_place_id; @@ -653,7 +652,7 @@ BEGIN -- roads may cause reparenting for >27 rank places update placex set indexed_status = 2 where indexed_status = 0 and rank_search > NEW.rank_search and ST_DWithin(placex.geometry, NEW.geometry, diameter); -- reparenting also for OSM Interpolation Lines (and for Tiger?) - update location_property_osmline set indexed_status = 2 where indexed_status = 0 and ST_DWithin(location_property_osmline.linegeo, NEW.geometry, diameter); + update location_property_osmline set indexed_status = 2 where indexed_status = 0 and startnumber is not null and ST_DWithin(location_property_osmline.linegeo, NEW.geometry, diameter); ELSEIF NEW.rank_search >= 16 THEN -- up to rank 16, street-less addresses may need reparenting update placex set indexed_status = 2 where indexed_status = 0 and rank_search > NEW.rank_search and ST_DWithin(placex.geometry, NEW.geometry, diameter) and (rank_search < 28 or name is not null or address ? 'place'); diff --git a/lib-sql/indices.sql b/lib-sql/indices.sql index fed34524..ad6753c6 100644 --- a/lib-sql/indices.sql +++ b/lib-sql/indices.sql @@ -28,7 +28,8 @@ CREATE INDEX IF NOT EXISTS idx_placex_geometry_reverse_lookupPolygon AND name is not null AND indexed_status = 0 AND linked_place_id is null; CREATE INDEX IF NOT EXISTS idx_osmline_parent_place_id - ON location_property_osmline USING BTREE (parent_place_id) {{db.tablespace.search_index}}; + ON location_property_osmline USING BTREE (parent_place_id) {{db.tablespace.search_index}} + WHERE parent_place_id is not null; CREATE INDEX IF NOT EXISTS idx_osmline_parent_osm_id ON location_property_osmline USING BTREE (osm_id) {{db.tablespace.search_index}}; @@ -66,8 +67,12 @@ CREATE INDEX IF NOT EXISTS idx_postcode_postcode {% if postgres.has_index_non_key_column %} CREATE INDEX IF NOT EXISTS idx_placex_housenumber - ON placex USING btree (parent_place_id) INCLUDE (housenumber) WHERE housenumber is not null; + ON placex USING btree (parent_place_id) {{db.tablespace.search_index}} + INCLUDE (housenumber) + WHERE housenumber is not null; CREATE INDEX IF NOT EXISTS idx_osmline_parent_osm_id_with_hnr - ON location_property_osmline USING btree(parent_place_id) INCLUDE (startnumber, endnumber); + ON location_property_osmline USING btree(parent_place_id) {{db.tablespace.search_index}} + INCLUDE (startnumber, endnumber) + WHERE startnumber is not null; {% endif %} {% endif %} diff --git a/lib-sql/tables.sql b/lib-sql/tables.sql index 0b35cad2..10713661 100644 --- a/lib-sql/tables.sql +++ b/lib-sql/tables.sql @@ -106,7 +106,8 @@ CREATE TABLE location_property_osmline ( ){{db.tablespace.search_data}}; CREATE UNIQUE INDEX idx_osmline_place_id ON location_property_osmline USING BTREE (place_id) {{db.tablespace.search_index}}; CREATE INDEX idx_osmline_geometry_sector ON location_property_osmline USING BTREE (geometry_sector) {{db.tablespace.address_index}}; -CREATE INDEX idx_osmline_linegeo ON location_property_osmline USING GIST (linegeo) {{db.tablespace.search_index}}; +CREATE INDEX idx_osmline_linegeo ON location_property_osmline USING GIST (linegeo) {{db.tablespace.search_index}} + WHERE startnumber is not null; GRANT SELECT ON location_property_osmline TO "{{config.DATABASE_WEBUSER}}"; drop table IF EXISTS search_name; From e6d855b954e97c04c62032a4f1af8f9307eacada Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Tue, 25 Jan 2022 12:00:35 +0100 Subject: [PATCH 04/13] add migration for new lookup index --- nominatim/tools/migration.py | 11 +++++++++++ nominatim/version.py | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/nominatim/tools/migration.py b/nominatim/tools/migration.py index 94058f33..284bd316 100644 --- a/nominatim/tools/migration.py +++ b/nominatim/tools/migration.py @@ -216,3 +216,14 @@ def create_tiger_housenumber_index(conn, **_): ON location_property_tiger USING btree(parent_place_id) INCLUDE (startnumber, endnumber) """) + + +@_migration(4, 0, 99, 1) +def create_interpolation_index_on_place(conn, **_): + """ Create idx_place_interpolations for lookup of interpolation lines + on updates. + """ + with conn.cursor() as cur: + cur.execute("""CREATE INDEX IF NOT EXISTS idx_place_interpolations + ON place USING gist(geometry) + WHERE osm_type = 'W' and address ? 'interpolation'""") diff --git a/nominatim/version.py b/nominatim/version.py index cb7f59bc..b666c8e7 100644 --- a/nominatim/version.py +++ b/nominatim/version.py @@ -20,11 +20,11 @@ Version information for Nominatim. # to 99 to make sure that the migration is applied when updating from a # patch release to the next minor version. Patch releases usually shouldn't # have migrations in them. When they are needed, then make sure that the -# migration can reapplied and set the migration version to the appropriate +# migration can be reapplied and set the migration version to the appropriate # patch level when cherry-picking the commit with the migration. # # Released versions always have a database patch level of 0. -NOMINATIM_VERSION = (4, 0, 99, 1) +NOMINATIM_VERSION = (4, 0, 99, 2) POSTGRESQL_REQUIRED_VERSION = (9, 5) POSTGIS_REQUIRED_VERSION = (2, 2) From 83d2c440d51d3e1e29b51397b5a6c82856e285af Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Thu, 27 Jan 2022 11:12:40 +0100 Subject: [PATCH 05/13] add migration for new interpolation table layout --- nominatim/tools/migration.py | 32 ++++++++++++++++++++++++++++++++ nominatim/version.py | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/nominatim/tools/migration.py b/nominatim/tools/migration.py index 284bd316..c24b09cb 100644 --- a/nominatim/tools/migration.py +++ b/nominatim/tools/migration.py @@ -227,3 +227,35 @@ def create_interpolation_index_on_place(conn, **_): cur.execute("""CREATE INDEX IF NOT EXISTS idx_place_interpolations ON place USING gist(geometry) WHERE osm_type = 'W' and address ? 'interpolation'""") + + +@_migration(4, 0, 99, 2) +def add_step_column_for_interpolation(conn, **_): + """ Add a new column 'step' to the interpolations table which will. + + Also convers the data into the stricter format which requires that + startnumbers comply with the odd/even requirements. + """ + with conn.cursor() as cur: + # Mark invalid all interpolations with no intermediate numbers. + cur.execute("""UPDATE location_property_osmline SET startnumber = null + WHERE endnumber - startnumber <= 1 """) + # Align the start numbers where odd/even does not match. + cur.execute("""UPDATE location_property_osmline + SET startnumber = startnumber + 1, + linegeo = ST_LineSubString(linegeo, + 1.0 / (endnumber - startnumber)::float, + 1) + WHERE (interpolationtype = 'odd' and startnumber % 2 = 0) + or (interpolationtype = 'even' and startnumber % 2 = 1) + """) + # Mark invalid odd/even interpolations with no intermediate numbers. + cur.execute("""UPDATE location_property_osmline SET startnumber = null + WHERE interpolationtype in ('odd', 'even') + and endnumber - startnumber = 2""") + # Finally add the new column and populate it. + cur.execute("ALTER TABLE location_property_osmline ADD COLUMN step SMALLINT") + cur.execute("""UPDATE location_property_osmline + SET step = CASE WHEN interpolationtype = 'all' + THEN 1 ELSE 2 END + """) diff --git a/nominatim/version.py b/nominatim/version.py index b666c8e7..88bb881f 100644 --- a/nominatim/version.py +++ b/nominatim/version.py @@ -24,7 +24,7 @@ Version information for Nominatim. # patch level when cherry-picking the commit with the migration. # # Released versions always have a database patch level of 0. -NOMINATIM_VERSION = (4, 0, 99, 2) +NOMINATIM_VERSION = (4, 0, 99, 3) POSTGRESQL_REQUIRED_VERSION = (9, 5) POSTGIS_REQUIRED_VERSION = (2, 2) From fea4dbba50d5a53ef982b1eb273e0bb81a2cd036 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Wed, 26 Jan 2022 12:05:04 +0100 Subject: [PATCH 06/13] inherit tags from interpolation not parent Nodes on an interpolation now only get the address tags of interpolations and then compute their own parent from that. They no longer inherit the parent directly. --- lib-sql/functions/interpolation.sql | 229 ++++++++++++++------------ lib-sql/functions/placex_triggers.sql | 29 ++-- lib-sql/tables.sql | 2 +- 3 files changed, 144 insertions(+), 116 deletions(-) diff --git a/lib-sql/functions/interpolation.sql b/lib-sql/functions/interpolation.sql index 098db26c..c0181556 100644 --- a/lib-sql/functions/interpolation.sql +++ b/lib-sql/functions/interpolation.sql @@ -7,17 +7,6 @@ -- Functions for address interpolation objects in location_property_osmline. --- Splits the line at the given point and returns the two parts --- in a multilinestring. -CREATE OR REPLACE FUNCTION split_line_on_node(line GEOMETRY, point GEOMETRY) -RETURNS GEOMETRY - AS $$ -BEGIN - RETURN ST_Split(ST_Snap(line, point, 0.0005), point); -END; -$$ -LANGUAGE plpgsql IMMUTABLE; - CREATE OR REPLACE FUNCTION get_interpolation_address(in_address HSTORE, wayid BIGINT) RETURNS HSTORE @@ -128,8 +117,10 @@ BEGIN IF NEW.indexed_status IS NULL THEN IF NEW.address is NULL OR NOT NEW.address ? 'interpolation' - OR NEW.address->'interpolation' NOT IN ('odd', 'even', 'all') THEN - -- other interpolation types than odd/even/all (e.g. numeric ones) are not supported + OR NOT (NEW.address->'interpolation' in ('odd', 'even', 'all') + or NEW.address->'interpolation' similar to '[1-9]') + THEN + -- alphabetic interpolation is not supported RETURN NULL; END IF; @@ -150,18 +141,20 @@ CREATE OR REPLACE FUNCTION osmline_update() RETURNS TRIGGER AS $$ DECLARE - place_centroid GEOMETRY; waynodes BIGINT[]; prevnode RECORD; nextnode RECORD; startnumber INTEGER; endnumber INTEGER; - housenum INTEGER; + newstart INTEGER; + newend INTEGER; + moddiff SMALLINT; linegeo GEOMETRY; splitline GEOMETRY; sectiongeo GEOMETRY; interpol_postcode TEXT; postcode TEXT; + stepmod SMALLINT; BEGIN -- deferred delete IF OLD.indexed_status = 100 THEN @@ -173,107 +166,139 @@ BEGIN RETURN NEW; END IF; - NEW.interpolationtype = NEW.address->'interpolation'; - - place_centroid := ST_PointOnSurface(NEW.linegeo); - NEW.parent_place_id = get_interpolation_parent(NEW.token_info, NEW.partition, - place_centroid, NEW.linegeo); + NEW.parent_place_id := get_interpolation_parent(NEW.token_info, NEW.partition, + ST_PointOnSurface(NEW.linegeo), + NEW.linegeo); interpol_postcode := token_normalized_postcode(NEW.address->'postcode'); NEW.token_info := token_strip_info(NEW.token_info); IF NEW.address ? '_inherited' THEN - NEW.address := hstore('interpolation', NEW.interpolationtype); + NEW.address := hstore('interpolation', NEW.address->'interpolation'); END IF; - -- if the line was newly inserted, split the line as necessary + -- If the line was newly inserted, split the line as necessary. IF OLD.indexed_status = 1 THEN - select nodes from planet_osm_ways where id = NEW.osm_id INTO waynodes; + IF NEW.address->'interpolation' in ('odd', 'even') THEN + NEW.step := 2; + stepmod := CASE WHEN NEW.address->'interpolation' = 'odd' THEN 1 ELSE 0 END; + ELSE + NEW.step := CASE WHEN NEW.address->'interpolation' = 'all' + THEN 1 + ELSE (NEW.address->'interpolation')::SMALLINT END; + stepmod := NULL; + END IF; - IF array_upper(waynodes, 1) IS NULL THEN - RETURN NEW; + SELECT nodes INTO waynodes + FROM planet_osm_ways WHERE id = NEW.osm_id; + + IF array_upper(waynodes, 1) IS NULL THEN + RETURN NEW; + END IF; + + linegeo := null; + SELECT null::integer as hnr INTO prevnode; + + -- Go through all nodes on the interpolation line that have a housenumber. + FOR nextnode IN + SELECT DISTINCT ON (nodeidpos) + osm_id, address, geometry, + substring(address->'housenumber','[0-9]+')::integer as hnr + FROM placex, generate_series(1, array_upper(waynodes, 1)) nodeidpos + WHERE osm_type = 'N' and osm_id = waynodes[nodeidpos]::BIGINT + and address is not NULL and address ? 'housenumber' + ORDER BY nodeidpos + LOOP + RAISE WARNING 'processing point % (%)', nextnode.hnr, ST_AsText(nextnode.geometry); + IF linegeo is null THEN + linegeo := NEW.linegeo; + ELSE + splitline := ST_Split(ST_Snap(linegeo, nextnode.geometry, 0.0005), nextnode.geometry); + sectiongeo := ST_GeometryN(splitline, 1); + linegeo := ST_GeometryN(splitline, 2); END IF; - linegeo := NEW.linegeo; - startnumber := NULL; - - FOR nodeidpos in 1..array_upper(waynodes, 1) LOOP - - select osm_id, address, geometry - from place where osm_type = 'N' and osm_id = waynodes[nodeidpos]::BIGINT - and address is not NULL and address ? 'housenumber' limit 1 INTO nextnode; - --RAISE NOTICE 'Nextnode.place_id: %s', nextnode.place_id; - IF nextnode.osm_id IS NOT NULL THEN - --RAISE NOTICE 'place_id is not null'; - IF nodeidpos > 1 and nodeidpos < array_upper(waynodes, 1) THEN - -- Make sure that the point is actually on the line. That might - -- be a bit paranoid but ensures that the algorithm still works - -- should osm2pgsql attempt to repair geometries. - splitline := split_line_on_node(linegeo, nextnode.geometry); - sectiongeo := ST_GeometryN(splitline, 1); - linegeo := ST_GeometryN(splitline, 2); - ELSE - sectiongeo = linegeo; - END IF; - endnumber := substring(nextnode.address->'housenumber','[0-9]+')::integer; - - IF startnumber IS NOT NULL AND endnumber IS NOT NULL - AND startnumber != endnumber - AND ST_GeometryType(sectiongeo) = 'ST_LineString' THEN - - IF (startnumber > endnumber) THEN - housenum := endnumber; - endnumber := startnumber; - startnumber := housenum; - sectiongeo := ST_Reverse(sectiongeo); - END IF; - - -- determine postcode - postcode := coalesce(interpol_postcode, - token_normalized_postcode(prevnode.address->'postcode'), - token_normalized_postcode(nextnode.address->'postcode'), - postcode); - - IF postcode is NULL THEN - SELECT token_normalized_postcode(placex.postcode) - FROM placex WHERE place_id = NEW.parent_place_id INTO postcode; - END IF; - IF postcode is NULL THEN - postcode := get_nearest_postcode(NEW.country_code, nextnode.geometry); - END IF; - - IF NEW.startnumber IS NULL THEN - NEW.startnumber := startnumber; - NEW.endnumber := endnumber; - NEW.linegeo := sectiongeo; - NEW.postcode := postcode; - ELSE - insert into location_property_osmline - (linegeo, partition, osm_id, parent_place_id, - startnumber, endnumber, interpolationtype, - address, postcode, country_code, - geometry_sector, indexed_status) - values (sectiongeo, NEW.partition, NEW.osm_id, NEW.parent_place_id, - startnumber, endnumber, NEW.interpolationtype, - NEW.address, postcode, - NEW.country_code, NEW.geometry_sector, 0); - END IF; - END IF; - - -- early break if we are out of line string, - -- might happen when a line string loops back on itself - IF ST_GeometryType(linegeo) != 'ST_LineString' THEN - RETURN NEW; - END IF; - - startnumber := substring(nextnode.address->'housenumber','[0-9]+')::integer; - prevnode := nextnode; + IF prevnode.hnr is not null + -- Check if there are housenumbers to interpolate between the + -- regularly mapped housenumbers. + -- (Conveniently also fails if one of the house numbers is not a number.) + and abs(prevnode.hnr - nextnode.hnr) > NEW.step + THEN + IF prevnode.hnr < nextnode.hnr THEN + startnumber := prevnode.hnr; + endnumber := nextnode.hnr; + ELSE + startnumber := nextnode.hnr; + endnumber := prevnode.hnr; + sectiongeo := ST_Reverse(sectiongeo); END IF; - END LOOP; + + -- Adjust the interpolation, so that only inner housenumbers + -- are taken into account. + IF stepmod is null THEN + newstart := startnumber + NEW.step; + ELSE + newstart := startnumber + 1; + moddiff := newstart % NEW.step - stepmod; + IF moddiff < 0 THEN + newstart := newstart + (NEW.step + moddiff); + ELSE + newstart := newstart + moddiff; + END IF; + END IF; + newend := newstart + ((endnumber - 1 - newstart) / NEW.step) * NEW.step; + + -- If newstart and newend are the same, then this returns a point. + sectiongeo := ST_LineSubstring(sectiongeo, + (newstart - startnumber)::float / (endnumber - startnumber)::float, + (newend - startnumber)::float / (endnumber - startnumber)::float); + startnumber := newstart; + endnumber := newend; + + -- determine postcode + postcode := coalesce(interpol_postcode, + token_normalized_postcode(prevnode.address->'postcode'), + token_normalized_postcode(nextnode.address->'postcode'), + postcode); + IF postcode is NULL THEN + SELECT token_normalized_postcode(placex.postcode) + FROM placex WHERE place_id = NEW.parent_place_id INTO postcode; + END IF; + IF postcode is NULL THEN + postcode := get_nearest_postcode(NEW.country_code, nextnode.geometry); + END IF; + + -- Add the interpolation. If this is the first segment, just modify + -- the interpolation to be inserted, otherwise add an additional one + -- (marking it indexed already). + IF NEW.startnumber IS NULL THEN + NEW.startnumber := startnumber; + NEW.endnumber := endnumber; + NEW.linegeo := sectiongeo; + NEW.postcode := postcode; + ELSE + INSERT INTO location_property_osmline + (linegeo, partition, osm_id, parent_place_id, + startnumber, endnumber, step, + address, postcode, country_code, + geometry_sector, indexed_status) + VALUES (sectiongeo, NEW.partition, NEW.osm_id, NEW.parent_place_id, + startnumber, endnumber, NEW.step, + NEW.address, postcode, + NEW.country_code, NEW.geometry_sector, 0); + END IF; + + -- early break if we are out of line string, + -- might happen when a line string loops back on itself + IF ST_GeometryType(linegeo) != 'ST_LineString' THEN + RETURN NEW; + END IF; + END IF; + + prevnode := nextnode; + END LOOP; END IF; - -- marking descendants for reparenting is not needed, because there are - -- actually no descendants for interpolation lines RETURN NEW; END; $$ diff --git a/lib-sql/functions/placex_triggers.sql b/lib-sql/functions/placex_triggers.sql index 5db76b7b..6ab73a3b 100644 --- a/lib-sql/functions/placex_triggers.sql +++ b/lib-sql/functions/placex_triggers.sql @@ -29,9 +29,9 @@ DECLARE BEGIN -- For POI nodes, check if the address should be derived from a surrounding -- building. - IF p.rank_search < 30 OR p.osm_type != 'N' OR p.address is not null THEN + IF p.rank_search < 30 OR p.osm_type != 'N' THEN result.address := p.address; - ELSE + ELSEIF p.address is null THEN -- The additional && condition works around the misguided query -- planner of postgis 3.0. SELECT placex.address || hstore('_inherited', '') INTO result.address @@ -42,6 +42,20 @@ BEGIN and (placex.address ? 'housenumber' or placex.address ? 'street' or placex.address ? 'place') and rank_search = 30 AND ST_GeometryType(geometry) in ('ST_Polygon','ST_MultiPolygon') LIMIT 1; + ELSE + result.address := p.address; + -- See if we can inherit addtional address tags from an interpolation. + -- These will become permanent. + FOR location IN + SELECT (address - 'interpolation'::text - 'housenumber'::text) as address + FROM place, planet_osm_ways w + WHERE place.osm_type = 'W' and place.address ? 'interpolation' + and place.geometry && p.geometry + and place.osm_id = w.id + and p.osm_id = any(w.nodes) + LOOP + result.address := location.address || result.address; + END LOOP; END IF; result.address := result.address - '_unlisted_place'::TEXT; @@ -131,17 +145,6 @@ BEGIN END IF; IF parent_place_id is null and poi_osm_type = 'N' THEN - -- Is this node part of an interpolation? - FOR location IN - SELECT q.parent_place_id - FROM location_property_osmline q, planet_osm_ways x - WHERE q.linegeo && bbox and startnumber is not null - and x.id = q.osm_id and poi_osm_id = any(x.nodes) - LOOP - {% if debug %}RAISE WARNING 'Get parent from interpolation: %', location.parent_place_id;{% endif %} - RETURN location.parent_place_id; - END LOOP; - FOR location IN SELECT p.place_id, p.osm_id, p.rank_search, p.address, coalesce(p.centroid, ST_Centroid(p.geometry)) as centroid diff --git a/lib-sql/tables.sql b/lib-sql/tables.sql index 10713661..0c0f78fc 100644 --- a/lib-sql/tables.sql +++ b/lib-sql/tables.sql @@ -95,10 +95,10 @@ CREATE TABLE location_property_osmline ( indexed_date TIMESTAMP, startnumber INTEGER, endnumber INTEGER, + step SMALLINT, partition SMALLINT, indexed_status SMALLINT, linegeo GEOMETRY, - interpolationtype TEXT, address HSTORE, token_info JSONB, -- custom column for tokenizer use only postcode TEXT, From 4b28b4fed469ef5826e61633f82b93a29e1c9b3f Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Wed, 26 Jan 2022 15:24:06 +0100 Subject: [PATCH 07/13] adapt BDD tests for new interpolation style --- lib-sql/functions/interpolation.sql | 8 ++- test/bdd/db/import/interpolation.feature | 64 ++++++++++++------------ test/bdd/db/update/interpolation.feature | 54 ++++++++++---------- 3 files changed, 64 insertions(+), 62 deletions(-) diff --git a/lib-sql/functions/interpolation.sql b/lib-sql/functions/interpolation.sql index c0181556..64775678 100644 --- a/lib-sql/functions/interpolation.sql +++ b/lib-sql/functions/interpolation.sql @@ -53,9 +53,13 @@ BEGIN IF parent_place_id is null THEN FOR location IN SELECT place_id FROM placex WHERE ST_DWithin(geom, placex.geometry, 0.001) and placex.rank_search = 26 - ORDER BY (ST_distance(placex.geometry, ST_LineInterpolatePoint(geom,0))+ + ORDER BY CASE WHEN ST_GeometryType(geom) = 'ST_Line' THEN + (ST_distance(placex.geometry, ST_LineInterpolatePoint(geom,0))+ ST_distance(placex.geometry, ST_LineInterpolatePoint(geom,0.5))+ - ST_distance(placex.geometry, ST_LineInterpolatePoint(geom,1))) ASC limit 1 + ST_distance(placex.geometry, ST_LineInterpolatePoint(geom,1))) + ELSE ST_distance(placex.geometry, geom) END + ASC + LIMIT 1 LOOP parent_place_id := location.place_id; END LOOP; diff --git a/test/bdd/db/import/interpolation.feature b/test/bdd/db/import/interpolation.feature index 181e87ee..4624705e 100644 --- a/test/bdd/db/import/interpolation.feature +++ b/test/bdd/db/import/interpolation.feature @@ -16,23 +16,23 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 6 | 1 1, 1 1.001 | + | 4 | 4 | 1 1.0005 | Scenario: Backwards even two point interpolation line Given the places | osm | class | type | housenr | geometry | | N1 | place | house | 2 | 1 1 | - | N2 | place | house | 6 | 1 1.001 | + | N2 | place | house | 8 | 1 1.003 | And the places | osm | class | type | addr+interpolation | geometry | - | W1 | place | houses | even | 1 1.001, 1 1 | + | W1 | place | houses | even | 1 1.003, 1 1 | And the ways | id | nodes | | 1 | 2,1 | When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 6 | 1 1, 1 1.001 | + | 4 | 6 | 1 1.001, 1 1.002 | Scenario: Simple odd two point interpolation Given the places @@ -48,23 +48,23 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 1 | 11 | 1 1, 1 1.001 | + | 3 | 9 | 1 1.0002, 1 1.0008 | Scenario: Simple all two point interpolation Given the places | osm | class | type | housenr | geometry | | N1 | place | house | 1 | 1 1 | - | N2 | place | house | 3 | 1 1.001 | + | N2 | place | house | 4 | 1 1.003 | And the places | osm | class | type | addr+interpolation | geometry | - | W1 | place | houses | all | 1 1, 1 1.001 | + | W1 | place | houses | all | 1 1, 1 1.003 | And the ways | id | nodes | | 1 | 1,2 | When importing Then W1 expands to interpolation | start | end | geometry | - | 1 | 3 | 1 1, 1 1.001 | + | 2 | 3 | 1 1.001, 1 1.002 | Scenario: Even two point interpolation line with intermediate empty node Given the places @@ -80,7 +80,7 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 10 | 1 1, 1 1.001, 1.001 1.001 | + | 4 | 8 | 1 1.0005, 1 1.001, 1.0005 1.001 | Scenario: Even two point interpolation line with intermediate duplicated empty node Given the places @@ -96,7 +96,7 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 10 | 1 1, 1 1.001, 1.001 1.001 | + | 4 | 8 | 1 1.0005, 1 1.001, 1.0005 1.001 | Scenario: Simple even three point interpolation line Given the places @@ -113,8 +113,8 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 10 | 1 1, 1 1.001 | - | 10 | 14 | 1 1.001, 1.001 1.001 | + | 4 | 8 | 1 1.00025, 1 1.00075 | + | 12 | 12 | 1.0005 1.001 | Scenario: Simple even four point interpolation line Given the places @@ -132,9 +132,9 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 10 | 1 1, 1 1.001 | - | 10 | 14 | 1 1.001, 1.001 1.001 | - | 14 | 18 | 1.001 1.001, 1.001 1.002 | + | 4 | 8 | 1 1.00025, 1 1.00075 | + | 12 | 12 | 1.0005 1.001 | + | 16 | 16 | 1.001 1.0015 | Scenario: Reverse simple even three point interpolation line Given the places @@ -151,8 +151,8 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 10 | 1 1, 1 1.001 | - | 10 | 14 | 1 1.001, 1.001 1.001 | + | 4 | 8 | 1 1.00025, 1 1.00075 | + | 12 | 12 | 1.0005 1.001 | Scenario: Even three point interpolation line with odd center point Given the places @@ -169,8 +169,7 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 7 | 1 1, 1 1.001 | - | 7 | 8 | 1 1.001, 1.001 1.001 | + | 4 | 6 | 1 1.0004, 1 1.0008 | Scenario: Interpolation line with self-intersecting way Given the places @@ -187,9 +186,9 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 6 | 0 0, 0 0.001 | - | 6 | 10 | 0 0.001, 0 0.002 | - | 6 | 10 | 0 0.001, 0 0.002 | + | 4 | 4 | 0 0.0005 | + | 8 | 8 | 0 0.0015 | + | 8 | 8 | 0 0.0015 | Scenario: Interpolation line with self-intersecting way II Given the places @@ -205,7 +204,7 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 6 | 0 0, 0 0.001 | + | 4 | 4 | 0 0.0005 | Scenario: addr:street on interpolation way Given the scene parallel-road @@ -236,10 +235,10 @@ Feature: Import of address interpolations | N4 | W3 | Then W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | Then W11 expands to interpolation | parent_place_id | start | end | - | W3 | 12 | 16 | + | W3 | 14 | 14 | When sending search query "16 Cloud Street" Then results contain | ID | osm_type | osm_id | @@ -278,10 +277,10 @@ Feature: Import of address interpolations | N4 | W3 | Then W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | Then W11 expands to interpolation | parent_place_id | start | end | - | W3 | 12 | 16 | + | W3 | 14 | 14 | When sending search query "16 Cloud Street" Then results contain | ID | osm_type | osm_id | @@ -306,8 +305,8 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 6 | 144.9629794 -37.7630755, 144.9630541 -37.7628174 | - | 6 | 10 | 144.9630541 -37.7628174, 144.9632341 -37.76163 | + | 4 | 4 | 144.963016 -37.762946 | + | 8 | 8 | 144.963144 -37.7622237 | Scenario: Place with missing address information Given the grid @@ -326,7 +325,7 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 23 | 29 | 1,2,3 | + | 25 | 27 | 0.000016 0,0.00002 0,0.000033 0 | Scenario: Ways without node entries are ignored Given the places @@ -348,7 +347,7 @@ Feature: Import of address interpolations Given the places | osm | class | type | housenr | geometry | | N1 | place | house | 0 | 1 1 | - | N2 | place | house | 2 | 1 1.001 | + | N2 | place | house | 8 | 1 1.001 | And the places | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001 | @@ -358,9 +357,8 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 0 | 2 | 1 1, 1 1.001 | + | 2 | 6 | 1 0002, 1 1.0008 | When sending jsonv2 reverse coordinates 1,1 Then results contain | ID | osm_type | osm_id | type | display_name | | 0 | way | 1 | house | 0 | - diff --git a/test/bdd/db/update/interpolation.feature b/test/bdd/db/update/interpolation.feature index 27ac552e..5c07f434 100644 --- a/test/bdd/db/update/interpolation.feature +++ b/test/bdd/db/update/interpolation.feature @@ -26,7 +26,7 @@ Feature: Update of address interpolations | N2 | W2 | And W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | Scenario: addr:street added to interpolation Given the scene parallel-road @@ -51,7 +51,7 @@ Feature: Update of address interpolations | N2 | W2 | And W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | When updating places | osm | class | type | addr+interpolation | street | geometry | | W10 | place | houses | even | Cloud Street | :w-middle | @@ -61,7 +61,7 @@ Feature: Update of address interpolations | N2 | W3 | And W10 expands to interpolation | parent_place_id | start | end | - | W3 | 2 | 6 | + | W3 | 4 | 4 | Scenario: addr:street added to housenumbers Given the scene parallel-road @@ -86,7 +86,7 @@ Feature: Update of address interpolations | N2 | W2 | And W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | When updating places | osm | class | type | street | housenr | geometry | | N1 | place | house | Cloud Street| 2 | :n-middle-w | @@ -97,7 +97,7 @@ Feature: Update of address interpolations | N2 | W3 | And W10 expands to interpolation | parent_place_id | start | end | - | W3 | 2 | 6 | + | W3 | 4 | 4 | Scenario: interpolation tag removed Given the scene parallel-road @@ -122,7 +122,7 @@ Feature: Update of address interpolations | N2 | W2 | And W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | When marking for delete W10 Then W10 expands to no interpolation And placex contains @@ -152,7 +152,7 @@ Feature: Update of address interpolations | N2 | W2 | And W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | When updating places | osm | class | type | name | geometry | | W3 | highway | unclassified | Cloud Street | :w-south | @@ -162,7 +162,7 @@ Feature: Update of address interpolations | N2 | W3 | And W10 expands to interpolation | parent_place_id | start | end | - | W3 | 2 | 6 | + | W3 | 4 | 4 | Scenario: referenced road deleted Given the scene parallel-road @@ -187,7 +187,7 @@ Feature: Update of address interpolations | N2 | W3 | And W10 expands to interpolation | parent_place_id | start | end | - | W3 | 2 | 6 | + | W3 | 4 | 4 | When marking for delete W3 Then placex contains | object | parent_place_id | @@ -195,7 +195,7 @@ Feature: Update of address interpolations | N2 | W2 | And W10 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | Scenario: building becomes interpolation Given the scene building-with-parallel-streets @@ -222,7 +222,7 @@ Feature: Update of address interpolations Then placex has no entry for W1 And W1 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | Scenario: interpolation becomes building Given the scene building-with-parallel-streets @@ -243,7 +243,7 @@ Feature: Update of address interpolations Then placex has no entry for W1 And W1 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | When updating places | osm | class | type | housenr | geometry | | W1 | place | house | 3 | :w-building | @@ -273,7 +273,7 @@ Feature: Update of address interpolations | W1 | place | houses | even | Cloud Street| :w-north | Then W1 expands to interpolation | parent_place_id | start | end | - | W2 | 2 | 6 | + | W2 | 4 | 4 | Scenario: housenumber added in middle of interpolation Given the grid @@ -294,15 +294,15 @@ Feature: Update of address interpolations | N5 | place | house | 10 | When importing Then W2 expands to interpolation - | parent_place_id | start | end | geometry | - | W1 | 2 | 10 | 3,4,5 | + | parent_place_id | start | end | + | W1 | 4 | 8 | When updating places | osm | class | type | housenr | | N4 | place | house | 6 | Then W2 expands to interpolation - | parent_place_id | start | end | geometry | - | W1 | 2 | 6 | 3,4 | - | W1 | 6 | 10 | 4,5 | + | parent_place_id | start | end | + | W1 | 4 | 4 | + | W1 | 8 | 8 | @Fail Scenario: housenumber removed in middle of interpolation @@ -325,13 +325,13 @@ Feature: Update of address interpolations | N5 | place | house | 10 | When importing Then W2 expands to interpolation - | parent_place_id | start | end | geometry | - | W1 | 2 | 6 | 3,4 | - | W1 | 6 | 10 | 4,5 | + | parent_place_id | start | end | + | W1 | 4 | 4 | + | W1 | 8 | 8 | When marking for delete N4 Then W2 expands to interpolation - | parent_place_id | start | end | geometry | - | W1 | 2 | 10 | 3,4,5 | + | parent_place_id | start | end | + | W1 | 4 | 8 | Scenario: Change the start housenumber Given the grid @@ -352,12 +352,12 @@ Feature: Update of address interpolations | N4 | place | house | 6 | When importing Then W2 expands to interpolation - | parent_place_id | start | end | geometry | - | W1 | 2 | 6 | 3,4 | + | parent_place_id | start | end | + | W1 | 4 | 4 | When updating places | osm | class | type | housenr | | N4 | place | house | 8 | Then W2 expands to interpolation - | parent_place_id | start | end | geometry | - | W1 | 2 | 8 | 3,4 | + | parent_place_id | start | end | + | W1 | 4 | 6 | From 6b89624f339dbf36bd668d83b3d0b12edc62f616 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Wed, 26 Jan 2022 21:24:24 +0100 Subject: [PATCH 08/13] adapt frontend to new interpolation table layout --- lib-php/PlaceLookup.php | 2 +- lib-php/ReverseGeocode.php | 10 +++++++--- lib-php/SearchDescription.php | 15 +++------------ test/bdd/db/import/interpolation.feature | 6 +++--- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/lib-php/PlaceLookup.php b/lib-php/PlaceLookup.php index 09acb544..0e4b63bc 100644 --- a/lib-php/PlaceLookup.php +++ b/lib-php/PlaceLookup.php @@ -405,7 +405,7 @@ class PlaceLookup $sSQL .= ' CASE '; // interpolate the housenumbers here $sSQL .= ' WHEN startnumber != endnumber '; $sSQL .= ' THEN ST_LineInterpolatePoint(linegeo, (housenumber_for_place-startnumber::float)/(endnumber-startnumber)::float) '; - $sSQL .= ' ELSE ST_LineInterpolatePoint(linegeo, 0.5) '; + $sSQL .= ' ELSE linegeo '; $sSQL .= ' END as centroid, '; $sSQL .= ' parent_place_id, '; $sSQL .= ' housenumber_for_place '; diff --git a/lib-php/ReverseGeocode.php b/lib-php/ReverseGeocode.php index 5a852889..cccd5b35 100644 --- a/lib-php/ReverseGeocode.php +++ b/lib-php/ReverseGeocode.php @@ -64,8 +64,8 @@ class ReverseGeocode { Debug::newFunction('lookupInterpolation'); $sSQL = 'SELECT place_id, parent_place_id, 30 as rank_search,'; - $sSQL .= ' ST_LineLocatePoint(linegeo,'.$sPointSQL.') as fraction,'; - $sSQL .= ' startnumber, endnumber, interpolationtype,'; + $sSQL .= ' (endnumber - startnumber) * ST_LineLocatePoint(linegeo,'.$sPointSQL.') as fhnr,'; + $sSQL .= ' startnumber, endnumber, step,'; $sSQL .= ' ST_Distance(linegeo,'.$sPointSQL.') as distance'; $sSQL .= ' FROM location_property_osmline'; $sSQL .= ' WHERE ST_DWithin('.$sPointSQL.', linegeo, '.$fSearchDiam.')'; @@ -363,7 +363,11 @@ class ReverseGeocode if ($aHouse) { $oResult = new Result($aHouse['place_id'], Result::TABLE_OSMLINE); - $oResult->iHouseNumber = closestHouseNumber($aHouse); + $iRndNum = max(0, round($aHouse['fhnr'] / $aHouse['step']) * $aHouse['step']); + $oResult->iHouseNumber = $aHouse['startnumber'] + $iRndNum; + if ($oResult->iHouseNumber > $aHouse['endnumber']) { + $oResult->iHouseNumber = $aHouse['endnumber']; + } $aPlace = $aHouse; } } diff --git a/lib-php/SearchDescription.php b/lib-php/SearchDescription.php index 089f09c0..d6fa704f 100644 --- a/lib-php/SearchDescription.php +++ b/lib-php/SearchDescription.php @@ -769,18 +769,9 @@ class SearchDescription // if nothing found, search in the interpolation line table $sSQL = 'SELECT distinct place_id FROM location_property_osmline'; $sSQL .= ' WHERE startnumber is not NULL'; - $sSQL .= ' AND parent_place_id in ('.$sRoadPlaceIDs.') AND ('; - if ($iHousenumber % 2 == 0) { - // If housenumber is even, look for housenumber in streets - // with interpolationtype even or all. - $sSQL .= "interpolationtype='even'"; - } else { - // Else look for housenumber with interpolationtype odd or all. - $sSQL .= "interpolationtype='odd'"; - } - $sSQL .= " or interpolationtype='all') and "; - $sSQL .= $iHousenumber.'>=startnumber and '; - $sSQL .= $iHousenumber.'<=endnumber'; + $sSQL .= ' and parent_place_id in ('.$sRoadPlaceIDs.')'; + $sSQL .= ' and ('.$iHousenumber.' - startnumber) % step = 0'; + $sSQL .= ' and '.$iHousenumber.' between startnumber and endnumber'; $sSQL .= $this->oContext->excludeSQL(' AND place_id'); Debug::printSQL($sSQL); diff --git a/test/bdd/db/import/interpolation.feature b/test/bdd/db/import/interpolation.feature index 4624705e..54d22962 100644 --- a/test/bdd/db/import/interpolation.feature +++ b/test/bdd/db/import/interpolation.feature @@ -347,7 +347,7 @@ Feature: Import of address interpolations Given the places | osm | class | type | housenr | geometry | | N1 | place | house | 0 | 1 1 | - | N2 | place | house | 8 | 1 1.001 | + | N2 | place | house | 10 | 1 1.001 | And the places | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001 | @@ -357,8 +357,8 @@ Feature: Import of address interpolations When importing Then W1 expands to interpolation | start | end | geometry | - | 2 | 6 | 1 0002, 1 1.0008 | + | 2 | 8 | 1 1.0002, 1 1.0008 | When sending jsonv2 reverse coordinates 1,1 Then results contain | ID | osm_type | osm_id | type | display_name | - | 0 | way | 1 | house | 0 | + | 0 | node | 1 | house | 0 | From 98432395c31f0ae46962907e2c19c4eebc12c8bb Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Thu, 27 Jan 2022 11:41:16 +0100 Subject: [PATCH 09/13] add migration for upcoming change to tiger tables --- nominatim/tools/migration.py | 14 +++++++++++++- nominatim/version.py | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/nominatim/tools/migration.py b/nominatim/tools/migration.py index c24b09cb..dc6dfeca 100644 --- a/nominatim/tools/migration.py +++ b/nominatim/tools/migration.py @@ -231,7 +231,7 @@ def create_interpolation_index_on_place(conn, **_): @_migration(4, 0, 99, 2) def add_step_column_for_interpolation(conn, **_): - """ Add a new column 'step' to the interpolations table which will. + """ Add a new column 'step' to the interpolations table. Also convers the data into the stricter format which requires that startnumbers comply with the odd/even requirements. @@ -259,3 +259,15 @@ def add_step_column_for_interpolation(conn, **_): SET step = CASE WHEN interpolationtype = 'all' THEN 1 ELSE 2 END """) + + +@_migration(4, 0, 99, 3) +def add_step_column_for_tiger(conn, **_): + """ Add a new column 'step' to the tiger data table. + """ + with conn.cursor() as cur: + cur.execute("ALTER TABLE location_property_tiger ADD COLUMN step SMALLINT") + cur.execute("""UPDATE location_property_tiger + SET step = CASE WHEN interpolationtype = 'all' + THEN 1 ELSE 2 END + """) diff --git a/nominatim/version.py b/nominatim/version.py index 88bb881f..289af934 100644 --- a/nominatim/version.py +++ b/nominatim/version.py @@ -24,7 +24,7 @@ Version information for Nominatim. # patch level when cherry-picking the commit with the migration. # # Released versions always have a database patch level of 0. -NOMINATIM_VERSION = (4, 0, 99, 3) +NOMINATIM_VERSION = (4, 0, 99, 4) POSTGRESQL_REQUIRED_VERSION = (9, 5) POSTGIS_REQUIRED_VERSION = (2, 2) From 788505095e9710369537f4b61cb4d40d8e5af580 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Thu, 27 Jan 2022 11:54:12 +0100 Subject: [PATCH 10/13] add step column to tiger data table This replaces the interpolationtype column. --- lib-sql/tables.sql | 2 +- lib-sql/tiger_import_finish.sql | 2 +- lib-sql/tiger_import_start.sql | 23 +++++++++++++++-------- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib-sql/tables.sql b/lib-sql/tables.sql index 0c0f78fc..f554c58f 100644 --- a/lib-sql/tables.sql +++ b/lib-sql/tables.sql @@ -80,9 +80,9 @@ CREATE TABLE location_property_tiger ( parent_place_id BIGINT, startnumber INTEGER, endnumber INTEGER, + step SMALLINT, partition SMALLINT, linegeo GEOMETRY, - interpolationtype TEXT, postcode TEXT); GRANT SELECT ON location_property_tiger TO "{{config.DATABASE_WEBUSER}}"; diff --git a/lib-sql/tiger_import_finish.sql b/lib-sql/tiger_import_finish.sql index afe69c37..c02ce2a3 100644 --- a/lib-sql/tiger_import_finish.sql +++ b/lib-sql/tiger_import_finish.sql @@ -9,7 +9,7 @@ CREATE INDEX IF NOT EXISTS idx_location_property_tiger_parent_place_id_imp ON location_property_tiger_import (parent_place_id) {% if postgres.has_index_non_key_column %} - INCLUDE (startnumber, endnumber) + INCLUDE (startnumber, endnumber, step) {% endif %} {{db.tablespace.aux_index}}; CREATE UNIQUE INDEX IF NOT EXISTS idx_location_property_tiger_place_id_imp diff --git a/lib-sql/tiger_import_start.sql b/lib-sql/tiger_import_start.sql index 0ad38fbb..84992dfd 100644 --- a/lib-sql/tiger_import_start.sql +++ b/lib-sql/tiger_import_start.sql @@ -24,11 +24,12 @@ DECLARE BEGIN IF in_endnumber > in_startnumber THEN - startnumber = in_startnumber; - endnumber = in_endnumber; + startnumber := in_startnumber; + endnumber := in_endnumber; ELSE - startnumber = in_endnumber; - endnumber = in_startnumber; + startnumber := in_endnumber; + endnumber := in_startnumber; + linegeo := ST_Reverse(linegeo); END IF; IF startnumber < 0 THEN @@ -50,8 +51,10 @@ BEGIN END IF; -- Filter out really broken tiger data - IF numberrange > 0 AND (numberrange::float/stepsize::float > 500) - AND ST_length(linegeo)/(numberrange::float/stepsize::float) < 0.000001 THEN + IF numberrange > 0 + and numberrange::float/stepsize::float > 500 + and ST_length(linegeo)/(numberrange::float/stepsize::float) < 0.000001 + THEN RAISE WARNING 'Road too short for number range % to % (%)',startnumber,endnumber, ST_length(linegeo)/(numberrange::float/stepsize::float); RETURN 0; @@ -74,8 +77,12 @@ BEGIN END IF; --insert street(line) into import table -insert into location_property_tiger_import (linegeo, place_id, partition, parent_place_id, startnumber, endnumber, interpolationtype, postcode) -values (linegeo, nextval('seq_place'), out_partition, out_parent_place_id, startnumber, endnumber, interpolationtype, in_postcode); +insert into location_property_tiger_import (linegeo, place_id, partition, + parent_place_id, startnumber, endnumber, + step, postcode) +values (linegeo, nextval('seq_place'), out_partition, + out_parent_place_id, startnumber, endnumber, + stepsize, in_postcode); RETURN 1; END; From 64abc90d3072bbfbc9aae316f5ef709b8782532e Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Thu, 27 Jan 2022 14:08:08 +0100 Subject: [PATCH 11/13] use new tiger step column for queries --- lib-php/PlaceLookup.php | 4 +++- lib-php/ReverseGeocode.php | 12 ++++++++---- lib-php/SearchDescription.php | 12 +++--------- lib-php/lib.php | 20 -------------------- lib-sql/tiger_import_start.sql | 10 +++++++++- test/bdd/api/reverse/queries.feature | 2 +- 6 files changed, 24 insertions(+), 36 deletions(-) diff --git a/lib-php/PlaceLookup.php b/lib-php/PlaceLookup.php index 0e4b63bc..120f5543 100644 --- a/lib-php/PlaceLookup.php +++ b/lib-php/PlaceLookup.php @@ -348,7 +348,9 @@ class PlaceLookup $sSQL .= ' null::text AS extra_place '; $sSQL .= ' FROM ('; $sSQL .= ' SELECT place_id, '; // interpolate the Tiger housenumbers here - $sSQL .= ' ST_LineInterpolatePoint(linegeo, (housenumber_for_place-startnumber::float)/(endnumber-startnumber)::float) AS centroid, '; + $sSQL .= ' CASE WHEN startnumber != endnumber'; + $sSQL .= ' THEN ST_LineInterpolatePoint(linegeo, (housenumber_for_place-startnumber::float)/(endnumber-startnumber)::float)'; + $sSQL .= ' ELSE ST_LineInterpolatePoint(linegeo, 0.5) END AS centroid, '; $sSQL .= ' parent_place_id, '; $sSQL .= ' housenumber_for_place'; $sSQL .= ' FROM ('; diff --git a/lib-php/ReverseGeocode.php b/lib-php/ReverseGeocode.php index cccd5b35..646c592b 100644 --- a/lib-php/ReverseGeocode.php +++ b/lib-php/ReverseGeocode.php @@ -327,9 +327,9 @@ class ReverseGeocode && $this->iMaxRank >= 28 ) { $sSQL = 'SELECT place_id,parent_place_id,30 as rank_search,'; - $sSQL .= 'ST_LineLocatePoint(linegeo,'.$sPointSQL.') as fraction,'; - $sSQL .= 'ST_distance('.$sPointSQL.', linegeo) as distance,'; - $sSQL .= 'startnumber,endnumber,interpolationtype'; + $sSQL .= ' (endnumber - startnumber) * ST_LineLocatePoint(linegeo,'.$sPointSQL.') as fhnr,'; + $sSQL .= ' startnumber, endnumber, step,'; + $sSQL .= ' ST_Distance('.$sPointSQL.', linegeo) as distance'; $sSQL .= ' FROM location_property_tiger WHERE parent_place_id = '.$oResult->iId; $sSQL .= ' AND ST_DWithin('.$sPointSQL.', linegeo, 0.001)'; $sSQL .= ' ORDER BY distance ASC limit 1'; @@ -341,7 +341,11 @@ class ReverseGeocode if ($aPlaceTiger) { $aPlace = $aPlaceTiger; $oResult = new Result($aPlaceTiger['place_id'], Result::TABLE_TIGER); - $oResult->iHouseNumber = closestHouseNumber($aPlaceTiger); + $iRndNum = max(0, round($aPlaceTiger['fhnr'] / $aPlaceTiger['step']) * $aPlaceTiger['step']); + $oResult->iHouseNumber = $aPlaceTiger['startnumber'] + $iRndNum; + if ($oResult->iHouseNumber > $aPlaceTiger['endnumber']) { + $oResult->iHouseNumber = $aPlaceTiger['endnumber']; + } $iRankAddress = 30; } } diff --git a/lib-php/SearchDescription.php b/lib-php/SearchDescription.php index d6fa704f..d2995e8e 100644 --- a/lib-php/SearchDescription.php +++ b/lib-php/SearchDescription.php @@ -786,15 +786,9 @@ class SearchDescription // If nothing found then search in Tiger data (location_property_tiger) if (CONST_Use_US_Tiger_Data && $sRoadPlaceIDs && $bIsIntHouseNumber && empty($aResults)) { $sSQL = 'SELECT place_id FROM location_property_tiger'; - $sSQL .= ' WHERE parent_place_id in ('.$sRoadPlaceIDs.') and ('; - if ($iHousenumber % 2 == 0) { - $sSQL .= "interpolationtype='even'"; - } else { - $sSQL .= "interpolationtype='odd'"; - } - $sSQL .= " or interpolationtype='all') and "; - $sSQL .= $iHousenumber.'>=startnumber and '; - $sSQL .= $iHousenumber.'<=endnumber'; + $sSQL .= ' WHERE parent_place_id in ('.$sRoadPlaceIDs.')'; + $sSQL .= ' and ('.$iHousenumber.' - startnumber) % step = 0'; + $sSQL .= ' and '.$iHousenumber.' between startnumber and endnumber'; $sSQL .= $this->oContext->excludeSQL(' AND place_id'); Debug::printSQL($sSQL); diff --git a/lib-php/lib.php b/lib-php/lib.php index 3ba50dc0..9babe5ed 100644 --- a/lib-php/lib.php +++ b/lib-php/lib.php @@ -206,26 +206,6 @@ function parseLatLon($sQuery) return array($sFound, $fQueryLat, $fQueryLon); } -function closestHouseNumber($aRow) -{ - $fHouse = $aRow['startnumber'] - + ($aRow['endnumber'] - $aRow['startnumber']) * $aRow['fraction']; - - switch ($aRow['interpolationtype']) { - case 'odd': - $iHn = (int)($fHouse/2) * 2 + 1; - break; - case 'even': - $iHn = (int)(round($fHouse/2)) * 2; - break; - default: - $iHn = (int)(round($fHouse)); - break; - } - - return max(min($aRow['endnumber'], $iHn), $aRow['startnumber']); -} - if (!function_exists('array_key_last')) { function array_key_last(array $array) { diff --git a/lib-sql/tiger_import_start.sql b/lib-sql/tiger_import_start.sql index 84992dfd..0ff53436 100644 --- a/lib-sql/tiger_import_start.sql +++ b/lib-sql/tiger_import_start.sql @@ -5,7 +5,15 @@ -- Copyright (C) 2022 by the Nominatim developer community. -- For a full list of authors see the git log. DROP TABLE IF EXISTS location_property_tiger_import; -CREATE TABLE location_property_tiger_import (linegeo GEOMETRY, place_id BIGINT, partition INTEGER, parent_place_id BIGINT, startnumber INTEGER, endnumber INTEGER, interpolationtype TEXT, postcode TEXT); +CREATE TABLE location_property_tiger_import ( + linegeo GEOMETRY, + place_id BIGINT, + partition INTEGER, + parent_place_id BIGINT, + startnumber INTEGER, + endnumber INTEGER, + step SMALLINT, + postcode TEXT); CREATE OR REPLACE FUNCTION tiger_line_import(linegeo GEOMETRY, in_startnumber INTEGER, in_endnumber INTEGER, interpolationtype TEXT, diff --git a/test/bdd/api/reverse/queries.feature b/test/bdd/api/reverse/queries.feature index 303af2c3..a2b0f033 100644 --- a/test/bdd/api/reverse/queries.feature +++ b/test/bdd/api/reverse/queries.feature @@ -10,7 +10,7 @@ Feature: Reverse geocoding | way | place | house | And result addresses contain | house_number | road | postcode | country_code | - | 697 | Upper Kingston Road | 36067 | us | + | 707 | Upper Kingston Road | 36067 | us | @Tiger Scenario: No TIGER house number for zoom < 18 From 2ffc1537e7a7e37eaec4af81fd2176fc1ae69bce Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Thu, 27 Jan 2022 15:15:56 +0100 Subject: [PATCH 12/13] raise PostgreSQL requirement to 9.6 The new code uses the open-ended array notation which is only available sind psql 9.6. --- .github/workflows/ci-tests.yml | 2 +- docs/admin/Faq.md | 2 +- docs/admin/Installation.md | 2 +- module/CMakeLists.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index f326c3ca..6d474a2e 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -40,7 +40,7 @@ jobs: ubuntu: [18, 20] include: - ubuntu: 18 - postgresql: 9.5 + postgresql: 9.6 postgis: 2.5 pytest: pytest php: 7.2 diff --git a/docs/admin/Faq.md b/docs/admin/Faq.md index d933a84d..5737cef5 100644 --- a/docs/admin/Faq.md +++ b/docs/admin/Faq.md @@ -79,7 +79,7 @@ When running the import you may get a version mismatch: pg_config seems to use bad includes sometimes when multiple versions of PostgreSQL are available in the system. Make sure you remove the -server development libraries (`postgresql-server-dev-9.5` on Ubuntu) +server development libraries (`postgresql-server-dev-13` on Ubuntu) and recompile (`cmake .. && make`). diff --git a/docs/admin/Installation.md b/docs/admin/Installation.md index 6b63b0d3..19ad2dbb 100644 --- a/docs/admin/Installation.md +++ b/docs/admin/Installation.md @@ -41,7 +41,7 @@ For compiling: For running Nominatim: - * [PostgreSQL](https://www.postgresql.org) (9.5+ will work, 11+ strongly recommended) + * [PostgreSQL](https://www.postgresql.org) (9.6+ will work, 11+ strongly recommended) * [PostGIS](https://postgis.net) (2.2+ will work, 3.0+ strongly recommended) * [Python 3](https://www.python.org/) (3.6+) * [Psycopg2](https://www.psycopg.org) (2.7+) diff --git a/module/CMakeLists.txt b/module/CMakeLists.txt index 6aef6a5a..9684a817 100644 --- a/module/CMakeLists.txt +++ b/module/CMakeLists.txt @@ -1,6 +1,6 @@ # just use the pgxs makefile -foreach(suffix ${PostgreSQL_ADDITIONAL_VERSIONS} "13" "12" "11" "10" "9.6" "9.5" "9.4" "9.3") +foreach(suffix ${PostgreSQL_ADDITIONAL_VERSIONS} "14" "13" "12" "11" "10" "9.6") list(APPEND PG_CONFIG_HINTS "/usr/pgsql-${suffix}/bin") endforeach() From b6fa121f530f85736b7645afdd1ae20fe50d0de3 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Thu, 27 Jan 2022 16:21:45 +0100 Subject: [PATCH 13/13] remove tests for closest housenumber function --- test/php/Nominatim/LibTest.php | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/test/php/Nominatim/LibTest.php b/test/php/Nominatim/LibTest.php index 249adce2..5d711240 100644 --- a/test/php/Nominatim/LibTest.php +++ b/test/php/Nominatim/LibTest.php @@ -91,33 +91,4 @@ class LibTest extends \PHPUnit\Framework\TestCase $this->assertEquals($sQuery, $aRes[0]); } } - - private function closestHouseNumberEvenOddOther($startnumber, $endnumber, $fraction, $aExpected) - { - foreach (array('even', 'odd', 'other') as $itype) { - $this->assertEquals( - $aExpected[$itype], - closestHouseNumber(array( - 'startnumber' => $startnumber, - 'endnumber' => $endnumber, - 'fraction' => $fraction, - 'interpolationtype' => $itype - )), - "$startnumber => $endnumber, $fraction, $itype" - ); - } - } - - public function testClosestHouseNumber() - { - $this->closestHouseNumberEvenOddOther(50, 100, 0.5, array('even' => 76, 'odd' => 75, 'other' => 75)); - // upper bound - $this->closestHouseNumberEvenOddOther(50, 100, 1.5, array('even' => 100, 'odd' => 100, 'other' => 100)); - // lower bound - $this->closestHouseNumberEvenOddOther(50, 100, -0.5, array('even' => 50, 'odd' => 50, 'other' => 50)); - // fraction 0 - $this->closestHouseNumberEvenOddOther(50, 100, 0, array('even' => 50, 'odd' => 51, 'other' => 50)); - // start == end - $this->closestHouseNumberEvenOddOther(50, 50, 0.5, array('even' => 50, 'odd' => 50, 'other' => 50)); - } }