prepare release 4.2.4

Before postgis 3.4 ST_Project required a geography as input and seemed
to have implicitly converted to geography. Since 3.4 geometry input
is supported but leads to a completely different result.

.github/workflows/ci-tests.yml

@@ -37,13 +37,8 @@ jobs:
         needs: create-archive
         strategy:
             matrix:
-                ubuntu: [18, 20, 22]
+                ubuntu: [20, 22]
                 include:
-                    - ubuntu: 18
-                      postgresql: 9.6
-                      postgis: 2.5
-                      pytest: pytest
-                      php: 7.2
                     - ubuntu: 20
                       postgresql: 13
                       postgis: 3

CMakeLists.txt

@@ -20,7 +20,7 @@ project(nominatim)
 
 set(NOMINATIM_VERSION_MAJOR 4)
 set(NOMINATIM_VERSION_MINOR 2)
-set(NOMINATIM_VERSION_PATCH 2)
+set(NOMINATIM_VERSION_PATCH 4)
 
 set(NOMINATIM_VERSION "${NOMINATIM_VERSION_MAJOR}.${NOMINATIM_VERSION_MINOR}.${NOMINATIM_VERSION_PATCH}")
 

ChangeLog

@@ -1,3 +1,14 @@
+4.2.4
+ * fix a potential SQL injection in 'nominatim admin --collect-os-info'
+ * fix compatibility issue with PostGIS 3.4
+
+4.2.3
+
+ * fix deletion handling for 'nominatim add-data'
+ * adapt place_force_delete() to new deletion handling
+ * flex style: avoid dropping of postcode areas
+ * fix update errors on address interpolation handling
+
 4.2.2
 
  * extend flex-style library to fully support all default styles

lib-sql/functions/interpolation.sql

@@ -164,7 +164,7 @@ DECLARE
   newend INTEGER;
   moddiff SMALLINT;
   linegeo GEOMETRY;
-  splitline GEOMETRY;
+  splitpoint FLOAT;
   sectiongeo GEOMETRY;
   postcode TEXT;
   stepmod SMALLINT;
@@ -223,15 +223,27 @@ BEGIN
         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'
+              and ST_Distance(NEW.linegeo, geometry) < 0.0005
         ORDER BY nodeidpos
     LOOP
       {% if debug %}RAISE WARNING 'processing point % (%)', nextnode.hnr, ST_AsText(nextnode.geometry);{% endif %}
       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);
+        splitpoint := ST_LineLocatePoint(linegeo, nextnode.geometry);
+        IF splitpoint = 0 THEN
+          -- Corner case where the splitpoint falls on the first point
+          -- and thus would not return a geometry. Skip that section.
+          sectiongeo := NULL;
+        ELSEIF splitpoint = 1 THEN
+          -- Point is at the end of the line.
+          sectiongeo := linegeo;
+          linegeo := NULL;
+        ELSE
+          -- Split the line.
+          sectiongeo := ST_LineSubstring(linegeo, 0, splitpoint);
+          linegeo := ST_LineSubstring(linegeo, splitpoint, 1);
+        END IF;
       END IF;
 
       IF prevnode.hnr is not null
@@ -239,6 +251,9 @@ BEGIN
          -- regularly mapped housenumbers.
          -- (Conveniently also fails if one of the house numbers is not a number.)
          and abs(prevnode.hnr - nextnode.hnr) > NEW.step
+         -- If the interpolation geometry is broken or two nodes are at the
+         -- same place, then splitting might produce a point. Ignore that.
+         and ST_GeometryType(sectiongeo) = 'ST_LineString'
       THEN
         IF prevnode.hnr < nextnode.hnr THEN
           startnumber := prevnode.hnr;
@@ -300,12 +315,12 @@ BEGIN
                   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;
+      -- early break if we are out of line string,
+      -- might happen when a line string loops back on itself
+      IF linegeo is null or ST_GeometryType(linegeo) != 'ST_LineString' THEN
+          RETURN NEW;
       END IF;
 
       prevnode := nextnode;

lib-sql/functions/utils.sql

