diff --git a/nominatim/api/reverse.py b/nominatim/api/reverse.py index 2ef4c05615..7b1c6431f8 100644 --- a/nominatim/api/reverse.py +++ b/nominatim/api/reverse.py @@ -402,9 +402,8 @@ def _place_inside_area_query() -> SaSelect: .where(t.c.rank_search <= MAX_RANK_PARAM)\ .where(t.c.indexed_status == 0)\ .where(snfn.select_index_placex_geometry_reverse_lookupplacenode('placex'))\ - .where(t.c.geometry - .ST_Buffer(snfn.ReversePlaceDiameter(t.c.rank_search)) - .intersects(WKT_PARAM))\ + .where(snfn.IntersectsReverseDistance(t.c.geometry, t.c.rank_search, + WKT_PARAM))\ .order_by(sa.desc(t.c.rank_search))\ .limit(50)\ .subquery('places') @@ -413,7 +412,7 @@ def _place_inside_area_query() -> SaSelect: return _select_from_placex(inner, False)\ .join(touter, touter.c.geometry.ST_Contains(inner.c.geometry))\ .where(touter.c.place_id == address_id)\ - .where(inner.c.distance < snfn.ReversePlaceDiameter(inner.c.rank_search))\ + .where(snfn.IsBelowReverseDistance(inner.c.distance, inner.c.rank_search))\ .order_by(sa.desc(inner.c.rank_search), inner.c.distance)\ .limit(1) @@ -440,9 +439,8 @@ async def _lookup_area_others(self) -> Optional[SaRow]: .where(t.c.indexed_status == 0)\ .where(t.c.linked_place_id == None)\ .where(self._filter_by_layer(t))\ - .where(t.c.geometry - .ST_Buffer(snfn.ReversePlaceDiameter(t.c.rank_search)) - .intersects(WKT_PARAM))\ + .where(snfn.IntersectsReverseDistance(t.c.geometry, t.c.rank_search, + WKT_PARAM))\ .order_by(sa.desc(t.c.rank_search))\ .order_by('distance')\ .limit(50)\ @@ -516,15 +514,13 @@ def _base_query() -> SaSelect: .where(t.c.indexed_status == 0)\ .where(t.c.country_code.in_(ccodes))\ .where(snfn.select_index_placex_geometry_reverse_lookupplacenode('placex'))\ - .where(t.c.geometry - .ST_Buffer(snfn.ReversePlaceDiameter(t.c.rank_search)) - .intersects(WKT_PARAM))\ + .where(snfn.IntersectsReverseDistance(t.c.geometry, t.c.rank_search, WKT_PARAM))\ .order_by(sa.desc(t.c.rank_search))\ .limit(50)\ .subquery('area') return _select_from_placex(inner, False)\ - .where(inner.c.distance < snfn.ReversePlaceDiameter(inner.c.rank_search))\ + .where(snfn.IsBelowReverseDistance(inner.c.distance, inner.c.rank_search))\ .order_by(sa.desc(inner.c.rank_search), inner.c.distance)\ .limit(1) diff --git a/nominatim/db/sqlalchemy_functions.py b/nominatim/db/sqlalchemy_functions.py index f7f88c2baa..037cbcb156 100644 --- a/nominatim/db/sqlalchemy_functions.py +++ b/nominatim/db/sqlalchemy_functions.py @@ -16,17 +16,51 @@ # pylint: disable=abstract-method,missing-function-docstring,consider-using-f-string -class ReversePlaceDiameter(sa.sql.functions.GenericFunction): - type_ = sa.Float +class IsBelowReverseDistance(sa.sql.functions.GenericFunction): + type_ = sa.Boolean + inherit_cache = True + +@compiles(IsBelowReverseDistance) +def default_reverse_place_diameter(element, compiler, **kw): + dist, rank = list(element.clauses) + return "%s < reverse_place_diameter(%s)" % ( + compiler.process(dist, **kw), + compiler.process(rank, **kw)) + + +@compiles(IsBelowReverseDistance, 'sqlite') +def sqlite_reverse_place_diameter(element, compiler, **kw): + dist, rank = list(element.clauses) + return "%s < 14.0 * exp(-0.2 * %s) - 0.03" % ( + compiler.process(dist, **kw), + compiler.process(rank, **kw)) + + +class IntersectsReverseDistance(sa.sql.functions.GenericFunction): + type_ = sa.Boolean inherit_cache = True -@compiles(ReversePlaceDiameter) +@compiles(IntersectsReverseDistance) def default_reverse_place_diameter(element, compiler, **kw): - return "reverse_place_diameter(%s)" % compiler.process(element.clauses, **kw) + geom1, rank, geom2 = list(element.clauses) + return "ST_Buffer(%s, reverse_place_diameter(%s)) && %s" % ( + compiler.process(geom1, **kw), + compiler.process(rank, **kw), + compiler.process(geom2, **kw)) -@compiles(ReversePlaceDiameter, 'sqlite') +@compiles(IntersectsReverseDistance, 'sqlite') def sqlite_reverse_place_diameter(element, compiler, **kw): - return "14.0 * exp(-0.2 * %s) - 0.03" % compiler.process(element.clauses, **kw) + geom1, rank, geom2 = list(element.clauses) + return "MbrIntersects(%s, ST_Expand(%s, 14.0 * exp(-0.2 * %s) - 0.03)) "\ + "AND %s.ROWID IN (SELECT ROWID FROM SpatialIndex AS si "\ + "WHERE si.f_table_name = '%s' and si.f_geometry_column = '%s' "\ + "AND search_frame = ST_Expand(%s, 14.0 * exp(-0.2 * %s) - 0.03))" % ( + compiler.process(geom1, **kw), + compiler.process(geom2, **kw), + compiler.process(rank, **kw), + geom1.table.name, geom1.table.name, geom1.name, + compiler.process(geom2, **kw), + compiler.process(rank, **kw)) diff --git a/nominatim/db/sqlalchemy_types.py b/nominatim/db/sqlalchemy_types.py index d03819009a..206a0df757 100644 --- a/nominatim/db/sqlalchemy_types.py +++ b/nominatim/db/sqlalchemy_types.py @@ -33,6 +33,30 @@ def spatialite_dwithin(element, compiler, **kw): return compiler.process(sa.func.ST_Intersects(geom1, sa.func.ST_Buffer(geom2, dist)), **kw) +class Geometry_DWithin_Column(sa.sql.expression.FunctionElement): + type = sa.Boolean + name = 'Geometry_DWithin_Column' + inherit_cache = True + +@compiles(Geometry_DWithin_Column) +def default_dwithin_column(element, compiler, **kw): + return "ST_DWithin(%s)" % compiler.process(element.clauses, **kw) + +@compiles(Geometry_DWithin_Column, 'sqlite') +def spatialite_dwithin_column(element, compiler, **kw): + geom1, geom2, dist = list(element.clauses) + return "ST_Distance(%s, %s) < %s and "\ + "%s.ROWID IN (SELECT ROWID FROM SpatialIndex "\ + "WHERE f_table_name = '%s' AND f_geometry_column = '%s' "\ + "AND search_frame = ST_Expand(%s, %s))" %( + compiler.process(geom1, **kw), + compiler.process(geom2, **kw), + compiler.process(dist, **kw), + geom1.table.name, geom1.table.name, geom1.name, + compiler.process(geom2, **kw), + compiler.process(dist, **kw)) + + class Geometry_LineLocatePoint(sa.sql.functions.GenericFunction): type = sa.Float name = 'Geometry_LineLocatePoint' @@ -127,7 +151,7 @@ def spatialite_distance_spheroid(element, compiler, **kw): return "Distance(%s, %s, true)" % (compiler.process(arg1, **kw), compiler.process(arg2, **kw)) -class Geometry_IntersectsBbox(sa.sql.functions.GenericFunction): +class Geometry_IntersectsBbox(sa.sql.expression.FunctionElement): type = sa.Boolean inherit_cache = True @@ -141,6 +165,28 @@ def spatialite_intersects_bbox(element, compiler, **kw): return "MbrIntersects(%s)" % compiler.process(element.clauses, **kw) +class Geometry_IntersectsColumn(sa.sql.expression.FunctionElement): + type = sa.Boolean + inherit_cache = True + +@compiles(Geometry_IntersectsColumn) +def default_intersects_column(element, compiler, **kw): + arg1, arg2 = list(element.clauses) + return "%s && %s" % (compiler.process(arg1, **kw), compiler.process(arg2, **kw)) + +@compiles(Geometry_IntersectsColumn, 'sqlite') +def spatialite_intersects_column(element, compiler, **kw): + arg1, arg2 = list(element.clauses) + return "MbrIntersects(%s, %s) and "\ + "%s.ROWID IN (SELECT ROWID FROM SpatialIndex "\ + "WHERE f_table_name = '%s' AND f_geometry_column = '%s' "\ + "AND search_frame = %s)" %( + compiler.process(arg1, **kw), + compiler.process(arg2, **kw), + arg1.table.name, arg1.table.name, arg1.name, + compiler.process(arg2, **kw)) + + class Geometry(types.UserDefinedType): # type: ignore[type-arg] """ Simplified type decorator for PostGIS geometry. This type only supports geometries in 4326 projection. @@ -181,8 +227,13 @@ def column_expression(self, col): class comparator_factory(types.UserDefinedType.Comparator): # type: ignore[type-arg] - def intersects(self, other: SaColumn) -> 'sa.Operators': - return Geometry_IntersectsBbox(self, other, type_=sa.Boolean) + def intersects(self, other: SaColumn, use_index: bool = True) -> 'sa.Operators': + if not use_index: + return Geometry_IntersectsBbox(sa.func.coalesce(sa.null(), self), other) + if isinstance(self.expr, sa.Column): + return Geometry_IntersectsColumn(self.expr, other) + + return Geometry_IntersectsBbox(self, other) def is_line_like(self) -> SaColumn: @@ -197,18 +248,14 @@ def distance_spheroid(self, other: SaColumn) -> SaColumn: return Geometry_DistanceSpheroid(self, other) - def ST_DWithin(self, other: SaColumn, distance: SaColumn) -> SaColumn: + def ST_DWithin(self, other: SaColumn, distance: SaColumn, use_index: bool = True) -> SaColumn: + if not use_index: + return Geometry_DWithin(sa.func.coalesce(sa.null(), self), other, distance) + if isinstance(self.expr, sa.Column): + return Geometry_DWithin_Column(self.expr, other, distance) return Geometry_DWithin(self, other, distance) - def ST_DWithin_no_index(self, other: SaColumn, distance: SaColumn) -> SaColumn: - return Geometry_DWithin(sa.func.coalesce(sa.null(), self), other, distance) - - - def ST_Intersects_no_index(self, other: SaColumn) -> 'sa.Operators': - return Geometry_IntersectsBbox(sa.func.coalesce(sa.null(), self), other) - - def ST_Distance(self, other: SaColumn) -> SaColumn: return sa.func.ST_Distance(self, other, type_=sa.Float) diff --git a/nominatim/tools/convert_sqlite.py b/nominatim/tools/convert_sqlite.py index c9de59d9f6..fdc460e282 100644 --- a/nominatim/tools/convert_sqlite.py +++ b/nominatim/tools/convert_sqlite.py @@ -50,7 +50,7 @@ async def write(self): async def create_tables(self): # Modifications to how tables are created. self.outmeta.remove(self.outconn.t.search_name) - trim_table(self.outconn.t.placex, ['partition']) + #trim_table(self.outconn.t.placex, ['partition']) trim_table(self.outconn.t.addressline, ['cached_rank_address']) trim_table(self.outconn.t.country_name, ['country_default_language_code', 'partition']) @@ -84,12 +84,21 @@ async def copy_data(self) -> None: async def create_indexes(self): await self.create_spatial_index('country_grid', 'geometry') await self.create_spatial_index('placex', 'geometry') + await self.create_spatial_index('osmline', 'linegeo') + await self.create_spatial_index('tiger', 'linegeo') + await self.create_index('placex', 'place_id') + await self.create_index('placex', 'rank_address') + await self.create_index('addressline', 'place_id') async def create_spatial_index(self, table, column): await self.outconn.execute(sa.select(sa.func.CreateSpatialIndex(getattr(self.outconn.t, table).name, column))) + async def create_index(self, table, column): + table = getattr(self.outconn.t, table) + await self.outconn.connection.run_sync(sa.Index(f"idx_{table}_{column}", getattr(table.c, column)).create) + async def copy_table(self): columns = [sa.func.ST_AsText(c).label(c.name) if isinstance(c.type, Geometry) else c diff --git a/test/python/api/conftest.py b/test/python/api/conftest.py index 8fc2ac82f9..9c67ee761f 100644 --- a/test/python/api/conftest.py +++ b/test/python/api/conftest.py @@ -203,6 +203,8 @@ async def create_tables(self): await conn.execute(sa.select( sa.func.RecoverGeometryColumn(table.name, col.name, 4326, col.type.subtype.upper(), 'XY'))) + await conn.execute(sa.select( + sa.func.CreateSpatialIndex(table.name, col.name))) @contextlib.contextmanager