From: Sarah Hoffmann Date: Wed, 4 Mar 2020 07:08:39 +0000 (+0100) Subject: Merge pull request #1702 from lonvia/rename-derived-place X-Git-Tag: v3.5.0~70 X-Git-Url: https://git.openstreetmap.org/nominatim.git/commitdiff_plain/a00ea23847b37b7e953c556475a1946681e33fe0?hp=b00d16fd7d41b41eb41128fdc4866dd3d7a67a95 Merge pull request #1702 from lonvia/rename-derived-place Make admin boundaries inherit address rank from place nodes --- diff --git a/lib/Geocode.php b/lib/Geocode.php index f45b2caa..ab9446e0 100644 --- a/lib/Geocode.php +++ b/lib/Geocode.php @@ -808,9 +808,7 @@ class Geocode $sSQL .= 'WHERE place_id in ('.$sPlaceIds.') '; $sSQL .= ' AND ('; $sSQL .= " placex.rank_address between $this->iMinAddressRank and $this->iMaxAddressRank "; - if (14 >= $this->iMinAddressRank && 14 <= $this->iMaxAddressRank) { - $sSQL .= " OR (extratags->'place') = 'city'"; - } + $sSQL .= " OR placex.rank_search between $this->iMinAddressRank and $this->iMaxAddressRank "; if ($this->aAddressRankList) { $sSQL .= ' OR placex.rank_address in ('.join(',', $this->aAddressRankList).')'; } diff --git a/lib/PlaceLookup.php b/lib/PlaceLookup.php index fce49701..9f4f143e 100644 --- a/lib/PlaceLookup.php +++ b/lib/PlaceLookup.php @@ -215,7 +215,7 @@ class PlaceLookup 'ST_Collect(centroid)', 'min(CASE WHEN placex.rank_search < 28 THEN placex.place_id ELSE placex.parent_place_id END)' ); - $sSQL .= " (extratags->'place') AS extra_place "; + $sSQL .= " COALESCE(extratags->'place', extratags->'linked_place') AS extra_place "; $sSQL .= ' FROM placex'; $sSQL .= " WHERE place_id in ($sPlaceIDs) "; $sSQL .= ' AND ('; @@ -248,7 +248,7 @@ class PlaceLookup $sSQL .= ' ref, '; if ($this->bExtraTags) $sSQL .= 'extratags, '; if ($this->bNameDetails) $sSQL .= 'name, '; - $sSQL .= " extratags->'place' "; + $sSQL .= ' extra_place '; $aSubSelects[] = $sSQL; } diff --git a/lib/SearchDescription.php b/lib/SearchDescription.php index 4fafbec2..bb478b29 100644 --- a/lib/SearchDescription.php +++ b/lib/SearchDescription.php @@ -660,10 +660,7 @@ class SearchDescription $aTerms[] = 'address_rank between 16 and 27'; } elseif (!$this->sClass || $this->iOperator == Operator::NAME) { if ($iMinAddressRank > 0) { - $aTerms[] = 'address_rank >= '.$iMinAddressRank; - } - if ($iMaxAddressRank < 30) { - $aTerms[] = 'address_rank <= '.$iMaxAddressRank; + $aTerms[] = "((address_rank between $iMinAddressRank and $iMaxAddressRank) or (search_rank between $iMinAddressRank and $iMaxAddressRank))"; } } diff --git a/sql/functions/address_lookup.sql b/sql/functions/address_lookup.sql index b1b4c4da..8436fdb1 100644 --- a/sql/functions/address_lookup.sql +++ b/sql/functions/address_lookup.sql @@ -202,8 +202,9 @@ BEGIN FOR location IN SELECT placex.place_id, osm_type, osm_id, name, - CASE WHEN extratags ? 'place' THEN 'place' ELSE class END as class, - CASE WHEN extratags ? 'place' THEN extratags->'place' ELSE type END as type, + CASE WHEN extratags ? 'place' or extratags ? 'linked_place' + THEN 'place' ELSE class END as class, + coalesce(extratags->'place', extratags->'linked_place', type) as type, admin_level, fromarea, isaddress, CASE WHEN rank_address = 11 THEN 5 ELSE rank_address END as rank_address, distance, country_code, postcode diff --git a/sql/functions/placex_triggers.sql b/sql/functions/placex_triggers.sql index ffc83fa3..e16b7783 100644 --- a/sql/functions/placex_triggers.sql +++ b/sql/functions/placex_triggers.sql @@ -204,9 +204,26 @@ BEGIN END IF; END IF; + -- If extratags has a place tag, look for linked nodes by their place type. + -- Area and node still have to have the same name. + IF bnd.extratags ? 'place' and bnd_name is not null THEN + FOR linked_placex IN + SELECT * FROM placex + WHERE make_standard_name(name->'name') = bnd_name + AND placex.class = 'place' AND placex.type = bnd.extratags->'place' + AND placex.osm_type = 'N' + AND placex.rank_search < 26 -- needed to select the right index + AND _st_covers(bnd.geometry, placex.geometry) + LOOP + --DEBUG: RAISE WARNING 'Found type-matching place node %', linked_placex.osm_id; + RETURN linked_placex; + END LOOP; + END IF; + -- Search for relation members with role admin_center. IF bnd.osm_type = 'R' and bnd_name is not null - and relation_members is not null THEN + and relation_members is not null + THEN FOR rel_member IN SELECT get_rel_node_members(relation_members, ARRAY['admin_center','admin_centre']) as member @@ -231,7 +248,7 @@ BEGIN END IF; -- Name searches can be done for ways as well as relations - IF bnd.osm_type in ('W','R') and bnd_name is not null THEN + IF bnd_name is not null THEN --DEBUG: RAISE WARNING 'Looking for nodes with matching names'; FOR linked_placex IN SELECT placex.* from placex @@ -241,7 +258,7 @@ BEGIN AND placex.rank_search < 26 -- needed to select the right index AND _st_covers(bnd.geometry, placex.geometry) LOOP - --DEBUG: RAISE WARNING 'Found matching place node %', linkedPlacex.osm_id; + --DEBUG: RAISE WARNING 'Found matching place node %', linked_placex.osm_id; RETURN linked_placex; END LOOP; END IF; @@ -818,19 +835,24 @@ BEGIN --DEBUG: RAISE WARNING 'Using full index mode for % %', NEW.osm_type, NEW.osm_id; SELECT * INTO location FROM find_linked_place(NEW); IF location.place_id is not null THEN - --DEBUG: RAISE WARNING 'Linked %', location; + --DEBUG: RAISE WARNING 'Linked %', location; -- Use this as the centre point of the geometry NEW.centroid := coalesce(location.centroid, ST_Centroid(location.geometry)); + -- Use the address rank of the linked place, if it has one + IF location.rank_address between 5 and 25 THEN + NEW.rank_address := location.rank_address; + END IF; + -- merge in the label name IF NOT location.name IS NULL THEN NEW.name := location.name || NEW.name; END IF; -- merge in extra tags - NEW.extratags := hstore(location.class, location.type) + NEW.extratags := hstore('linked_' || location.class, location.type) || coalesce(location.extratags, ''::hstore) || coalesce(NEW.extratags, ''::hstore); diff --git a/sql/partition-functions.src.sql b/sql/partition-functions.src.sql index 8f78032e..34b4d390 100644 --- a/sql/partition-functions.src.sql +++ b/sql/partition-functions.src.sql @@ -20,7 +20,8 @@ BEGIN FOR r IN SELECT place_id, keywords, rank_address, rank_search, min(ST_Distance(feature, centroid)) as distance, isguess, postcode, centroid FROM location_area_large_-partition- - WHERE ST_Intersects(geometry, feature) and rank_search < maxrank + WHERE ST_Intersects(geometry, feature) + AND rank_search < maxrank AND rank_address < maxrank GROUP BY place_id, keywords, rank_address, rank_search, isguess, postcode, centroid ORDER BY rank_address, isin_tokens && keywords desc, isguess asc, ST_Distance(feature, centroid) * diff --git a/test/bdd/db/import/linking.feature b/test/bdd/db/import/linking.feature index ceb45586..1bca7dfc 100644 --- a/test/bdd/db/import/linking.feature +++ b/test/bdd/db/import/linking.feature @@ -127,3 +127,74 @@ Feature: Linking of places | N3:natural | - | | N3:place | R1 | + Scenario: Nodes with 'role' label are always linked + Given the places + | osm | class | type | admin | name | geometry | + | R13 | boundary | administrative | 6 | Garbo | poly-area:0.1 | + | N2 | place | hamlet | 15 | Vario | 0.006 0.00001 | + And the relations + | id | members | tags+type | + | 13 | N2:label | boundary | + When importing + Then placex contains + | object | linked_place_id | + | N2 | R13 | + And placex contains + | object | centroid | name+name | extratags+linked_place | + | R13 | 0.006 0.00001 | Garbo | hamlet | + + Scenario: Boundaries with place tags are linked against places with same type + Given the places + | osm | class | type | admin | name | extra+place | geometry | + | R13 | boundary | administrative | 4 | Berlin | city |poly-area:0.1 | + And the places + | osm | class | type | name | geometry | + | N2 | place | city | Berlin | 0.006 0.00001 | + When importing + Then placex contains + | object | linked_place_id | + | N2 | R13 | + And placex contains + | object | rank_address | + | R13 | 16 | + When searching for "" + | city | + | Berlin | + Then results contain + | ID | osm_type | osm_id | + | 0 | R | 13 | + When searching for "" + | state | + | Berlin | + Then results contain + | ID | osm_type | osm_id | + | 0 | R | 13 | + + + Scenario: Boundaries without place tags only link against same admin level + Given the places + | osm | class | type | admin | name | geometry | + | R13 | boundary | administrative | 4 | Berlin |poly-area:0.1 | + And the places + | osm | class | type | name | geometry | + | N2 | place | city | Berlin | 0.006 0.00001 | + When importing + Then placex contains + | object | linked_place_id | + | N2 | - | + And placex contains + | object | rank_address | + | R13 | 8 | + When searching for "" + | state | + | Berlin | + Then results contain + | ID | osm_type | osm_id | + | 0 | R | 13 | + When searching for "" + | city | + | Berlin | + Then results contain + | ID | osm_type | osm_id | + | 0 | N | 2 | + diff --git a/test/bdd/db/update/linked_places.feature b/test/bdd/db/update/linked_places.feature index 6c31bd89..647d5eaf 100644 --- a/test/bdd/db/update/linked_places.feature +++ b/test/bdd/db/update/linked_places.feature @@ -125,11 +125,11 @@ Feature: Updates of linked places When importing Then placex contains | object | extratags | - | R1 | 'wikidata' : '34', 'place' : 'city' | + | R1 | 'wikidata' : '34', 'linked_place' : 'city' | When updating places | osm | class | type | name | extra+oneway | admin | geometry | | N3 | place | city | newname | yes | 30 | 0.00001 0 | Then placex contains | object | extratags | - | R1 | 'wikidata' : '34', 'oneway' : 'yes', 'place' : 'city' | + | R1 | 'wikidata' : '34', 'oneway' : 'yes', 'linked_place' : 'city' | diff --git a/test/bdd/steps/queries.py b/test/bdd/steps/queries.py index 3f0cffff..4e6ec1ff 100644 --- a/test/bdd/steps/queries.py +++ b/test/bdd/steps/queries.py @@ -297,7 +297,8 @@ def query_cmd(context, query, dups): """ cmd = ['/usr/bin/env', 'php'] cmd.append(os.path.join(context.nominatim.build_dir, 'utils', 'query.php')) - cmd.extend(['--search', query]) + if query: + cmd.extend(['--search', query]) # add more parameters in table form if context.table: for h in context.table.headings: