Fix DistanceWithin algorithm with PostgreSQL provider (backport)

python/core/auto_generated/qgsfeatureiterator.sip.in

@@ -75,6 +75,12 @@ This indicates that there is something wrong with the expression compiler.
 .. versionadded:: 3.2
 %End
 
+    enum class RequestToSourceCrsResult
+    {
+      Success,
+      DistanceWithinMustBeCheckedManually,
+    };
+
   protected:
 
     virtual bool fetchFeature( QgsFeature &f ) = 0;
@@ -138,6 +144,21 @@ Will throw a :py:class:`QgsCsException` if the rect cannot be transformed from t
 .. versionadded:: 3.0
 %End
 
+    RequestToSourceCrsResult updateRequestToSourceCrs( QgsFeatureRequest &request, const QgsCoordinateTransform &transform ) const throw( QgsCsException );
+%Docstring
+Update a :py:class:`QgsFeatureRequest` so that spatial filters are
+transformed to the source's coordinate reference system.
+Iterators should call this method against the request used for filtering
+features to ensure that any :py:func:`QgsFeatureRequest.destinationCrs()` set on the request is respected.
+
+:return: result of operation. See QgsAbstractFeatureIterator.RequestToSourceCrsResult for interpretation.
+
+:raises QgsCsException: if the rect cannot be transformed from the destination CRS.
+
+
+.. versionadded:: 3.22
+%End
+
 
 
 

python/core/auto_generated/qgsfeaturerequest.sip.in

@@ -205,6 +205,10 @@ Dumps the content to an SQL equivalent
 %End
 
 
+        bool operator==( const OrderByClause &v ) const;
+
+        bool operator!=( const OrderByClause &v ) const;
+
     };
 
 

python/core/auto_generated/qgssimplifymethod.sip.in

@@ -74,6 +74,10 @@ Gets whether the simplification executes after fetch the geometries from provide
 Creates a geometry simplifier according to specified method
 %End
 
+    bool operator==( const QgsSimplifyMethod &v ) const;
+
+    bool operator!=( const QgsSimplifyMethod &v ) const;
+
   protected:
 };
 

src/core/qgsfeatureiterator.cpp

@@ -17,6 +17,7 @@
 
 #include "qgssimplifymethod.h"
 #include "qgsexception.h"
+#include "qgslinestring.h"
 #include "qgsexpressionsorter.h"
 #include "qgsfeedback.h"
 #include "qgscoordinatetransform.h"
@@ -120,6 +121,39 @@ void QgsAbstractFeatureIterator::geometryToDestinationCrs( QgsFeature &feature,
   }
 }
 
