[ogr] Fix absence of proper feature ID when adding features to CSV, O…

…DS, and XLSX datasets

src/core/providers/ogr/qgsogrprovider.cpp

@@ -1669,7 +1669,7 @@ QString QgsOgrProvider::jsonStringValue( const QVariant &value ) const
   return stringValue;
 }
 
-bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags )
+bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags, QgsFeatureId incrementalFeatureId )
 {
   bool returnValue = true;
   QgsOgrFeatureDefn &featureDefinition = mOgrLayer->GetLayerDefn();
@@ -1860,16 +1860,23 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags )
     pushError( tr( "OGR error creating feature %1: %2" ).arg( f.id() ).arg( CPLGetLastErrorMsg() ) );
     returnValue = false;
   }
-  else if ( !( flags & QgsFeatureSink::FastInsert ) )
+  else
   {
-    QgsFeatureId id = static_cast<QgsFeatureId>( OGR_F_GetFID( feature.get() ) );
-    if ( id >= 0 )
+    if ( !( flags & QgsFeatureSink::FastInsert ) )
     {
-      f.setId( id );
+      QgsFeatureId id = static_cast<QgsFeatureId>( OGR_F_GetFID( feature.get() ) );
+      if ( id >= 0 )
+      {
+        f.setId( id );
 
-      if ( mFirstFieldIsFid && attributes.count() > 0 )
+        if ( mFirstFieldIsFid && attributes.count() > 0 )
+        {
+          f.setAttribute( 0, id );
+        }
+      }
+      else if ( incrementalFeatureId >= 0 )
       {
-        f.setAttribute( 0, id );
+        f.setId( incrementalFeatureId );
       }
     }
   }
@@ -1890,13 +1897,33 @@ bool QgsOgrProvider::addFeatures( QgsFeatureList &flist, Flags flags )
 
   const bool inTransaction = startTransaction();
 
+  QgsFeatureId incrementalFeatureId = -1;
+  OGRGeometryH filter;
+  if ( !( flags & QgsFeatureSink::FastInsert ) &&
+       ( mGDALDriverName == QLatin1String( "CSV" ) || mGDALDriverName == QLatin1String( "XLSX" ) || mGDALDriverName == QLatin1String( "ODS" ) ) )
+  {
+    filter = mOgrLayer->GetSpatialFilter();
+    if ( filter )
+    {
+      filter = OGR_G_Clone( filter );
+      mOgrLayer->SetSpatialFilter( nullptr );
+    }
+
+    incrementalFeatureId = static_cast< QgsFeatureId >( mOgrLayer->GetFeatureCount() ) + 1;
+
+    if ( filter )
+      mOgrLayer->SetSpatialFilter( filter );
+  }
+
   bool returnvalue = true;
   for ( QgsFeatureList::iterator it = flist.begin(); it != flist.end(); ++it )
   {
-    if ( !addFeaturePrivate( *it, flags ) )
+    if ( !addFeaturePrivate( *it, flags, incrementalFeatureId ) )
     {
       returnvalue = false;
     }
+    if ( incrementalFeatureId >= 0 )
+      incrementalFeatureId++;
   }
 
   if ( inTransaction )

src/core/providers/ogr/qgsogrprovider.h

@@ -310,10 +310,12 @@ class QgsOgrProvider final: public QgsVectorDataProvider
 
     mutable QStringList mSubLayerList;
 
-    //! converts \a value from json QVariant to QString
+    //! Converts \a value from json QVariant to QString
     QString jsonStringValue( const QVariant &value ) const;
 
-    bool addFeaturePrivate( QgsFeature &f, QgsFeatureSink::Flags flags );
+    //! The \a incrementalFeatureId will generally be -1, except for a few OGR drivers where QGIS will pass on a value when OGR doesn't set it
+    bool addFeaturePrivate( QgsFeature &f, QgsFeatureSink::Flags flags, QgsFeatureId incrementalFeatureId = -1 );
+
     //! Deletes one feature
     bool deleteFeature( QgsFeatureId id );
 

tests/src/core/testqgsogrprovider.cpp

@@ -49,6 +49,7 @@ class TestQgsOgrProvider : public QObject
     void decodeUri();
     void encodeUri();
     void testThread();
+    void testCsvFeatureAddition();
 
   private:
     QString mTestDataDir;
@@ -352,6 +353,37 @@ void TestQgsOgrProvider::testThread()
 
 }
 
+void TestQgsOgrProvider::testCsvFeatureAddition()
+{
+  QString csvFilename = QDir::tempPath() + "/csvfeatureadditiontest.csv";
+  QFile csvFile( csvFilename );
+  if ( csvFile.open( QIODevice::WriteOnly | QIODevice::Append ) )
+  {
+    QTextStream textStream( &csvFile );
+    textStream << QLatin1String( "col1,col2\n1,2\n" );
+    csvFile.close();
+  }
+
+  QgsVectorLayer *csvLayer = new QgsVectorLayer( csvFilename, QStringLiteral( "csv" ) );
+  QVERIFY( csvLayer->isValid() );
+  QCOMPARE( csvLayer->featureCount(), 1 );
+
+
+  QgsFeature feature( csvLayer->fields() );
+  feature.setAttribute( 0, 1 );
+  feature.setAttribute( 1, 1 );
+  feature.setAttribute( 2, QLatin1String( "csv" ) );
+
+  QgsFeatureList features;
+  features << feature << feature;
+  csvLayer->dataProvider()->addFeatures( features );
+  QCOMPARE( features.at( 0 ).id(), 2 );
+  QCOMPARE( features.at( 1 ).id(), 3 );
+
+  delete csvLayer;
+  QFile::remove( csvFilename );
+}
+
 
 QGSTEST_MAIN( TestQgsOgrProvider )
 #include "testqgsogrprovider.moc"