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

nyalldawson · nyalldawson · commit f9a8d9f72700 · 2018-05-03T11:23:23.000+10: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. (cherry-picked from 2d7632f)
diff --git a/src/providers/oracle/qgsoraclefeatureiterator.cpp b/src/providers/oracle/qgsoraclefeatureiterator.cpp
@@ -305,22 +305,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 )
@@ -329,7 +329,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
`@@ -305,22 +305,22 @@ bool QgsOracleFeatureIterator::fetchFeature( QgsFeature &feature )`
`305`	`305`	`{`
`306`	`306`	`if ( !feature.hasGeometry() )`
`307`	`307`	`{`
`308`		`- QgsDebugMsg( QStringLiteral( "no geometry to intersect" ) );`
	`308`	`+ QgsDebugMsgLevel( QStringLiteral( "no geometry to intersect" ), 4 );`
`309`	`309`	`continue;`
`310`	`310`	`}`
`311`	`311`
`312`	`312`	`if ( ( mRequest.flags() & QgsFeatureRequest::ExactIntersect ) == 0 )`
`313`	`313`	`{`
`314`		`- // couldn't use sdo_filter earlier`
`315`		`- if ( !mSource->mHasSpatialIndex )`
	`314`	`+ // even if we could use sdo_filter earlier, we still need to double-check the results`
	`315`	`+ // as sdo_filter can return results outside the filter (it's only a first-pass`
	`316`	`+ // filtering operation!)`
	`317`	`+`
	`318`	`+ // only want features which intersect with bbox`
	`319`	`+ if ( !feature.geometry().boundingBox().intersects( mFilterRect ) )`
`316`	`320`	`{`
`317`		`- // only intersect with bbox`
`318`		`- if ( !feature.geometry().boundingBox().intersects( mFilterRect ) )`
`319`		`- {`
`320`		`- // skip feature that don't intersect with our rectangle`
`321`		`- QgsDebugMsg( QStringLiteral( "no bbox intersect" ) );`
`322`		`- continue;`
`323`		`- }`
	`321`	`+ // skip feature that don't intersect with our rectangle`
	`322`	`+ QgsDebugMsgLevel( QStringLiteral( "no bbox intersect" ), 4 );`
	`323`	`+ continue;`
`324`	`324`	`}`
`325`	`325`	`}`
`326`	`326`	`else if ( !mConnection->hasSpatial() \|\| !mSource->mHasSpatialIndex )`
`@@ -329,7 +329,7 @@ bool QgsOracleFeatureIterator::fetchFeature( QgsFeature &feature )`
`329`	`329`	`if ( !feature.geometry().intersects( mFilterRect ) )`
`330`	`330`	`{`
`331`	`331`	`// skip feature that don't intersect with our rectangle`
`332`		`- QgsDebugMsg( QStringLiteral( "no exact intersect" ) );`
	`332`	`+ QgsDebugMsgLevel( QStringLiteral( "no exact intersect" ), 4 );`
`333`	`333`	`continue;`
`334`	`334`	`}`
`335`	`335`	`}`