+QgsAbstractFeatureIterator::RequestToSourceCrsResult QgsAbstractFeatureIterator::updateRequestToSourceCrs( QgsFeatureRequest &request, const QgsCoordinateTransform &transform ) const
+{
+  if ( transform.isShortCircuited() )
+    return RequestToSourceCrsResult::Success; // nothing to do
+
+  switch ( request.spatialFilterType() )
+  {
+    case Qgis::SpatialFilterType::NoFilter:
+      return RequestToSourceCrsResult::Success;
+
+    case Qgis::SpatialFilterType::BoundingBox:
+    {
+      QgsRectangle newRect = transform.transformBoundingBox( request.filterRect(), Qgis::TransformDirection::Reverse );
+      request.setFilterRect( newRect );
+      return RequestToSourceCrsResult::Success;
+    }
+    case Qgis::SpatialFilterType::DistanceWithin:
+    {
+      // we can't safely handle a distance within query, as we cannot transform the
+      // static within tolerance distance from one CRS to a static distance in a different CRS.
+
+      // in this case we transform the request's distance within requirement to a "worst case" bounding box filter, so
+      // that the request itself can still take advantage of spatial indices even when we have to do the distance within check locally
+      QgsRectangle newRect = transform.transformBoundingBox( request.filterRect(), Qgis::TransformDirection::Reverse );
+      request.setFilterRect( newRect );
+
+      return RequestToSourceCrsResult::DistanceWithinMustBeCheckedManually;
+    }
+  }
+
+  BUILTIN_UNREACHABLE
+}
+
 QgsRectangle QgsAbstractFeatureIterator::filterRectToSourceCrs( const QgsCoordinateTransform &transform ) const
 {
   if ( mRequest.filterRect().isNull() )

src/core/qgsfeatureiterator.h

@@ -91,6 +91,17 @@ class CORE_EXPORT QgsAbstractFeatureIterator
      */
     bool compileFailed() const;
 
+    /**
+     * Possible results from the updateRequestToSourceCrs() method.
+     *
+     * \since QGIS 3.22
+     */
+    enum class RequestToSourceCrsResult : int
+    {
+      Success, //!< Request was successfully updated to the source CRS, or no changes were required
+      DistanceWithinMustBeCheckedManually, //!< The distance within request cannot be losslessly updated to the source CRS, and callers will need to take appropriate steps to handle the distance within requirement manually during feature iteration
+    };
+
   protected:
 
     /**
@@ -149,6 +160,20 @@ class CORE_EXPORT QgsAbstractFeatureIterator
      */
     QgsRectangle filterRectToSourceCrs( const QgsCoordinateTransform &transform ) const SIP_THROW( QgsCsException );
 
+    /**
+     * Update a QgsFeatureRequest so that spatial filters are
+     * transformed to the source's coordinate reference system.
+     * Iterators should call this method against the request used for filtering
+     * features to ensure that any QgsFeatureRequest::destinationCrs() set on the request is respected.
+     *
+     * \returns result of operation. See QgsAbstractFeatureIterator::RequestToSourceCrsResult for interpretation.
+     *
+     * \throws QgsCsException if the rect cannot be transformed from the destination CRS.
+     *
+     * \since QGIS 3.22
+     */
+    RequestToSourceCrsResult updateRequestToSourceCrs( QgsFeatureRequest &request, const QgsCoordinateTransform &transform ) const SIP_THROW( QgsCsException );
+
     //! A copy of the feature request.
     QgsFeatureRequest mRequest;
 

src/core/qgsfeaturerequest.h

@@ -222,6 +222,18 @@ class CORE_EXPORT QgsFeatureRequest
 
         // friend inline int qHash(const OrderByClause &a) { return qHash(a.mExpression.expression()) ^ qHash(a.mAscending) ^ qHash( a.mNullsFirst); }
 
+        bool operator==( const OrderByClause &v ) const
+        {
+          return mExpression == v.mExpression &&
+                 mAscending == v.mAscending &&
+                 mNullsFirst == v.mNullsFirst;
+        }
+
+        bool operator!=( const OrderByClause &v ) const
+        {
+          return !( v == *this );
+        }
+
       private:
         QgsExpression mExpression;
         bool mAscending;

src/core/qgssimplifymethod.cpp

@@ -54,3 +54,17 @@ QgsAbstractGeometrySimplifier *QgsSimplifyMethod::createGeometrySimplifier( cons
     return nullptr;
   }
 }
+
+bool QgsSimplifyMethod::operator==( const QgsSimplifyMethod &v ) const
+{
+  return
+    mMethodType == v.mMethodType &&
+    mTolerance == v.mTolerance &&
+    mThreshold == v.mThreshold &&
+    mForceLocalOptimization == v.mForceLocalOptimization;
+}
+
+bool QgsSimplifyMethod::operator!=( const QgsSimplifyMethod &v ) const
+{
+  return !( v == *this );
+}

src/core/qgssimplifymethod.h

@@ -61,6 +61,12 @@ class CORE_EXPORT QgsSimplifyMethod
     //! Creates a geometry simplifier according to specified method
     static QgsAbstractGeometrySimplifier *createGeometrySimplifier( const QgsSimplifyMethod &simplifyMethod );
 
+    //! Equality operator
+    bool operator==( const QgsSimplifyMethod &v ) const;
+
+    //! Inequality operator
+    bool operator!=( const QgsSimplifyMethod &v ) const;
+
   protected:
     //! Simplification method
     MethodType mMethodType = QgsSimplifyMethod::NoSimplification;

src/core/vector/qgsvectorlayerfeatureiterator.cpp

@@ -128,19 +128,11 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
   {
     mTransform = QgsCoordinateTransform( mSource->mCrs, mRequest.destinationCrs(), mRequest.transformContext() );
   }
