Skip to content
Permalink
Browse files

Merge pull request #42198 from qgis/backport-42189-to-release-3_18

[Backport release-3_18] [ogr] Fix absence of proper feature ID when adding features to CSV, ODS, and XLSX datasets
  • Loading branch information
nirvn committed Mar 16, 2021
2 parents b2b7956 + 1802269 commit 0b36c7b20ec40f7fadb2c2bf22032ec553524f20
@@ -1696,7 +1696,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();
@@ -1893,16 +1893,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 );
}
}
}
@@ -1923,13 +1930,34 @@ bool QgsOgrProvider::addFeatures( QgsFeatureList &flist, Flags flags )

const bool inTransaction = startTransaction();

QgsFeatureId incrementalFeatureId = -1;
if ( !( flags & QgsFeatureSink::FastInsert ) &&
( mGDALDriverName == QLatin1String( "CSV" ) || mGDALDriverName == QLatin1String( "XLSX" ) || mGDALDriverName == QLatin1String( "ODS" ) ) )
{
QMutex *mutex = nullptr;
OGRLayerH layer = mOgrOrigLayer->getHandleAndMutex( mutex );
{
QMutexLocker locker( mutex );

if ( !mSubsetString.isEmpty() )
OGR_L_SetAttributeFilter( layer, nullptr );

incrementalFeatureId = static_cast< QgsFeatureId >( OGR_L_GetFeatureCount( layer, false ) ) + 1;

if ( !mSubsetString.isEmpty() )
OGR_L_SetAttributeFilter( layer, textEncoding()->fromUnicode( mSubsetString ).constData() );
}
}

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 )
@@ -309,10 +309,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 );

@@ -49,6 +49,7 @@ class TestQgsOgrProvider : public QObject
void decodeUri();
void encodeUri();
void testThread();
void testCsvFeatureAddition();

private:
QString mTestDataDir;
@@ -352,6 +353,48 @@ void TestQgsOgrProvider::testThread()

}

void TestQgsOgrProvider::testCsvFeatureAddition()
{
QString csvFilename = QDir::tempPath() + "/csvfeatureadditiontest.csv";
QFile csvFile( csvFilename );
if ( csvFile.open( QIODevice::WriteOnly | QIODevice::Truncate ) )
{
QTextStream textStream( &csvFile );
textStream << QLatin1String( "col1,col2,col3\n0,0,\"csv0\"\n" );
csvFile.close();
}

QgsVectorLayer *csvLayer = new QgsVectorLayer( csvFilename, QStringLiteral( "csv" ) );
QVERIFY( csvLayer->isValid() );
QCOMPARE( csvLayer->featureCount(), 1 );

QgsFeature f1( csvLayer->fields() );
f1.setAttribute( 0, 1 );
f1.setAttribute( 1, 1 );
f1.setAttribute( 2, QLatin1String( "csv1" ) );
QgsFeature f2( csvLayer->fields() );
f2.setAttribute( 0, 2 );
f2.setAttribute( 1, 2 );
f2.setAttribute( 2, QLatin1String( "csv2" ) );

QgsFeatureList features;
features << f1 << f2;
csvLayer->dataProvider()->addFeatures( features );
QCOMPARE( features.at( 0 ).id(), 2 );
QCOMPARE( features.at( 1 ).id(), 3 );

csvLayer->setSubsetString( QStringLiteral( "col1 = '2'" ) );
QCOMPARE( csvLayer->featureCount(), 1 );

features.clear();
features << f1;
csvLayer->dataProvider()->addFeatures( features );
QCOMPARE( features.at( 0 ).id(), 4 );

delete csvLayer;
QFile::remove( csvFilename );
}


QGSTEST_MAIN( TestQgsOgrProvider )
#include "testqgsogrprovider.moc"

0 comments on commit 0b36c7b

Please sign in to comment.