@@ -273,8 +273,8 @@ BEGIN
   END IF;
 
   RETURN ST_Envelope(ST_Collect(
-                     ST_Project(geom, radius, 0.785398)::geometry,
-                     ST_Project(geom, radius, 3.9269908)::geometry));
+                     ST_Project(geom::geography, radius, 0.785398)::geometry,
+                     ST_Project(geom::geography, radius, 3.9269908)::geometry));
 END;
 $$
 LANGUAGE plpgsql IMMUTABLE;
@@ -429,9 +429,10 @@ BEGIN
   SELECT osm_type, osm_id, class, type FROM placex WHERE place_id = placeid INTO osmtype, osmid, pclass, ptype;
   DELETE FROM import_polygon_delete where osm_type = osmtype and osm_id = osmid and class = pclass and type = ptype;
   DELETE FROM import_polygon_error where osm_type = osmtype and osm_id = osmid and class = pclass and type = ptype;
-  -- force delete from place/placex by making it a very small geometry
-  UPDATE place set geometry = ST_SetSRID(ST_Point(0,0), 4326) where osm_type = osmtype and osm_id = osmid and class = pclass and type = ptype;
-  DELETE FROM place where osm_type = osmtype and osm_id = osmid and class = pclass and type = ptype;
+  -- force delete by directly entering it into the to-be-deleted table
+  INSERT INTO place_to_be_deleted (osm_type, osm_id, class, type, deferred)
+         VALUES(osmtype, osmid, pclass, ptype, false);
+  PERFORM flush_deleted_places();
 
   RETURN TRUE;
 END;

nominatim/clicmd/add_data.py

@@ -76,21 +76,25 @@ class UpdateAddData:
 
         osm2pgsql_params = args.osm2pgsql_options(default_cache=1000, default_threads=1)
         if args.file or args.diff:
