QgsFeatureRequest::NoGeometry is optional for non-geometry-layers

src/core/qgsfeaturerequest.h

@@ -59,7 +59,7 @@ class CORE_EXPORT QgsFeatureRequest
     enum Flag
     {
       NoFlags            = 0,
-      NoGeometry         = 1,  //!< Do not fetch geometry
+      NoGeometry         = 1,  //!< Geometry is not required. It may still be returned if e.g. required for a filter condition.
       SubsetOfAttributes = 2,  //!< Fetch only a subset of attributes (setSubsetOfAttributes sets this flag)
       ExactIntersect     = 4   //!< Use exact geometry intersection (slower) instead of bounding boxes
     };
@@ -68,7 +68,7 @@ class CORE_EXPORT QgsFeatureRequest
     enum FilterType
     {
       FilterNone,       //!< No filter is applied
-      FilterRect,       //!< Filter using a rectangle
+      FilterRect,       //!< Filter using a rectangle, no need to set NoGeometry
       FilterFid,        //!< Filter using feature ID
       FilterExpression, //!< Filter using expression
       FilterFids        //!< Filter using feature ID's
@@ -117,8 +117,8 @@ class CORE_EXPORT QgsFeatureRequest
 
     //! Set a subset of attributes by names that will be fetched
     QgsFeatureRequest& setSubsetOfAttributes( const QStringList& attrNames, const QgsFields& fields );
-    
-    /** 
+
+    /**
      * Check if a feature is accepted by this requests filter
      *
      * @param feature  The feature which will be tested

src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp

@@ -36,7 +36,7 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
 
   // Does the layer have geometry - will revise later to determine if we actually need to
   // load it.
-  mLoadGeometry = P->mGeomRep != QgsDelimitedTextProvider::GeomNone;
+  bool hasGeometry = P->mGeomRep != QgsDelimitedTextProvider::GeomNone;
 
   // Does the layer have an explicit or implicit subset (implicit subset is if we have geometry which can
   // be invalid)
@@ -59,8 +59,7 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
   // requesting no geometry? Have preserved current logic of ignoring spatial filter
   // if not requesting geometry.
 
-  else if ( request.filterType() == QgsFeatureRequest::FilterRect && mLoadGeometry
-            && !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
+  else if ( request.filterType() == QgsFeatureRequest::FilterRect && hasGeometry )
   {
     QgsDebugMsg( "Configuring for rectangle select" );
     mTestGeometry = true;
@@ -114,14 +113,23 @@ QgsDelimitedTextFeatureIterator::QgsDelimitedTextFeatureIterator( QgsDelimitedTe
     QgsDebugMsg( "File will be scanned for desired features" );
   }
 
-  // If the request does not require geometry, can we avoid loading it?
-  // We need it if we are testing geometry (ie spatial filter), or
-  // if testing the subset expression, and it uses geometry.
-  if ( mRequest.flags() & QgsFeatureRequest::NoGeometry &&
-       ! mTestGeometry &&
-       !( mTestSubset && P->mSubsetExpression->needsGeometry() ) )
+  // If the layer has geometry, do we really need to load it?
+  // We need it if it is asked for explicitly in the request,
+  // if we are testing geometry (ie spatial filter), or
+  // if testing the subset expression.
+  if ( hasGeometry
+       && (
+         !( mRequest.flags() & QgsFeatureRequest::NoGeometry )
+         || mTestGeometry
+         || ( mTestSubset && P->mSubsetExpression->needsGeometry() )
+       )
+     )
   {
-    QgsDebugMsg( "Feature geometries not required" );
+    mLoadGeometry = true;
+  }
+  else
+  {
+    QgsDebugMsgLevel( "Feature geometries not required", 4 );
     mLoadGeometry = false;
   }
 

src/providers/mssql/qgsmssqlfeatureiterator.cpp

@@ -122,7 +122,7 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
     ++fieldCount;
   }
   // get geometry col
-  if ( request.flags() != QgsFeatureRequest::NoGeometry && !mProvider->mGeometryColName.isEmpty() )
+  if ( !( request.flags() & QgsFeatureRequest::NoGeometry ) && !mProvider->mGeometryColName.isEmpty() )
   {
     if ( fieldCount != 0 )
       mStatement += ",";

src/providers/ogr/qgsogrfeatureiterator.cpp

@@ -89,9 +89,9 @@ QgsOgrFeatureIterator::~QgsOgrFeatureIterator()
 
 void QgsOgrFeatureIterator::ensureRelevantFields()
 {
-  bool needGeom = ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) || !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
+  mFetchGeometry = ( mRequest.filterType() == QgsFeatureRequest::FilterRect ) || !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
   QgsAttributeList attrs = ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) ? mRequest.subsetOfAttributes() : P->attributeIndexes();
-  P->setRelevantFields( needGeom, attrs );
+  P->setRelevantFields( mFetchGeometry, attrs );
   P->mRelevantFieldsForNextFeature = true;
 }
 
@@ -225,10 +225,9 @@ bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature )
   feature.initAttributes( P->fields().count() );
   feature.setFields( &P->mAttributeFields ); // allow name-based attribute lookups
 
-  bool fetchGeom    = !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
   bool useIntersect = mRequest.flags() & QgsFeatureRequest::ExactIntersect;
   bool geometryTypeFilter = P->mOgrGeometryTypeFilter != wkbUnknown;
-  if ( fetchGeom || useIntersect || geometryTypeFilter )
+  if ( mFetchGeometry || useIntersect || geometryTypeFilter )
   {
     OGRGeometryH geom = OGR_F_GetGeometryRef( fet );
 
@@ -248,7 +247,7 @@ bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature )
     }
   }
 
-  if ( !fetchGeom )
+  if ( !mFetchGeometry )
   {
     feature.setGeometry( 0 );
   }

src/providers/ogr/qgsogrfeatureiterator.h

@@ -53,6 +53,9 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIterator
     OGRLayerH ogrLayer;
 
     bool mSubsetStringSet;
+
+    //! Set to true, if geometry is in the requested columns
+    bool mFetchGeometry;
 };
 
 

src/providers/postgres/qgspostgresfeatureiterator.cpp

@@ -149,7 +149,7 @@ bool QgsPostgresFeatureIterator::fetchFeature( QgsFeature& feature )
   }
 
   // Now return the next feature from the queue
-  if ( mRequest.flags() & QgsFeatureRequest::NoGeometry )
+  if ( !mFetchGeometry )
   {
     feature.setGeometryAndOwnership( 0, 0 );
   }
@@ -264,18 +264,13 @@ QString QgsPostgresFeatureIterator::whereClauseRect()
 
 bool QgsPostgresFeatureIterator::declareCursor( const QString& whereClause )
 {
-  bool fetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
-  if ( fetchGeometry && P->mGeometryColumn.isNull() )
-  {
-    QgsMessageLog::logMessage( QObject::tr( "Trying to fetch geometry on a layer without geometry." ), QObject::tr( "PostgreSQL" ) );
-    return false;
-  }
+  mFetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) && !P->mGeometryColumn.isNull();
 
   try
   {
     QString query = "SELECT ", delim = "";
 
-    if ( fetchGeometry )
+    if ( mFetchGeometry )
     {
       query += QString( "%1(%2%3,'%4')" )
                .arg( P->mConnectionRO->majorVersion() < 2 ? "asbinary" : "st_asbinary" )
@@ -356,7 +351,7 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
 
     int col = 0;
 
-    if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
+    if ( mFetchGeometry )
     {
       int returnedLength = ::PQgetlength( queryResult.result(), row, col );
       if ( returnedLength > 0 )

src/providers/postgres/qgspostgresfeatureiterator.h

@@ -58,9 +58,12 @@ class QgsPostgresFeatureIterator : public QgsAbstractFeatureIterator
     //! Maximal size of the feature queue
     int mFeatureQueueSize;
 
-    //!< Number of retrieved features
+    //! Number of retrieved features
     int mFetched;
 
+    //! Set to true, if geometry is in the requested columns
+    bool mFetchGeometry;
+
     static const int sFeatureQueueSize;
 
 };

src/providers/spatialite/qgsspatialitefeatureiterator.cpp

@@ -144,12 +144,6 @@ bool QgsSpatiaLiteFeatureIterator::close()
 
 bool QgsSpatiaLiteFeatureIterator::prepareStatement( QString whereClause )
 {
-  if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) && P->mGeometryColumn.isNull() )
-  {
-    QgsMessageLog::logMessage( QObject::tr( "Trying to fetch geometry on a layer without geometry." ), QObject::tr( "SpatiaLite" ) );
-    return false;
-  }
-
   try
   {
     QString sql = QString( "SELECT %1" ).arg( quotedPrimaryKey() );
@@ -174,7 +168,7 @@ bool QgsSpatiaLiteFeatureIterator::prepareStatement( QString whereClause )
       }
     }
 
-    if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
+    if ( mFetchGeometry )
     {
       sql += QString( ", AsBinary(%1)" ).arg( P->quotedIdentifier( P->mGeometryColumn ) );
       mGeomColIdx = colIdx;
@@ -284,7 +278,7 @@ QString QgsSpatiaLiteFeatureIterator::fieldName( const QgsField& fld )
 
 bool QgsSpatiaLiteFeatureIterator::getFeature( sqlite3_stmt *stmt, QgsFeature &feature )
 {
-  bool fetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
+  mFetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
   bool subsetAttributes = mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes;
 
   int ret = sqlite3_step( stmt );
@@ -301,7 +295,7 @@ bool QgsSpatiaLiteFeatureIterator::getFeature( sqlite3_stmt *stmt, QgsFeature &f
   }
 
   // one valid row has been fetched from the result set
-  if ( !fetchGeometry )
+  if ( !mFetchGeometry )
   {
     // no geometry was required
     feature.setGeometryAndOwnership( 0, 0 );
@@ -321,7 +315,7 @@ bool QgsSpatiaLiteFeatureIterator::getFeature( sqlite3_stmt *stmt, QgsFeature &f
       QgsDebugMsgLevel( QString( "fid=%1" ).arg( fid ), 3 );
       feature.setFeatureId( fid );
     }
-    else if ( fetchGeometry && ic == mGeomColIdx )
+    else if ( mFetchGeometry && ic == mGeomColIdx )
     {
       getFeatureGeometry( stmt, ic, feature );
     }

src/providers/spatialite/qgsspatialitefeatureiterator.h

@@ -63,6 +63,9 @@ class QgsSpatiaLiteFeatureIterator : public QgsAbstractFeatureIterator
     /** geometry column index used when fetching geometry */
     int mGeomColIdx;
 
+    //! Set to true, if geometry is in the requested columns
+    bool mFetchGeometry;
+
 };
 
 #endif // QGSSPATIALITEFEATUREITERATOR_H

