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

qgis · Sep 21, 2017 · a51ef7d · a51ef7d
1 parent e09946a
commit a51ef7d
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 18 deletions.
diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp
@@ -172,23 +172,28 @@ 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 (
+      while (( fet = OGR_L_GetNextFeature( ogrLayer ) ) )
+      {
+        if (
 #if defined(GDAL_VERSION_NUM) && GDAL_VERSION_NUM >= 2000000
-        OGR_F_GetFieldAsInteger64
+          OGR_F_GetFieldAsInteger64
 #else
-        OGR_F_GetFieldAsInteger
+          OGR_F_GetFieldAsInteger
 #endif
-        ( fet, 0 ) == FID_TO_NUMBER( id ) )
-      {
-        break;
+          ( fet, lastField ) == FID_TO_NUMBER( id ) )
+        {
+          break;
+        }
+        OGR_F_Destroy( fet );
       }
-      OGR_F_Destroy( fet );
     }
   }
   else

diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -3453,8 +3453,8 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, OGRDataSourceH
   }
   else
   {
-    QByteArray sqlPart1 = "SELECT ";
-    QByteArray sqlPart3 = "* FROM " + quotedIdentifier( layerName, ogrDriverName )
+    QByteArray sqlPart1 = "SELECT *";
+    QByteArray sqlPart3 = " FROM " + quotedIdentifier( layerName, ogrDriverName )
                           + " WHERE " + encoding->fromUnicode( subsetString );
 
     origFidAddAttempted = true;
@@ -3466,15 +3466,15 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, OGRDataSourceH
       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 );
     }
@@ -3488,13 +3488,14 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, OGRDataSourceH
     }
   }
 
-  // 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;
     }
   }

diff --git a/tests/src/python/test_provider_ogr_sqlite.py b/tests/src/python/test_provider_ogr_sqlite.py
@@ -175,7 +175,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")
@@ -186,7 +186,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__':