-            return add_osm_data.add_data_from_file(cast(str, args.file or args.diff),
+            return add_osm_data.add_data_from_file(args.config.get_libpq_dsn(),
+                                                   cast(str, args.file or args.diff),
                                                    osm2pgsql_params)
 
         if args.node:
-            return add_osm_data.add_osm_object('node', args.node,
+            return add_osm_data.add_osm_object(args.config.get_libpq_dsn(),
+                                               'node', args.node,
                                                args.use_main_api,
                                                osm2pgsql_params)
 
         if args.way:
-            return add_osm_data.add_osm_object('way', args.way,
+            return add_osm_data.add_osm_object(args.config.get_libpq_dsn(),
+                                               'way', args.way,
                                                args.use_main_api,
                                                osm2pgsql_params)
 
         if args.relation:
-            return add_osm_data.add_osm_object('relation', args.relation,
+            return add_osm_data.add_osm_object(args.config.get_libpq_dsn(),
+                                               'relation', args.relation,
                                                args.use_main_api,
                                                osm2pgsql_params)
 

nominatim/db/async_connection.py

@@ -69,8 +69,8 @@ class DBConnection:
         self.current_params: Optional[Sequence[Any]] = None
         self.ignore_sql_errors = ignore_sql_errors
 
-        self.conn: Optional['psycopg2.connection'] = None
-        self.cursor: Optional['psycopg2.cursor'] = None
+        self.conn: Optional['psycopg2._psycopg.connection'] = None
+        self.cursor: Optional['psycopg2._psycopg.cursor'] = None
         self.connect(cursor_factory=cursor_factory)
 
     def close(self) -> None:
@@ -78,7 +78,7 @@ class DBConnection:
         """
         if self.conn is not None:
             if self.cursor is not None:
-                self.cursor.close() # type: ignore[no-untyped-call]
+                self.cursor.close()
                 self.cursor = None
             self.conn.close()
 

nominatim/db/connection.py

@@ -31,7 +31,7 @@ class Cursor(psycopg2.extras.DictCursor):
         """ Query execution that logs the SQL query when debugging is enabled.
         """
         if LOG.isEnabledFor(logging.DEBUG):
-            LOG.debug(self.mogrify(query, args).decode('utf-8')) # type: ignore[no-untyped-call]
+            LOG.debug(self.mogrify(query, args).decode('utf-8'))
 
         super().execute(query, args)
 

nominatim/db/utils.py

@@ -118,4 +118,4 @@ class CopyBuffer:
         """
         if self.buffer.tell() > 0:
             self.buffer.seek(0)
-            cur.copy_from(self.buffer, table, columns=columns) # type: ignore[no-untyped-call]
+            cur.copy_from(self.buffer, table, columns=columns)

nominatim/tools/add_osm_data.py

@@ -12,23 +12,34 @@ from pathlib import Path
 import logging
 import urllib
 
+from nominatim.db.connection import connect
 from nominatim.tools.exec_utils import run_osm2pgsql, get_url
 
 LOG = logging.getLogger()
 
-def add_data_from_file(fname: str, options: MutableMapping[str, Any]) -> int:
+def _run_osm2pgsql(dsn: str, options: MutableMapping[str, Any]) -> None:
+    run_osm2pgsql(options)
+
+    # Handle deletions
+    with connect(dsn) as conn:
+        with conn.cursor() as cur:
+            cur.execute('SELECT flush_deleted_places()')
+        conn.commit()
+
+
+def add_data_from_file(dsn: str, fname: str, options: MutableMapping[str, Any]) -> int:
     """ Adds data from a OSM file to the database. The file may be a normal
         OSM file or a diff file in all formats supported by libosmium.
     """
     options['import_file'] = Path(fname)
     options['append'] = True
-    run_osm2pgsql(options)
+    _run_osm2pgsql(dsn, options)
 
     # No status update. We don't know where the file came from.
     return 0
 
 
-def add_osm_object(osm_type: str, osm_id: int, use_main_api: bool,
+def add_osm_object(dsn: str, osm_type: str, osm_id: int, use_main_api: bool,
                    options: MutableMapping[str, Any]) -> int:
     """ Add or update a single OSM object from the latest version of the
         API.
@@ -51,6 +62,6 @@ def add_osm_object(osm_type: str, osm_id: int, use_main_api: bool,
     options['append'] = True
     options['import_data'] = get_url(base_url).encode('utf-8')
 
-    run_osm2pgsql(options)
+    _run_osm2pgsql(dsn, options)
 
     return 0

nominatim/tools/collect_os_info.py

@@ -12,14 +12,13 @@ import os
 import subprocess
 import sys
 from pathlib import Path
-from typing import List, Optional, Tuple, Union, cast
+from typing import List, Optional, Tuple, Union
 
 import psutil
 from psycopg2.extensions import make_dsn, parse_dsn
 
 from nominatim.config import Configuration
 from nominatim.db.connection import connect
-from nominatim.typing import DictCursorResults
 from nominatim.version import version_str
 
 
@@ -107,15 +106,15 @@ def report_system_information(config: Configuration) -> None:
         postgresql_ver: str = convert_version(conn.server_version_tuple())
 
         with conn.cursor() as cur:
-            cur.execute(f"""
-            SELECT datname FROM pg_catalog.pg_database 
-            WHERE datname='{parse_dsn(config.get_libpq_dsn())['dbname']}'""")
-            nominatim_db_exists = cast(Optional[DictCursorResults], cur.fetchall())
-            if nominatim_db_exists:
-                with connect(config.get_libpq_dsn()) as conn:
-                    postgis_ver: str = convert_version(conn.postgis_version_tuple())
-            else:
-                postgis_ver = "Unable to connect to database"
+            num = cur.scalar("SELECT count(*) FROM pg_catalog.pg_database WHERE datname=%s",
+                             (parse_dsn(config.get_libpq_dsn())['dbname'], ))
+            nominatim_db_exists = num == 1 if isinstance(num, int) else False
+
+    if nominatim_db_exists:
+        with connect(config.get_libpq_dsn()) as conn:
+            postgis_ver: str = convert_version(conn.postgis_version_tuple())
+    else:
+        postgis_ver = "Unable to connect to database"
 
     postgresql_config: str = get_postgresql_config(int(float(postgresql_ver)))
 

nominatim/version.py

@@ -25,7 +25,7 @@ from typing import Optional, Tuple
 # patch level when cherry-picking the commit with the migration.
 #
 # Released versions always have a database patch level of 0.
-NOMINATIM_VERSION = (4, 2, 2, 0)
+NOMINATIM_VERSION = (4, 2, 4, 0)
 
 POSTGRESQL_REQUIRED_VERSION = (9, 6)
 POSTGIS_REQUIRED_VERSION = (2, 2)

settings/import-address.lua

@@ -17,7 +17,7 @@ flex.set_main_tags{
                secondary_link = 'named',
                tertiary_link = 'named'},
     boundary = {administrative = 'named',
-                postal_code = 'named'},
+                postal_code = 'always'},
     landuse = 'fallback',
     place = 'always'
 }

settings/import-extratags.lua

@@ -27,7 +27,7 @@ flex.set_main_tags{
     man_made = 'always',
     aerialway = 'always',
     boundary = {'named',
-                postal_code = 'named'},
+                postal_code = 'always'},
     aeroway = 'always',
     amenity = 'always',
     club = 'always',

settings/import-full.lua

@@ -27,7 +27,7 @@ flex.set_main_tags{
     man_made = 'always',
     aerialway = 'always',
     boundary = {'named',
-                postal_code = 'named'},
+                postal_code = 'always'},
     aeroway = 'always',
     amenity = 'always',
     club = 'always',

settings/import-street.lua

@@ -16,7 +16,8 @@ flex.set_main_tags{
                primary_link = 'named',
                secondary_link = 'named',
                tertiary_link = 'named'},
-    boundary = {administrative = 'named'},
+    boundary = {administrative = 'named',
+                postal_code = 'always'},
     landuse = 'fallback',
     place = 'always'
 }

test/bdd/db/import/interpolation.feature

@@ -29,14 +29,14 @@ Feature: Import of address interpolations
           | N2  | place | house  | 8       |
         And the places
           | osm | class | type   | addr+interpolation | geometry |
-          | W1  | place | houses | even    | 1,2 |
+          | W1  | place | houses | even               | 2,1      |
         And the ways
           | id | nodes |
           | 1  | 2,1 |
         When importing
         Then W1 expands to interpolation
           | start | end | geometry |
-          | 4     | 6   | 8,9 |
+          | 4     | 6   | 9,8      |
 
     Scenario: Simple odd two point interpolation
         Given the grid with origin 1,1
@@ -341,7 +341,7 @@ Feature: Import of address interpolations
         Then W1 expands to interpolation
           | start | end | geometry |
           | 4     | 4   | 144.963016 -37.762946 |
-          | 8     | 8   | 144.963144 -37.7622237 |
+          | 8     | 8   | 144.96314407 -37.762223692 |
 
     Scenario: Place with missing address information
         Given the grid
@@ -456,3 +456,69 @@ Feature: Import of address interpolations
           | foo   |
           | x     |
           | 12-2  |
+
+
+    Scenario: Interpolation line where points have been moved (Github #3022)
+        Given the 0.00001 grid
+         | 1 | | | | | | | | 2 | 3 | 9 | | | | | | | | 4 |
+        Given the places
+          | osm | class | type   | housenr | geometry |
+          | N1  | place | house  | 2       | 1 |
+          | N2  | place | house  | 18      | 3 |
+          | N3  | place | house  | 24      | 9 |
+          | N4  | place | house  | 42      | 4 |
+        And the places
+          | osm | class | type   | addr+interpolation | geometry |
+          | W1  | place | houses | even               | 1,2,3,4  |
+        And the ways
+          | id | nodes   |
+          | 1  | 1,2,3,4 |
+        When importing
+        Then W1 expands to interpolation
+          | start | end |
+          | 4     | 16  |
+          | 20    | 22  |
+          | 26    | 40  |
+
+
+    Scenario: Interpolation line with duplicated points
+        Given the grid
+          | 7 | 10 | 8 | 11 | 9 |
+        Given the places
+          | osm | class | type   | housenr | geometry |
+          | N1  | place | house  | 2       | 7 |
+          | N2  | place | house  | 6       | 8 |
+          | N3  | place | house  | 10      | 8 |
+          | N4  | place | house  | 14      | 9 |
+        And the places
+          | osm | class | type   | addr+interpolation | geometry |
+          | W1  | place | houses | even               | 7,8,8,9  |
+        And the ways
+          | id | nodes   |
+          | 1  | 1,2,3,4 |
+        When importing
+        Then W1 expands to interpolation
+          | start | end | geometry |
+          | 4     | 4   | 10       |
+          | 12    | 12  | 11       |
+
+
+    Scenario: Interpolaton line with broken way geometry (Github #2986)
+        Given the grid
+          | 1 | 8 | 10 | 11 | 9 | 2 | 3 | 4 |
+        Given the places
+          | osm | class | type   | housenr |
+          | N1  | place | house  | 2       |
+          | N2  | place | house  | 8       |
+          | N3  | place | house  | 12      |
+          | N4  | place | house  | 14      |
+        And the places
+          | osm | class | type   | addr+interpolation | geometry |
+          | W1  | place | houses | even               | 8,9      |
+        And the ways
+          | id | nodes       |
+          | 1  | 1,8,9,2,3,4 |
+        When importing
+        Then W1 expands to interpolation
+          | start | end | geometry |
+          | 4     | 6   | 10,11    |

test/bdd/osm2pgsql/import/tags.feature

@@ -101,6 +101,19 @@ Feature: Tag evaluation
             | N6003  | shop  | -                   |
 
 
+    Scenario: Postcode areas
+        When loading osm data
+            """
+            n1 x12.36853 y51.50618
+            n2 x12.36853 y51.42362
+            n3 x12.63666 y51.42362
+            n4 x12.63666 y51.50618
+            w1 Tboundary=postal_code,ref=3456 Nn1,n2,n3,n4,n1
+            """
+        Then place contains exactly
+            | object | class    | type        | name          |
+            | W1     | boundary | postal_code | 'ref': '3456' |
+
     Scenario: Main with extra
         When loading osm data
             """

test/python/tools/test_add_osm_data.py

@@ -24,10 +24,14 @@ class CaptureGetUrl:
         return '<xml></xml>'
 
 
-def test_import_osm_file_simple(table_factory, osm2pgsql_options, capfd):
-    table_factory('place', content=((1, ), ))
+@pytest.fixture(autouse=True)
+def setup_delete_postprocessing(temp_db_cursor):
+    temp_db_cursor.execute("""CREATE OR REPLACE FUNCTION flush_deleted_places()
+                              RETURNS INTEGER AS $$ SELECT 1 $$ LANGUAGE SQL""")
 
-    assert add_osm_data.add_data_from_file(Path('change.osm'), osm2pgsql_options) == 0
+def test_import_osm_file_simple(dsn, table_factory, osm2pgsql_options, capfd):
+
+    assert add_osm_data.add_data_from_file(dsn, Path('change.osm'), osm2pgsql_options) == 0
     captured = capfd.readouterr()
 
     assert '--append' in captured.out
@@ -41,11 +45,11 @@ def test_import_osm_file_simple(table_factory, osm2pgsql_options, capfd):
 @pytest.mark.parametrize("osm_type", ['node', 'way', 'relation'])
 @pytest.mark.parametrize("main_api,url", [(True, 'https://www.openstreetmap.org/api'),
                                           (False, 'https://overpass-api.de/api/interpreter?')])
-def test_import_osm_object_main_api(osm2pgsql_options, monkeypatch, capfd,
-                                    osm_type, main_api, url):
+def test_import_osm_object_main_api(dsn, osm2pgsql_options, monkeypatch,
+                                    capfd, osm_type, main_api, url):
     get_url_mock = CaptureGetUrl(monkeypatch)
 
-    add_osm_data.add_osm_object(osm_type, 4536, main_api, osm2pgsql_options)
+    add_osm_data.add_osm_object(dsn, osm_type, 4536, main_api, osm2pgsql_options)
     captured = capfd.readouterr()
 
     assert get_url_mock.url.startswith(url)