[oracle] Always check bounding box intersection when using filter rect

nyalldawson · nyalldawson · commit 2d7632fc1a3e · 2018-04-24T09:13:18.000+12:00
Because oracle sdo_filter sometimes returns results outside
the given filter (depending on the spatial index), and is
only designed for use as a fast "first-pass" filter.

From the Oracle docs:

"This operator performs only a primary filter operation..

The secondary filtering operation, performed by the SDO_RELATE operator,
can be used to determine with certainty if objects interact spatially."

Instead of using SDO_RELATE we can instead just do a bounding
box intersection inside QGIS if an exact intersection is not
required.

Fixes failing provider conformance test suite for oracle provider.
diff --git a/src/providers/oracle/qgsoraclefeatureiterator.cpp b/src/providers/oracle/qgsoraclefeatureiterator.cpp
@@ -308,22 +308,22 @@ bool QgsOracleFeatureIterator::fetchFeature( QgsFeature &feature )
       {
         if ( !feature.hasGeometry() )
         {
-          QgsDebugMsg( QStringLiteral( "no geometry to intersect" ) );
+          QgsDebugMsgLevel( QStringLiteral( "no geometry to intersect" ), 4 );
           continue;
         }
 
         if ( ( mRequest.flags() & QgsFeatureRequest::ExactIntersect ) == 0 )
         {
-          // couldn't use sdo_filter earlier
-          if ( !mSource->mHasSpatialIndex )
+          // even if we could use sdo_filter earlier, we still need to double-check the results
+          // as sdo_filter can return results outside the filter (it's only a first-pass
+          // filtering operation!)
+
+          // only want features which intersect with bbox
+          if ( !feature.geometry().boundingBox().intersects( mFilterRect ) )
           {
-            // only intersect with bbox
-            if ( !feature.geometry().boundingBox().intersects( mFilterRect ) )
-            {
-              // skip feature that don't intersect with our rectangle
-              QgsDebugMsg( QStringLiteral( "no bbox intersect" ) );
-              continue;
-            }
+            // skip feature that don't intersect with our rectangle
+            QgsDebugMsgLevel( QStringLiteral( "no bbox intersect" ), 4 );
+            continue;
           }
         }
         else if ( !mConnection->hasSpatial() || !mSource->mHasSpatialIndex )
@@ -332,7 +332,7 @@ bool QgsOracleFeatureIterator::fetchFeature( QgsFeature &feature )
           if ( !feature.geometry().intersects( mFilterRect ) )
           {
             // skip feature that don't intersect with our rectangle
-            QgsDebugMsg( QStringLiteral( "no exact intersect" ) );
+            QgsDebugMsgLevel( QStringLiteral( "no exact intersect" ), 4 );
             continue;
           }
         }

Original file line number	Diff line number	Diff line change
`@@ -308,22 +308,22 @@ bool QgsOracleFeatureIterator::fetchFeature( QgsFeature &feature )`
`308`	`308`	`{`
`309`	`309`	`if ( !feature.hasGeometry() )`
`310`	`310`	`{`
`311`		`- QgsDebugMsg( QStringLiteral( "no geometry to intersect" ) );`
	`311`	`+ QgsDebugMsgLevel( QStringLiteral( "no geometry to intersect" ), 4 );`
`312`	`312`	`continue;`
`313`	`313`	`}`
`314`	`314`
`315`	`315`	`if ( ( mRequest.flags() & QgsFeatureRequest::ExactIntersect ) == 0 )`
`316`	`316`	`{`
`317`		`- // couldn't use sdo_filter earlier`
`318`		`- if ( !mSource->mHasSpatialIndex )`
	`317`	`+ // even if we could use sdo_filter earlier, we still need to double-check the results`
	`318`	`+ // as sdo_filter can return results outside the filter (it's only a first-pass`
	`319`	`+ // filtering operation!)`
	`320`	`+`
	`321`	`+ // only want features which intersect with bbox`
	`322`	`+ if ( !feature.geometry().boundingBox().intersects( mFilterRect ) )`
`319`	`323`	`{`
`320`		`- // only intersect with bbox`
`321`		`- if ( !feature.geometry().boundingBox().intersects( mFilterRect ) )`
`322`		`- {`
`323`		`- // skip feature that don't intersect with our rectangle`
`324`		`- QgsDebugMsg( QStringLiteral( "no bbox intersect" ) );`
`325`		`- continue;`
`326`		`- }`
	`324`	`+ // skip feature that don't intersect with our rectangle`
	`325`	`+ QgsDebugMsgLevel( QStringLiteral( "no bbox intersect" ), 4 );`
	`326`	`+ continue;`
`327`	`327`	`}`
`328`	`328`	`}`
`329`	`329`	`else if ( !mConnection->hasSpatial() \|\| !mSource->mHasSpatialIndex )`
`@@ -332,7 +332,7 @@ bool QgsOracleFeatureIterator::fetchFeature( QgsFeature &feature )`
`332`	`332`	`if ( !feature.geometry().intersects( mFilterRect ) )`
`333`	`333`	`{`
`334`	`334`	`// skip feature that don't intersect with our rectangle`
`335`		`- QgsDebugMsg( QStringLiteral( "no exact intersect" ) );`
	`335`	`+ QgsDebugMsgLevel( QStringLiteral( "no exact intersect" ), 4 );`
`336`	`336`	`continue;`
`337`	`337`	`}`
`338`	`338`	`}`