From: Sarah Hoffmann Date: Mon, 6 Jun 2022 07:30:10 +0000 (+0200) Subject: Merge remote-tracking branch 'upstream/master' X-Git-Tag: deploy~106 X-Git-Url: https://git.openstreetmap.org./nominatim.git/commitdiff_plain/bd951819916c9357e82dcb4c84c06064772c36f4?hp=18800a1a82f29d6d1dda177dcd60715b2c060fb2 Merge remote-tracking branch 'upstream/master' --- diff --git a/lib-php/template/address-geocodejson.php b/lib-php/template/address-geocodejson.php index 584e27cf..8a0a6289 100644 --- a/lib-php/template/address-geocodejson.php +++ b/lib-php/template/address-geocodejson.php @@ -36,7 +36,7 @@ if (empty($aPlace)) { $aFilteredPlaces['properties']['geocoding']['osm_id'] = $aPlace['osm_id']; } - $aFilteredPlaces['properties']['geocoding']['type'] = $aPlace['type']; + $aFilteredPlaces['properties']['geocoding']['type'] = addressRankToGeocodeJsonType($aPlace['rank_address']); $aFilteredPlaces['properties']['geocoding']['accuracy'] = (int) $fDistance; diff --git a/lib-sql/functions/placex_triggers.sql b/lib-sql/functions/placex_triggers.sql index 92c0c4ec..50467cf5 100644 --- a/lib-sql/functions/placex_triggers.sql +++ b/lib-sql/functions/placex_triggers.sql @@ -773,12 +773,6 @@ BEGIN and (linked_place is null or linked_place_id != linked_place); -- update not necessary for osmline, cause linked_place_id does not exist - IF NEW.linked_place_id is not null THEN - NEW.token_info := null; - {% if debug %}RAISE WARNING 'place already linked to %', OLD.linked_place_id;{% endif %} - RETURN NEW; - END IF; - -- Postcodes are just here to compute the centroids. They are not searchable -- unless they are a boundary=postal_code. -- There was an error in the style so that boundary=postal_code used to be @@ -823,6 +817,16 @@ BEGIN NEW.class, NEW.type, NEW.admin_level, (NEW.extratags->'capital') = 'yes', NEW.address->'postcode'); + + -- Short-cut out for linked places. Note that this must happen after the + -- address rank has been recomputed. The linking might nullify a shift in + -- address rank. + IF NEW.linked_place_id is not null THEN + NEW.token_info := null; + {% if debug %}RAISE WARNING 'place already linked to %', OLD.linked_place_id;{% endif %} + RETURN NEW; + END IF; + -- We must always increase the address level relative to the admin boundary. IF NEW.class = 'boundary' and NEW.type = 'administrative' and NEW.osm_type = 'R' and NEW.rank_address > 0 @@ -1001,7 +1005,14 @@ BEGIN -- Full indexing {% if debug %}RAISE WARNING 'Using full index mode for % %', NEW.osm_type, NEW.osm_id;{% endif %} IF linked_place is not null THEN - SELECT * INTO location FROM placex WHERE place_id = linked_place; + -- Recompute the ranks here as the ones from the linked place might + -- have been shifted to accomodate surrounding boundaries. + SELECT place_id, osm_id, class, type, extratags, + centroid, geometry, + (compute_place_rank(country_code, osm_type, class, type, admin_level, + (extratags->'capital') = 'yes', null)).* + INTO location + FROM placex WHERE place_id = linked_place; {% if debug %}RAISE WARNING 'Linked %', location;{% endif %} @@ -1012,11 +1023,11 @@ BEGIN NEW.centroid := geom; END IF; - {% if debug %}RAISE WARNING 'parent address: % rank address: %', parent_address_level, location.rank_address;{% endif %} - IF location.rank_address > parent_address_level - and location.rank_address < 26 + {% if debug %}RAISE WARNING 'parent address: % rank address: %', parent_address_level, location.address_rank;{% endif %} + IF location.address_rank > parent_address_level + and location.address_rank < 26 THEN - NEW.rank_address := location.rank_address; + NEW.rank_address := location.address_rank; END IF; -- merge in extra tags @@ -1025,7 +1036,9 @@ BEGIN || coalesce(NEW.extratags, ''::hstore); -- mark the linked place (excludes from search results) - UPDATE placex set linked_place_id = NEW.place_id + -- Force reindexing to remove any traces from the search indexes and + -- reset the address rank if necessary. + UPDATE placex set linked_place_id = NEW.place_id, indexed_status = 2 WHERE place_id = location.place_id; -- ensure that those places are not found anymore {% if 'search_name' in db.tables %} diff --git a/nominatim/config.py b/nominatim/config.py index 700af328..b3934b49 100644 --- a/nominatim/config.py +++ b/nominatim/config.py @@ -86,14 +86,14 @@ class Configuration: Values of '1', 'yes' and 'true' are accepted as truthy values, everything else is interpreted as false. """ - return self.__getattr__(name).lower() in ('1', 'yes', 'true') + return getattr(self, name).lower() in ('1', 'yes', 'true') def get_int(self, name): """ Return the given configuration parameter as an int. """ try: - return int(self.__getattr__(name)) + return int(getattr(self, name)) except ValueError as exp: LOG.fatal("Invalid setting NOMINATIM_%s. Needs to be a number.", name) raise UsageError("Configuration error.") from exp @@ -105,7 +105,7 @@ class Configuration: will be stripped before returning them. On empty values None is returned. """ - raw = self.__getattr__(name) + raw = getattr(self, name) return [v.strip() for v in raw.split(',')] if raw else None @@ -116,7 +116,7 @@ class Configuration: into an absolute path with the project directory as root path. If the configuration is unset, a falsy value is returned. """ - value = self.__getattr__(name) + value = getattr(self, name) if value: value = Path(value) @@ -152,7 +152,7 @@ class Configuration: name of the standard styles automatically into a file in the config style. """ - style = self.__getattr__('IMPORT_STYLE') + style = getattr(self, 'IMPORT_STYLE') if style in ('admin', 'street', 'address', 'full', 'extratags'): return self.config_dir / f'import-{style}.style' @@ -214,7 +214,7 @@ class Configuration: a regular file. """ if config is not None: - cfg_filename = self.__getattr__(config) + cfg_filename = getattr(self, config) if cfg_filename: cfg_filename = Path(cfg_filename) diff --git a/nominatim/indexer/indexer.py b/nominatim/indexer/indexer.py index 98bb5211..555f8704 100644 --- a/nominatim/indexer/indexer.py +++ b/nominatim/indexer/indexer.py @@ -160,15 +160,12 @@ class Indexer: minrank, maxrank, self.num_threads) with self.tokenizer.name_analyzer() as analyzer: - for rank in range(max(1, minrank), maxrank): - self._index(runners.RankRunner(rank, analyzer)) + for rank in range(max(1, minrank), maxrank + 1): + self._index(runners.RankRunner(rank, analyzer), 20 if rank == 30 else 1) if maxrank == 30: self._index(runners.RankRunner(0, analyzer)) self._index(runners.InterpolationRunner(analyzer), 20) - self._index(runners.RankRunner(30, analyzer), 20) - else: - self._index(runners.RankRunner(maxrank, analyzer)) def index_postcodes(self): diff --git a/test/bdd/db/update/linked_places.feature b/test/bdd/db/update/linked_places.feature index 3dedc468..8bc9585f 100644 --- a/test/bdd/db/update/linked_places.feature +++ b/test/bdd/db/update/linked_places.feature @@ -238,3 +238,29 @@ Feature: Updates of linked places Then placex contains | object | extratags | | R1 | 'linked_place' : 'town' | + + + Scenario: Keep linking and ranks when place type changes + Given the grid + | 1 | | | 2 | + | | | 9 | | + | 4 | | | 3 | + And the places + | osm | class | type | name | admin | geometry | + | R1 | boundary | administrative | foo | 8 | (1,2,3,4,1) | + And the places + | osm | class | type | name | geometry | + | N1 | place | city | foo | 9 | + When importing + Then placex contains + | object | linked_place_id | rank_address | + | N1 | R1 | 16 | + | R1 | - | 16 | + + When updating places + | osm | class | type | name | geometry | + | N1 | place | town | foo | 9 | + Then placex contains + | object | linked_place_id | rank_address | + | N1 | R1 | 16 | + | R1 | - | 16 | diff --git a/test/python/indexer/test_indexing.py b/test/python/indexer/test_indexing.py index e303f381..45c68a33 100644 --- a/test/python/indexer/test_indexing.py +++ b/test/python/indexer/test_indexing.py @@ -177,25 +177,16 @@ def test_index_all_by_rank(test_db, threads, test_tokenizer): SELECT count(*) FROM placex p WHERE rank_address > 0 AND indexed_date >= (SELECT min(indexed_date) FROM placex o WHERE p.rank_address < o.rank_address)""") == 0 - # placex rank < 30 objects come before interpolations + # placex address ranked objects come before interpolations assert test_db.scalar( - """SELECT count(*) FROM placex WHERE rank_address < 30 + """SELECT count(*) FROM placex WHERE rank_address > 0 AND indexed_date > (SELECT min(indexed_date) FROM location_property_osmline)""") == 0 - # placex rank = 30 objects come after interpolations + # rank 0 comes after all other placex objects assert test_db.scalar( - """SELECT count(*) FROM placex WHERE rank_address = 30 - AND indexed_date < - (SELECT max(indexed_date) FROM location_property_osmline)""") == 0 - # rank 0 comes after rank 29 and before rank 30 - assert test_db.scalar( - """SELECT count(*) FROM placex WHERE rank_address < 30 + """SELECT count(*) FROM placex WHERE rank_address > 0 AND indexed_date > (SELECT min(indexed_date) FROM placex WHERE rank_address = 0)""") == 0 - assert test_db.scalar( - """SELECT count(*) FROM placex WHERE rank_address = 30 - AND indexed_date < - (SELECT max(indexed_date) FROM placex WHERE rank_address = 0)""") == 0 @pytest.mark.parametrize("threads", [1, 15])