From: Sarah Hoffmann Date: Sun, 26 Mar 2017 19:19:52 +0000 (+0200) Subject: fix DB bd tests for address hstore X-Git-Tag: v3.0.0~42^2~5 X-Git-Url: https://git.openstreetmap.org./nominatim.git/commitdiff_plain/8c7fa0213fcd70e7df953f54d6b1efbe26cca8cf fix DB bd tests for address hstore --- diff --git a/sql/functions.sql b/sql/functions.sql index 590e1e45..3f64d289 100644 --- a/sql/functions.sql +++ b/sql/functions.sql @@ -654,7 +654,7 @@ BEGIN NEW.indexed_date := now(); IF NEW.indexed_status IS NULL THEN - IF NOT NEW.address ? 'interpolation' + 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 RETURN NULL; @@ -716,7 +716,7 @@ BEGIN -- By doing in postgres we have the country available to us - currently only used for postcode IF NEW.class in ('place','boundary') AND NEW.type in ('postcode','postal_code') THEN - IF NOT NEW.address ? 'postcode' THEN + IF NEW.address IS NULL OR NOT NEW.address ? 'postcode' THEN -- most likely just a part of a multipolygon postcode boundary, throw it away RETURN NULL; END IF; @@ -987,6 +987,9 @@ DECLARE street TEXT; addr_place TEXT; postcode TEXT; + seg_street TEXT; + seg_place TEXT; + seg_postcode TEXT; BEGIN -- deferred delete IF OLD.indexed_status = 100 THEN @@ -1000,111 +1003,113 @@ BEGIN NEW.interpolationtype = NEW.address->'interpolation'; - IF NEW.address ? 'street' THEN - NEW.street = NEW.address->'street'; - END IF; + IF NEW.address is not NULL THEN + IF NEW.address ? 'street' THEN + NEW.street = NEW.address->'street'; + END IF; - IF NEW.address ? 'place' THEN - NEW.addr_place = NEW.address->'place'; - END IF; + IF NEW.address ? 'place' THEN + NEW.addr_place = NEW.address->'place'; + END IF; - IF NEW.address ? 'postcode' THEN - NEW.addr_place = NEW.address->'postcode'; + IF NEW.address ? 'postcode' THEN + NEW.addr_place = NEW.address->'postcode'; + END IF; END IF; - -- do the reparenting: (finally here, because ALL places in placex, - -- that are needed for reparenting, need to be up to date) - -- (the osm interpolationline in location_property_osmline was marked for - -- reparenting in placex_insert/placex_delete with index_status = 1 or 2 (1 inset, 2 delete) - -- => index.c: sets index_status back to 0 - -- => triggers this function) - place_centroid := ST_PointOnSurface(NEW.linegeo); - -- marking descendants for reparenting is not needed, because there are - -- actually no descendants for interpolation lines - NEW.parent_place_id = get_interpolation_parent(NEW.osm_id, NEW.street, NEW.addr_place, - NEW.partition, place_centroid, NEW.linegeo); + -- 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 we are just updating then our work is done - IF OLD.indexed_status != 1 THEN - return NEW; - END IF; + IF array_upper(waynodes, 1) IS NULL THEN + RETURN NEW; + END IF; - -- otherwise split the line as necessary - select nodes from planet_osm_ways where id = NEW.osm_id INTO waynodes; + linegeo := NEW.linegeo; + startnumber := NULL; + street := NEW.street; + addr_place := NEW.addr_place; + postcode := NEW.postcode; + + 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 array_upper(waynodes, 1) IS NULL THEN - RETURN NEW; - END IF; + IF startnumber IS NOT NULL AND endnumber IS NOT NULL + AND startnumber != endnumber + AND ST_GeometryType(sectiongeo) = 'ST_LineString' THEN - linegeo := NEW.linegeo; - startnumber := NULL; - street := NEW.street; - addr_place := NEW.addr_place; - postcode := NEW.postcode; - - FOR nodeidpos in 1..array_upper(waynodes, 1) LOOP - - select * from place where osm_type = 'N' and osm_id = waynodes[nodeidpos]::BIGINT - and housenumber is not NULL 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.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; - IF (startnumber > endnumber) THEN - housenum := endnumber; - endnumber := startnumber; - startnumber := housenum; - sectiongeo := ST_Reverse(sectiongeo); - END IF; + seg_street := coalesce(street, + prevnode.address->'street', + nextnode.address->'street'); + seg_place := coalesce(addr_place, + prevnode.address->'place', + nextnode.address->'place'); + seg_postcode := coalesce(postcode, + prevnode.address->'postcode', + nextnode.address->'postcode'); + + IF NEW.startnumber IS NULL THEN + NEW.startnumber := startnumber; + NEW.endnumber := endnumber; + NEW.linegeo := sectiongeo; + NEW.street := seg_street; + NEW.addr_place := seg_place; + NEW.postcode := seg_postcode; + ELSE + insert into location_property_osmline + (linegeo, partition, osm_id, parent_place_id, + startnumber, endnumber, interpolationtype, + address, street, addr_place, postcode, country_code, + geometry_sector, indexed_status) + values (sectiongeo, NEW.partition, NEW.osm_id, NEW.parent_place_id, + startnumber, endnumber, NEW.interpolationtype, + NEW.address, seg_street, seg_place, seg_postcode, + NEW.country_code, NEW.geometry_sector, 0); + END IF; + END IF; - IF NEW.startnumber IS NULL THEN - NEW.startnumber := startnumber; - NEW.endnumber := endnumber; - NEW.linegeo := sectiongeo; - NEW.street := coalesce(street, prevnode.street, nextnode.street); - NEW.addr_place := coalesce(addr_place, prevnode.addr_place, nextnode.addr_place); - NEW.postcode := coalesce(postcode, prevnode.postcode, nextnode.postcode); - ELSE - insert into location_property_osmline - (linegeo, partition, osm_id, parent_place_id, - startnumber, endnumber, interpolationtype, - address, street, addr_place, postcode, country_code, - geometry_sector, indexed_status) - values (sectiongeo, NEW.partition, NEW.osm_id, NEW.parent_place_id, - startnumber, endnumber, NEW.interpolationtype, - address, coalesce(street, prevnode.street, nextnode.street), - coalesce(addr_place, prevnode.addr_place, nextnode.addr_place), - coalesce(postcode, prevnode.postcode, nextnode.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; - -- 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; + END IF; + END LOOP; + END IF; - startnumber := substring(nextnode.housenumber,'[0-9]+')::integer; - prevnode := nextnode; - END IF; - END LOOP; + place_centroid := ST_PointOnSurface(NEW.linegeo); + NEW.parent_place_id = get_interpolation_parent(NEW.osm_id, NEW.street, NEW.addr_place, + NEW.partition, place_centroid, NEW.linegeo); + -- marking descendants for reparenting is not needed, because there are + -- actually no descendants for interpolation lines RETURN NEW; END; $$ @@ -1191,32 +1196,34 @@ BEGIN RETURN NEW; END IF; - IF NEW.address ? 'conscriptionnumber' THEN - i := getorcreate_housenumber_id(make_standard_name(NEW.address->'conscriptionnumber')); - IF NEW.address ? 'streetnumber' THEN + IF NEW.address is not NULL THEN + IF NEW.address ? 'conscriptionnumber' THEN + i := getorcreate_housenumber_id(make_standard_name(NEW.address->'conscriptionnumber')); + IF NEW.address ? 'streetnumber' THEN + i := getorcreate_housenumber_id(make_standard_name(NEW.address->'streetnumber')); + NEW.housenumber := NEW.address->'conscriptionnumber' || '/' || NEW.address->'streetnumber'; + ELSE + NEW.housenumber := NEW.address->'conscriptionnumber'; + END IF; + ELSEIF NEW.address ? 'streetnumber' THEN + NEW.housenumber := NEW.address->'streetnumber'; i := getorcreate_housenumber_id(make_standard_name(NEW.address->'streetnumber')); - NEW.housenumber := NEW.address->'conscriptionnumber' || '/' || NEW.address->'streetnumber'; - ELSE - NEW.housenumber := NEW.address->'conscriptionnumber' - ENDIF - ELSEIF NEW.address ? 'streetnumber' THEN - NEW.housenumber := NEW.address->'streetnumber'; - i := getorcreate_housenumber_id(make_standard_name(NEW.address->'streetnumber')); - ELSEIF NEW.address ? 'housenumber' THEN - NEW.housenumber := NEW.address->'housenumber'; - i := getorcreate_housenumber_id(make_standard_name(NEW.housenumber)); - END IF; + ELSEIF NEW.address ? 'housenumber' THEN + NEW.housenumber := NEW.address->'housenumber'; + i := getorcreate_housenumber_id(make_standard_name(NEW.housenumber)); + END IF; - IF NEW.address ? 'street' THEN - NEW.street = NEW.address->'street'; - END IF; + IF NEW.address ? 'street' THEN + NEW.street = NEW.address->'street'; + END IF; - IF NEW.address ? 'place' THEN - NEW.addr_place = NEW.address->'place'; - END IF; + IF NEW.address ? 'place' THEN + NEW.addr_place = NEW.address->'place'; + END IF; - IF NEW.address ? 'postcode' THEN - NEW.addr_place = NEW.address->'postcode'; + IF NEW.address ? 'postcode' THEN + NEW.addr_place = NEW.address->'postcode'; + END IF; END IF; -- Speed up searches - just use the centroid of the feature @@ -1225,7 +1232,7 @@ BEGIN NEW.centroid := null; -- recalculate country and partition - IF NEW.rank_search = 4 AND NEW.address ? 'country' THEN + IF NEW.rank_search = 4 AND NEW.address is not NULL AND NEW.address ? 'country' THEN -- for countries, believe the mapped country code, -- so that we remain in the right partition if the boundaries -- suddenly expand. @@ -2024,7 +2031,8 @@ BEGIN ELSE -- insert to placex -- Patch in additional country names - IF NEW.admin_level = 2 AND NEW.type = 'administrative' AND NEW.address ? 'country' THEN + IF NEW.admin_level = 2 AND NEW.type = 'administrative' + AND NEW.address is not NULL AND NEW.address ? 'country' THEN SELECT name FROM country_name WHERE country_code = lower(NEW.address->'country') INTO existing; IF existing.name IS NOT NULL THEN NEW.name = existing.name || NEW.name; diff --git a/test/bdd/db/import/interpolation.feature b/test/bdd/db/import/interpolation.feature index 7dfc6b23..d7ac9a4f 100644 --- a/test/bdd/db/import/interpolation.feature +++ b/test/bdd/db/import/interpolation.feature @@ -7,6 +7,8 @@ Feature: Import of address interpolations | osm | class | type | housenr | geometry | | N1 | place | house | 2 | 1 1 | | N2 | place | house | 6 | 1 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001 | And the ways | id | nodes | @@ -21,6 +23,8 @@ Feature: Import of address interpolations | osm | class | type | housenr | geometry | | N1 | place | house | 2 | 1 1 | | N2 | place | house | 6 | 1 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1.001, 1 1 | And the ways | id | nodes | @@ -35,6 +39,8 @@ Feature: Import of address interpolations | osm | class | type | housenr | geometry | | N1 | place | house | 1 | 1 1 | | N2 | place | house | 11 | 1 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | odd | 1 1, 1 1.001 | And the ways | id | nodes | @@ -49,6 +55,8 @@ Feature: Import of address interpolations | osm | class | type | housenr | geometry | | N1 | place | house | 1 | 1 1 | | N2 | place | house | 3 | 1 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | all | 1 1, 1 1.001 | And the ways | id | nodes | @@ -63,6 +71,8 @@ Feature: Import of address interpolations | osm | class | type | housenr | geometry | | N1 | place | house | 2 | 1 1 | | N2 | place | house | 10 | 1.001 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001, 1.001 1.001 | And the ways | id | nodes | @@ -77,6 +87,8 @@ Feature: Import of address interpolations | osm | class | type | housenr | geometry | | N1 | place | house | 2 | 1 1 | | N2 | place | house | 10 | 1.001 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001, 1.001 1.001 | And the ways | id | nodes | @@ -92,6 +104,8 @@ Feature: Import of address interpolations | N1 | place | house | 2 | 1 1 | | N2 | place | house | 14 | 1.001 1.001 | | N3 | place | house | 10 | 1 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001, 1.001 1.001 | And the ways | id | nodes | @@ -109,6 +123,8 @@ Feature: Import of address interpolations | N2 | place | house | 14 | 1.001 1.001 | | N3 | place | house | 10 | 1 1.001 | | N4 | place | house | 18 | 1.001 1.002 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001, 1.001 1.001, 1.001 1.002 | And the ways | id | nodes | @@ -126,6 +142,8 @@ Feature: Import of address interpolations | N1 | place | house | 2 | 1 1 | | N2 | place | house | 14 | 1.001 1.001 | | N3 | place | house | 10 | 1 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1.001 1.001, 1 1.001, 1 1 | And the ways | id | nodes | @@ -142,6 +160,8 @@ Feature: Import of address interpolations | N1 | place | house | 2 | 1 1 | | N2 | place | house | 8 | 1.001 1.001 | | N3 | place | house | 7 | 1 1.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 1 1, 1 1.001, 1.001 1.001 | And the ways | id | nodes | @@ -158,6 +178,8 @@ Feature: Import of address interpolations | N1 | place | house | 2 | 0 0 | | N2 | place | house | 6 | 0 0.001 | | N3 | place | house | 10 | 0 0.002 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 0 0, 0 0.001, 0 0.002, 0 0.001 | And the ways | id | nodes | @@ -174,6 +196,8 @@ Feature: Import of address interpolations | osm | class | type | housenr | geometry | | N1 | place | house | 2 | 0 0 | | N2 | place | house | 6 | 0 0.001 | + And the places + | osm | class | type | addr+interpolation | geometry | | W1 | place | houses | even | 0 0, 0 0.001, 0 0.002, 0 0.001 | And the ways | id | nodes | @@ -192,7 +216,7 @@ Feature: Import of address interpolations | N3 | place | house | 12 | :n-middle-w | | N4 | place | house | 16 | :n-middle-e | And the places - | osm | class | type | housenr | street | geometry | + | osm | class | type | addr+interpolation | street | geometry | | W10 | place | houses | even | | :w-middle | | W11 | place | houses | even | Cloud Street | :w-middle | And the places @@ -238,9 +262,9 @@ Feature: Import of address interpolations | N3 | place | house | 12 | Cloud Street | :n-middle-w | | N4 | place | house | 16 | Cloud Street | :n-middle-e | And the places - | osm | class | type | housenr | geometry | - | W10 | place | houses | even | :w-middle | - | W11 | place | houses | even | :w-middle | + | osm | class | type | addr+interpolation | geometry | + | W10 | place | houses | even | :w-middle | + | W11 | place | houses | even | :w-middle | And the places | osm | class | type | name | geometry | | W2 | highway | tertiary | Sun Way | :w-north | @@ -277,7 +301,9 @@ Feature: Import of address interpolations | N1 | place | house | 10 | 144.9632341 -37.76163 | | N2 | place | house | 6 | 144.9630541 -37.7628174 | | N3 | shop | supermarket | 2 | 144.9629794 -37.7630755 | - | W1 | place | houses | even | 144.9632341 -37.76163,144.9630541 -37.7628172,144.9629794 -37.7630755 | + And the places + | osm | class | type | addr+interpolation | geometry | + | W1 | place | houses | even | 144.9632341 -37.76163,144.9630541 -37.7628172,144.9629794 -37.7630755 | And the ways | id | nodes | | 1 | 1,2,3 | @@ -288,19 +314,23 @@ Feature: Import of address interpolations | 6 | 10 | 144.9630541 -37.7628174, 144.9632341 -37.76163 | Scenario: Place with missing address information - Given the places - | osm | class | type | housenr | geometry | - | N1 | place | house | 23 | 0.0001 0.0001 | - | N2 | amenity | school | | 0.0001 0.0002 | - | N3 | place | house | 29 | 0.0001 0.0004 | - | W1 | place | houses | odd | 0.0001 0.0001,0.0001 0.0002,0.0001 0.0004 | + Given the grid + | 1 | | 2 | | | 3 | + And the places + | osm | class | type | housenr | + | N1 | place | house | 23 | + | N2 | amenity | school | | + | N3 | place | house | 29 | + And the places + | osm | class | type | addr+interpolation | geometry | + | W1 | place | houses | odd | 1,2,3 | And the ways | id | nodes | | 1 | 1,2,3 | When importing Then W1 expands to interpolation | start | end | geometry | - | 23 | 29 | 0.0001 0.0001, 0.0001 0.0002, 0.0001 0.0004 | + | 23 | 29 | 1,2,3 | Scenario: Ways without node entries are ignored Given the places diff --git a/test/bdd/db/import/naming.feature b/test/bdd/db/import/naming.feature index d2339376..f3019e2a 100644 --- a/test/bdd/db/import/naming.feature +++ b/test/bdd/db/import/naming.feature @@ -8,8 +8,8 @@ Feature: Import and search of names | N1 | place | locality | german | country:de | When importing Then placex contains - | object | calculated_country_code | name+name | - | N1 | de | german | + | object | country_code | name+name | + | N1 | de | german | Scenario: Copying name tag to default language if it does not exist Given the places @@ -17,8 +17,8 @@ Feature: Import and search of names | N1 | place | locality | german | finnish | country:de | When importing Then placex contains - | object | calculated_country_code | name | name+name:fi | name+name:de | - | N1 | de | german | finnish | german | + | object | country_code | name | name+name:fi | name+name:de | + | N1 | de | german | finnish | german | Scenario: Copying default language name tag to name if it does not exist Given the places @@ -26,8 +26,8 @@ Feature: Import and search of names | N1 | place | locality | german | finnish | country:de | When importing Then placex contains - | object | calculated_country_code | name | name+name:fi | name+name:de | - | N1 | de | german | finnish | german | + | object | country_code | name | name+name:fi | name+name:de | + | N1 | de | german | finnish | german | Scenario: Do not overwrite default language with name tag Given the places @@ -35,5 +35,5 @@ Feature: Import and search of names | N1 | place | locality | german | finnish | local | country:de | When importing Then placex contains - | object | calculated_country_code | name | name+name:fi | name+name:de | - | N1 | de | german | finnish | local | + | object | country_code | name | name+name:fi | name+name:de | + | N1 | de | german | finnish | local | diff --git a/test/bdd/db/import/placex.feature b/test/bdd/db/import/placex.feature index 7cbedaa3..f124e482 100644 --- a/test/bdd/db/import/placex.feature +++ b/test/bdd/db/import/placex.feature @@ -8,8 +8,8 @@ Feature: Import into placex | N1 | highway | primary | country:us | When importing Then placex contains - | object | country_code | calculated_country_code | - | N1 | None | us | + | object | addr+country | country_code | + | N1 | - | us | Scenario: Location overwrites country code tag Given the named places @@ -17,8 +17,8 @@ Feature: Import into placex | N1 | highway | primary | de | country:us | When importing Then placex contains - | object | country_code | calculated_country_code | - | N1 | de | us | + | object | addr+country | country_code | + | N1 | de | us | Scenario: Country code tag overwrites location for countries Given the named places @@ -26,8 +26,8 @@ Feature: Import into placex | R1 | boundary | administrative | 2 | de | (-100 40, -101 40, -101 41, -100 41, -100 40) | When importing Then placex contains - | object | country_code | calculated_country_code | - | R1 | de | de | + | object | addr+country | country_code | + | R1 | de | de | Scenario: Illegal country code tag for countries is ignored Given the named places @@ -35,8 +35,8 @@ Feature: Import into placex | R1 | boundary | administrative | 2 | xx | (-100 40, -101 40, -101 41, -100 41, -100 40) | When importing Then placex contains - | object | country_code | calculated_country_code | - | R1 | xx | us | + | object | addr+country | country_code | + | R1 | xx | us | Scenario: admin level is copied over Given the named places @@ -47,24 +47,6 @@ Feature: Import into placex | object | admin_level | | N1 | 3 | - Scenario: admin level is default 15 - Given the named places - | osm | class | type | - | N1 | amenity | prison | - When importing - Then placex contains - | object | admin_level | - | N1 | 15 | - - Scenario: admin level is never larger than 15 - Given the named places - | osm | class | type | admin | - | N1 | amenity | prison | 16 | - When importing - Then placex contains - | object | admin_level | - | N1 | 15 | - Scenario: postcode node without postcode is dropped Given the places | osm | class | type | name+ref | @@ -87,10 +69,10 @@ Feature: Import into placex | N3 | place | postcode | Y45 | country:gb | When importing Then placex contains - | object | postcode | calculated_country_code | rank_search | rank_address | - | N1 | E45 2CD | gb | 25 | 5 | - | N2 | E45 2 | gb | 23 | 5 | - | N3 | Y45 | gb | 21 | 5 | + | object | postcode | country_code | rank_search | rank_address | + | N1 | E45 2CD | gb | 25 | 5 | + | N2 | E45 2 | gb | 23 | 5 | + | N3 | Y45 | gb | 21 | 5 | Scenario: wrongly formatted GB postcodes are down-ranked Given the places @@ -100,10 +82,10 @@ Feature: Import into placex | N3 | place | postcode | y45 | country:gb | When importing Then placex contains - | object | calculated_country_code | rank_search | rank_address | - | N1 | gb | 30 | 30 | - | N2 | gb | 30 | 30 | - | N3 | gb | 30 | 30 | + | object | country_code | rank_search | rank_address | + | N1 | gb | 30 | 30 | + | N2 | gb | 30 | 30 | + | N3 | gb | 30 | 30 | Scenario: search and address rank for DE postcodes correctly assigned Given the places @@ -114,11 +96,11 @@ Feature: Import into placex | N4 | place | postcode | 564276 | country:de | When importing Then placex contains - | object | calculated_country_code | rank_search | rank_address | - | N1 | de | 21 | 11 | - | N2 | de | 30 | 30 | - | N3 | de | 30 | 30 | - | N4 | de | 30 | 30 | + | object | country_code | rank_search | rank_address | + | N1 | de | 21 | 11 | + | N2 | de | 30 | 30 | + | N3 | de | 30 | 30 | + | N4 | de | 30 | 30 | Scenario: search and address rank for other postcodes are correctly assigned Given the places @@ -134,16 +116,16 @@ Feature: Import into placex | N9 | place | postcode | A1:bc10 | country:ca | When importing Then placex contains - | object | calculated_country_code | rank_search | rank_address | - | N1 | ca | 21 | 11 | - | N2 | ca | 21 | 11 | - | N3 | ca | 21 | 11 | - | N4 | ca | 21 | 11 | - | N5 | ca | 21 | 11 | - | N6 | ca | 21 | 11 | - | N7 | ca | 25 | 11 | - | N8 | ca | 25 | 11 | - | N9 | ca | 25 | 11 | + | object | country_code | rank_search | rank_address | + | N1 | ca | 21 | 11 | + | N2 | ca | 21 | 11 | + | N3 | ca | 21 | 11 | + | N4 | ca | 21 | 11 | + | N5 | ca | 21 | 11 | + | N6 | ca | 21 | 11 | + | N7 | ca | 25 | 11 | + | N8 | ca | 25 | 11 | + | N9 | ca | 25 | 11 | Scenario: search and address ranks for places are correctly assigned Given the named places diff --git a/test/bdd/steps/db_ops.py b/test/bdd/steps/db_ops.py index c4384f07..07b377fa 100644 --- a/test/bdd/steps/db_ops.py +++ b/test/bdd/steps/db_ops.py @@ -8,7 +8,7 @@ import psycopg2.extras class PlaceColumn: def __init__(self, context, force_name): - self.columns = { 'admin_level' : 100} + self.columns = { 'admin_level' : 15} self.force_name = force_name self.context = context self.geometry = None @@ -21,7 +21,7 @@ class PlaceColumn: elif key.startswith('extra+'): self.add_hstore('extratags', key[6:], value) elif key.startswith('addr+'): - self.add_hstore('address', key[6:], value) + self.add_hstore('address', key[5:], value) else: assert_in(key, ('class', 'type')) self.columns[key] = None if value == '' else value @@ -42,6 +42,9 @@ class PlaceColumn: def set_key_housenr(self, value): self.add_hstore('address', 'housenumber', None if value == '' else value) + def set_key_postcode(self, value): + self.add_hstore('address', 'postcode', None if value == '' else value) + def set_key_street(self, value): self.add_hstore('address', 'street', None if value == '' else value) @@ -81,6 +84,46 @@ class PlaceColumn: self.geometry) cursor.execute(query, list(self.columns.values())) +class LazyFmt(object): + + def __init__(self, fmtstr, *args): + self.fmt = fmtstr + self.args = args + + def __str__(self): + return self.fmt % self.args + +class PlaceObjName(object): + + def __init__(self, placeid, conn): + self.pid = placeid + self.conn = conn + + def __str__(self): + if self.pid is None: + return "" + + self.conn.cursor().execute("""SELECT osm_type, osm_id, class + FROM placex WHERE place_id = %s""", + self.pid) + eq_(1, cur.rowcount, "No entry found for place id %s" % self.pid) + + return "%s%s:%s" % cur.fetchone() + +def compare_place_id(expected, result, column, context): + if expected == '0': + eq_(0, result, + LazyFmt("Bad place id in column %s. Expected: 0, got: %s.", + column, PlaceObjName(result, context.db))) + elif expected == '-': + assert_is_none(result, + LazyFmt("bad place id in column %s: %s.", + column, PlaceObjName(result, context.db))) + else: + eq_(NominatimID(expected).get_place_id(context.db.cursor()), result, + LazyFmt("Bad place id in column %s. Expected: %s, got: %s.", + column, expected, PlaceObjName(result, context.db))) + class NominatimID: """ Splits a unique identifier for places into its components. As place_ids cannot be used for testing, we use a unique @@ -305,14 +348,16 @@ def check_placex_contents(context, exact): eq_(res['name'][name], row[h]) elif h.startswith('extratags+'): eq_(res['extratags'][h[10:]], row[h]) - elif h in ('linked_place_id', 'parent_place_id'): - if row[h] == '0': - eq_(0, res[h]) - elif row[h] == '-': - assert_is_none(res[h]) + elif h.startswith('addr+'): + if row[h] == '-': + if res['address'] is not None: + assert_not_in(h[5:], res['address']) else: - eq_(NominatimID(row[h]).get_place_id(context.db.cursor()), - res[h]) + assert_in(h[5:], res['address'], "column " + h) + assert_equals(res['address'][h[5:]], row[h], + "column %s" % h) + elif h in ('linked_place_id', 'parent_place_id'): + compare_place_id(row[h], res[h], h, context) else: assert_db_column(res, h, row[h], context) @@ -358,13 +403,7 @@ def check_placex_contents(context, exact): else: assert_equals(res['address'][h[5:]], row[h], msg) elif h in ('linked_place_id', 'parent_place_id'): - if row[h] == '0': - assert_equals(0, res[h], msg) - elif row[h] == '-': - assert_is_none(res[h], msg) - else: - assert_equals(NominatimID(row[h]).get_place_id(context.db.cursor()), - res[h], msg) + compare_place_id(row[h], res[h], h, context) else: assert_db_column(res, h, row[h], context) @@ -455,13 +494,7 @@ def check_location_property_osmline(context, oid, neg): if h in ('start', 'end'): continue elif h == 'parent_place_id': - if row[h] == '0': - eq_(0, res[h]) - elif row[h] == '-': - assert_is_none(res[h]) - else: - eq_(NominatimID(row[h]).get_place_id(context.db.cursor()), - res[h]) + compare_place_id(row[h], res[h], h, context) else: assert_db_column(res, h, row[h], context)