[OGR] Add orig_ogc_fid as last field to avoid changing field order

Author: manisandro

src/providers/ogr/qgsogrfeatureiterator.cpp

@@ -196,17 +196,22 @@ bool QgsOgrFeatureIterator::nextFeatureFilterExpression( QgsFeature &f )
 bool QgsOgrFeatureIterator::fetchFeatureWithId( QgsFeatureId id, QgsFeature &feature ) const
 {
   feature.setValid( false );
-  OGRFeatureH fet;
+  OGRFeatureH fet = 0;
   if ( mOrigFidAdded )
   {
     OGR_L_ResetReading( ogrLayer );
-    while ( ( fet = OGR_L_GetNextFeature( ogrLayer ) ) )
+    OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( ogrLayer );
+    int lastField = OGR_FD_GetFieldCount( fdef ) - 1;
+    if ( lastField >= 0 )
     {
-      if ( OGR_F_GetFieldAsInteger64( fet, 0 ) == id )
+      while ( ( fet = OGR_L_GetNextFeature( ogrLayer ) ) )
       {
-        break;
+        if ( OGR_F_GetFieldAsInteger64( fet, lastField ) == id )
+        {
+          break;
+        }
+        OGR_F_Destroy( fet );
       }
-      OGR_F_Destroy( fet );
     }
   }
   else

src/providers/ogr/qgsogrprovider.cpp

@@ -3547,8 +3547,8 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds
   }
   else
   {
-    QByteArray sqlPart1 = "SELECT ";
-    QByteArray sqlPart3 = "* FROM " + quotedIdentifier( layerName, mGDALDriverName )
+    QByteArray sqlPart1 = "SELECT *";
+    QByteArray sqlPart3 = " FROM " + quotedIdentifier( layerName, mGDALDriverName )
                           + " WHERE " + encoding->fromUnicode( subsetString );
 
     origFidAddAttempted = true;
@@ -3560,15 +3560,15 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds
       fidColumn = "FID";
     }
 
-    QByteArray sql = sqlPart1 + fidColumn + " as orig_ogc_fid, " + sqlPart3;
+    QByteArray sql = sqlPart1 + ", " + fidColumn + " as orig_ogc_fid" + sqlPart3;
     QgsDebugMsg( QString( "SQL: %1" ).arg( encoding->toUnicode( sql ) ) );
     subsetLayer = GDALDatasetExecuteSQL( ds, sql.constData(), nullptr, nullptr );
 
     // See https://lists.osgeo.org/pipermail/qgis-developer/2017-September/049802.html
     // If execute SQL fails because it did not find the fidColumn, retry with hardcoded FID
     if ( !subsetLayer )
     {
-      QByteArray sql = sqlPart1 + "FID as orig_ogc_fid, " + sqlPart3;
+      QByteArray sql = sqlPart1 + ", " + "FID as orig_ogc_fid" + sqlPart3;
       QgsDebugMsg( QString( "SQL: %1" ).arg( encoding->toUnicode( sql ) ) );
       subsetLayer = GDALDatasetExecuteSQL( ds, sql.constData(), nullptr, nullptr );
     }
@@ -3582,13 +3582,14 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds
     }
   }
 
-  // Check if first column is orig_ogc_fid
+  // Check if last column is orig_ogc_fid
   if ( origFidAddAttempted && subsetLayer )
   {
     OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( subsetLayer );
-    if ( OGR_FD_GetFieldCount( fdef ) > 0 )
+    int fieldCount = OGR_FD_GetFieldCount( fdef );
+    if ( fieldCount > 0 )
     {
-      OGRFieldDefnH fldDef = OGR_FD_GetFieldDefn( fdef, 0 );
+      OGRFieldDefnH fldDef = OGR_FD_GetFieldDefn( fdef, fieldCount - 1 );
       origFidAdded = qstrcmp( OGR_Fld_GetNameRef( fldDef ), "orig_ogc_fid" ) == 0;
     }
   }

tests/src/python/test_provider_ogr_sqlite.py

@@ -245,7 +245,7 @@ def testSubsetStringFids(self):
 
         vl = QgsVectorLayer(tmpfile + "|subset=type=2", 'test', 'ogr')
         self.assertTrue(vl.isValid())
-        self.assertTrue(vl.fields().at(0).name() == "orig_ogc_fid")
+        self.assertTrue(vl.fields().at(vl.fields().count() - 1).name() == "orig_ogc_fid")
 
         req = QgsFeatureRequest()
         req.setFilterExpression("value=16")
@@ -256,7 +256,7 @@ def testSubsetStringFids(self):
 
         # Check that subset string is correctly set on reload
         vl.reload()
-        self.assertTrue(vl.fields().at(0).name() == "orig_ogc_fid")
+        self.assertTrue(vl.fields().at(vl.fields().count() - 1).name() == "orig_ogc_fid")
 
 
 if __name__ == '__main__':