Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #42189 from nirvn/ogr_csv_xlsx_ods
[ogr] Fix absence of proper feature ID when adding features to CSV, ODS, and XLSX datasets
  • Loading branch information
rouault authored and github-actions[bot] committed Mar 11, 2021
1 parent acc0eb4 commit 935e0c6
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 10 deletions.
43 changes: 35 additions & 8 deletions src/core/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -1693,7 +1693,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();
Expand Down Expand Up @@ -1890,16 +1890,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 );
}
}
}
Expand All @@ -1920,13 +1927,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 )
Expand Down
6 changes: 4 additions & 2 deletions src/core/providers/ogr/qgsogrprovider.h
Expand Up @@ -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 );

Expand Down
32 changes: 32 additions & 0 deletions tests/src/core/testqgsogrprovider.cpp
Expand Up @@ -49,6 +49,7 @@ class TestQgsOgrProvider : public QObject
void decodeUri();
void encodeUri();
void testThread();
void testCsvFeatureAddition();

private:
QString mTestDataDir;
Expand Down Expand Up @@ -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"

0 comments on commit 935e0c6

Please sign in to comment.