src/providers/sqlanywhere/qgssqlanywherefeatureiterator.cpp

@@ -106,7 +106,7 @@ bool QgsSqlAnywhereFeatureIterator::nextFeature( QgsFeature& feature, SqlAnyStat
 {
   feature.setValid( false );
 
-  bool fetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry );
+  mFetchGeometry = !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) && !P->mGeometryColumn.isNull();
   bool subsetAttributes = mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes;
 
   if ( mClosed )
@@ -130,7 +130,7 @@ bool QgsSqlAnywhereFeatureIterator::nextFeature( QgsFeature& feature, SqlAnyStat
   if ( !ok )
     return false;
 
-  if ( !fetchGeometry )
+  if ( !mFetchGeometry )
     feature.setGeometryAndOwnership( 0, 0 );
 
   int numAttributes = P->fields().count(); // also used later for sanity check
@@ -151,7 +151,7 @@ bool QgsSqlAnywhereFeatureIterator::nextFeature( QgsFeature& feature, SqlAnyStat
       QgsDebugMsgLevel( QString( "pid=%1" ).arg( id ), 3 );
       feature.setFeatureId( id );
     }
-    else if ( i == 1 && fetchGeometry )
+    else if ( i == 1 && mFetchGeometry )
     {
       // second column contains QKB geometry value
       ok = stmt->getColumn( i, &geom );
@@ -174,7 +174,7 @@ bool QgsSqlAnywhereFeatureIterator::nextFeature( QgsFeature& feature, SqlAnyStat
 
       // Sanity check before setting the attribute value
       if ( colidx - 1 == i  // First column is always pk, so colidx should be at least 1 behind
-           || ( colidx - 1 == i - 1 && fetchGeometry ) // if fetchGeometry is true, colidx should be 2 behind
+           || ( colidx - 1 == i - 1 && mFetchGeometry ) // if fetchGeometry is true, colidx should be 2 behind
            || attrIndex >= numAttributes ) // index should always be less than the count
       {
         SaDebugMsg( QString( "Error retrieving feature column %1 with attribute index %2" ).arg( i ).arg( attrIndex ) );
@@ -220,9 +220,6 @@ bool
 QgsSqlAnywhereFeatureIterator::prepareStatement( QString whereClause )
 // adapted from QgsSpatialLiteFeatureIterator::prepareStatement
 {
-  if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) && P->mGeometryColumn.isNull() )
-    return false;
-
   if ( !P->ensureConnRO() )
   {
     SaDebugMsg( "No read-only database connection." );
@@ -232,7 +229,7 @@ QgsSqlAnywhereFeatureIterator::prepareStatement( QString whereClause )
   QString sql = QString( "SELECT %1" ).arg( quotedPrimaryKey() ); // Column 0 is primary key
 
   // Column 1 is geometry, if applicable
-  if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
+  if ( mFetchGeometry )
   {
     sql += QString( ", %1 .ST_AsBinary('WKB(Version=1.1;endian=%2)') " )
            .arg( P->mGeometryColumn )

src/providers/sqlanywhere/qgssqlanywherefeatureiterator.h

@@ -61,6 +61,9 @@ class QgsSqlAnywhereFeatureIterator : public QgsAbstractFeatureIterator
 
     QgsRectangle mStmtRect;
 
+    //! Set to true, if geometry is in the requested columns
+    bool mFetchGeometry;
+
 };
 
 #endif // QGSSQLANYWHEREFEATUREITERATOR_H