From d743cf308ee6b30bb5ea39e903c4fae496fc14bc Mon Sep 17 00:00:00 2001
From: Sarah Hoffmann <lonvia@denofr.de>
Date: Sat, 8 Jul 2023 21:47:07 +0200
Subject: [PATCH] avoid index-use on rank parameters for reverse lookups

---
 nominatim/api/reverse.py         | 25 ++++++++++++++++++-------
 nominatim/db/sqlalchemy_types.py |  4 +---
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/nominatim/api/reverse.py b/nominatim/api/reverse.py
index 0163f935..3a8be0fd 100644
--- a/nominatim/api/reverse.py
+++ b/nominatim/api/reverse.py
@@ -28,6 +28,13 @@ RowFunc = Callable[[Optional[SaRow], Type[nres.ReverseResult]], Optional[nres.Re
 WKT_PARAM: SaBind = sa.bindparam('wkt', type_=Geometry)
 MAX_RANK_PARAM: SaBind = sa.bindparam('max_rank')
 
+def no_index(expr: SaColumn) -> SaColumn:
+    """ Wrap the given expression, so that the query planner will
+        refrain from using the expression for index lookup.
+    """
+    return sa.func.coalesce(sa.null(), expr) # pylint: disable=not-callable
+
+
 def _select_from_placex(t: SaFromClause, use_wkt: bool = True) -> SaSelect:
     """ Create a select statement with the columns relevant for reverse
         results.
@@ -172,12 +179,17 @@ class ReverseGeocoder:
         """
         t = self.conn.t.placex
 
+        # PostgreSQL must not get the distance as a parameter because
+        # there is a danger it won't be able to proberly estimate index use
+        # when used with prepared statements
+        dist_param = sa.text(f"{distance}")
+
         sql = _select_from_placex(t)\
-                .where(t.c.geometry.ST_DWithin(WKT_PARAM, distance))\
+                .where(t.c.geometry.ST_DWithin(WKT_PARAM, dist_param))\
                 .where(t.c.indexed_status == 0)\
                 .where(t.c.linked_place_id == None)\
                 .where(sa.or_(sa.not_(t.c.geometry.is_area()),
-                              t.c.centroid.ST_Distance(WKT_PARAM) < distance))\
+                              t.c.centroid.ST_Distance(WKT_PARAM) < dist_param))\
                 .order_by('distance')\
                 .limit(1)
 
@@ -186,17 +198,16 @@ class ReverseGeocoder:
         restrict: List[SaColumn] = []
 
         if self.layer_enabled(DataLayer.ADDRESS):
-            restrict.append(sa.and_(t.c.rank_address >= 26,
-                                    t.c.rank_address <= min(29, self.max_rank)))
+            restrict.append(no_index(t.c.rank_address).between(26, min(29, self.max_rank)))
             if self.max_rank == 30:
                 restrict.append(_is_address_point(t))
         if self.layer_enabled(DataLayer.POI) and self.max_rank == 30:
-            restrict.append(sa.and_(t.c.rank_search == 30,
+            restrict.append(sa.and_(no_index(t.c.rank_search) == 30,
                                     t.c.class_.not_in(('place', 'building')),
                                     sa.not_(t.c.geometry.is_line_like())))
         if self.has_feature_layers():
-            restrict.append(sa.and_(t.c.rank_search.between(26, MAX_RANK_PARAM),
-                                    t.c.rank_address == 0,
+            restrict.append(sa.and_(no_index(t.c.rank_search).between(26, MAX_RANK_PARAM),
+                                    no_index(t.c.rank_address) == 0,
                                     self._filter_by_layer(t)))
 
         if not restrict:
diff --git a/nominatim/db/sqlalchemy_types.py b/nominatim/db/sqlalchemy_types.py
index 1718b873..f31966cd 100644
--- a/nominatim/db/sqlalchemy_types.py
+++ b/nominatim/db/sqlalchemy_types.py
@@ -48,9 +48,7 @@ class Geometry(types.UserDefinedType): # type: ignore[type-arg]
 
 
     def bind_expression(self, bindvalue: SaBind) -> SaColumn:
-        return sa.func.ST_GeomFromText(bindvalue,
-                                       sa.bindparam('geometry_srid', value=4326, literal_execute=True),
-                                       type_=self)
+        return sa.func.ST_GeomFromText(bindvalue, sa.text('4326'), type_=self)
 
 
     class comparator_factory(types.UserDefinedType.Comparator): # type: ignore[type-arg]
-- 
2.39.5