-  try
-  {
-    mFilterRect = filterRectToSourceCrs( mTransform );
-  }
-  catch ( QgsCsException & )
-  {
-    // can't reproject mFilterRect
-    close();
-    return;
-  }
-
 
   // prepare spatial filter geometries for optimal speed
+  // since the mDistanceWithin* constraint member variables are all in the DESTINATION CRS,
+  // we set all these upfront before any transformation to the source CRS is done.
+
   switch ( mRequest.spatialFilterType() )
   {
     case Qgis::SpatialFilterType::NoFilter:
@@ -150,6 +142,10 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
     case Qgis::SpatialFilterType::DistanceWithin:
       if ( !mRequest.referenceGeometry().isEmpty() )
       {
+        // Note that regardless of whether or not we'll ultimately be able to handoff this check to the underlying provider,
+        // we still need these reference geometry constraints in the vector layer iterator as we need them to check against
+        // the features from the vector layer's edit buffer! (In other words, we cannot completely hand off responsibility for
+        // these checks to the provider and ignore them locally)
         mDistanceWithinGeom = mRequest.referenceGeometry();
         mDistanceWithinEngine = mRequest.referenceGeometryEngine();
         mDistanceWithinEngine->prepareGeometry();
@@ -158,10 +154,29 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
       break;
   }
 
-  if ( !mFilterRect.isNull() )
+  bool canDelegateLimitToProvider = true;
+  try
+  {
+    switch ( updateRequestToSourceCrs( mRequest, mTransform ) )
+    {
+      case QgsAbstractFeatureIterator::RequestToSourceCrsResult::Success:
+        break;
+
+      case QgsAbstractFeatureIterator::RequestToSourceCrsResult::DistanceWithinMustBeCheckedManually:
+        // we have to disable any limit on the provider's request -- since that request may be returning features which are outside the
+        // distance tolerance, we'll have to fetch them all and then handle the limit check manually only after testing for the distance within constraint
+        canDelegateLimitToProvider = false;
+        break;
+    }
+
+    // mFilterRect is in the source CRS, so we set that now (after request transformation has been done)
+    mFilterRect = mRequest.filterRect();
+  }
+  catch ( QgsCsException & )
   {
-    // update request to be the unprojected filter rect
-    mRequest.setFilterRect( mFilterRect );
+    // can't reproject request filters
+    close();
+    return;
   }
 
   // check whether the order by clause(s) can be delegated to the provider
@@ -218,6 +233,11 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
     mProviderRequest.setOrderBy( QgsFeatureRequest::OrderBy() );
   }
 
+  if ( !canDelegateLimitToProvider )
+  {
+    mProviderRequest.setLimit( -1 );
+  }
+
   if ( mProviderRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
   {
     // prepare list of attributes to match provider fields

src/core/vector/qgsvectorlayerfeatureiterator.h

@@ -268,9 +268,11 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
     QgsFeatureRequest mChangedFeaturesRequest;
     QgsFeatureIterator mChangedFeaturesIterator;
 
+    // filter bounding box constraint, in SOURCE CRS
     QgsRectangle mFilterRect;
     QgsCoordinateTransform mTransform;
 
+    // distance within constraint reference geometry and distance IN DESTINATION CRS
     QgsGeometry mDistanceWithinGeom;
     std::shared_ptr< QgsGeometryEngine > mDistanceWithinEngine;
     double mDistanceWithin = 0;

tests/src/core/CMakeLists.txt

@@ -53,6 +53,7 @@ set(TESTS
  testqgsexpression.cpp
  testqgsexpressioncontext.cpp
  testqgsfeature.cpp
+ testqgsfeaturerequest.cpp
  testqgsfield.cpp
  testqgsfields.cpp
  testqgsfilledmarker.cpp
@@ -163,6 +164,7 @@ set(TESTS
  testqgssettingsregistry.cpp
  testqgsshapeburst.cpp
  testqgssimplemarker.cpp
+ testqgssimplifymethod.cpp
  testqgssnappingutils.cpp
  testqgsspatialindex.cpp
  testqgsspatialindexkdbush.cpp