Skip to content

Commit

Permalink
[processing] When writing a layer to a temporary GPKG file for
Browse files Browse the repository at this point in the history
compatibilty with provider's supported formats, rename the
existing FID column if we can't write the FID values into
the GPKG

Eg when the source layer has string/duplicate fids, we warn
the user and then automatically move the existing fids to
a "OLD_FID" field
  • Loading branch information
nyalldawson committed Jul 9, 2024
1 parent 3ab7ace commit d154799
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 9 deletions.
26 changes: 21 additions & 5 deletions src/core/processing/qgsprocessingutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ QString QgsProcessingUtils::formatHelpMapAsHtml( const QVariantMap &map, const Q
}

QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool selectedFeaturesOnly, const QString &baseName, const QStringList &compatibleFormats, const QString &preferredFormat, QgsProcessingContext &context, QgsProcessingFeedback *feedback, QString *layerName,
long long featureLimit, const QString &filterExpression )
long long featureLimit, const QString &filterExpression, bool renameFid )
{
bool requiresTranslation = false;

Expand Down Expand Up @@ -1412,7 +1412,14 @@ QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool select
QgsVectorFileWriter::SaveVectorOptions saveOptions;
saveOptions.fileEncoding = context.defaultEncoding();
saveOptions.driverName = QgsVectorFileWriter::driverForExtension( preferredFormat );
std::unique_ptr< QgsVectorFileWriter > writer( QgsVectorFileWriter::create( temp, vl->fields(), vl->wkbType(), vl->crs(), context.transformContext(), saveOptions ) );
QgsFields fields = vl->fields();
if ( renameFid )
{
const int fidIndex = fields.lookupField( QStringLiteral( "fid" ) );
if ( fidIndex >= 0 )
fields.rename( fidIndex, QStringLiteral( "OLD_FID" ) );
}
std::unique_ptr< QgsVectorFileWriter > writer( QgsVectorFileWriter::create( temp, fields, vl->wkbType(), vl->crs(), context.transformContext(), saveOptions ) );
QgsFeature f;
QgsFeatureIterator it;
QgsFeatureRequest request;
Expand All @@ -1439,10 +1446,19 @@ QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool select

if ( !writer->addFeature( f, QgsFeatureSink::FastInsert ) && feedback )
{
const QString errorMessage = writer->errorMessage();
if ( !renameFid && saveOptions.driverName == QLatin1String( "GPKG" ) && errorMessage.contains( "fid", Qt::CaseInsensitive ) )
{
// try again, dropping the FID field
feedback->reportError( QObject::tr( "Cannot store existing FID values in temporary GeoPackage layer, these will be moved to \"OLD_FID\" instead." ), false );
return convertToCompatibleFormatInternal( vl, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, layerName,
featureLimit, filterExpression, true );
}

QString errorText;
if ( errorCounter++ < maxErrors )
{
errorText = QObject::tr( "Error writing feature # %1 to output layer: %2" ).arg( QString::number( f.id() ), writer->errorMessage() );
errorText = QObject::tr( "Error writing feature # %1 to output layer: %2" ).arg( QString::number( f.id() ), errorMessage );

feedback->reportError( errorText );
}
Expand All @@ -1462,13 +1478,13 @@ QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool select

QString QgsProcessingUtils::convertToCompatibleFormat( const QgsVectorLayer *vl, bool selectedFeaturesOnly, const QString &baseName, const QStringList &compatibleFormats, const QString &preferredFormat, QgsProcessingContext &context, QgsProcessingFeedback *feedback, long long featureLimit, const QString &filterExpression )
{
return convertToCompatibleFormatInternal( vl, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, nullptr, featureLimit, filterExpression );
return convertToCompatibleFormatInternal( vl, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, nullptr, featureLimit, filterExpression, false );
}

QString QgsProcessingUtils::convertToCompatibleFormatAndLayerName( const QgsVectorLayer *layer, bool selectedFeaturesOnly, const QString &baseName, const QStringList &compatibleFormats, const QString &preferredFormat, QgsProcessingContext &context, QgsProcessingFeedback *feedback, QString &layerName, long long featureLimit, const QString &filterExpression )
{
layerName.clear();
return convertToCompatibleFormatInternal( layer, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, &layerName, featureLimit, filterExpression );
return convertToCompatibleFormatInternal( layer, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, &layerName, featureLimit, filterExpression, false );
}

QgsFields QgsProcessingUtils::combineFields( const QgsFields &fieldsA, const QgsFields &fieldsB, const QString &fieldsBPrefix )
Expand Down
15 changes: 11 additions & 4 deletions tests/src/analysis/testqgsprocessing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12672,11 +12672,18 @@ void TestQgsProcessing::convertCompatibleDuplicateFids()
QVariantMap params;
params.insert( QStringLiteral( "source" ), QgsProcessingFeatureSourceDefinition( layer->id(), false ) );

QgsProcessingParameters::parameterAsCompatibleSourceLayerPath( def.get(), params, context, QStringList() << "gpkg", QString( "gpkg" ), &feedback );
const QString temporaryFile = QgsProcessingParameters::parameterAsCompatibleSourceLayerPath( def.get(), params, context, QStringList() << "gpkg", QString( "gpkg" ), &feedback );
const QStringList logs( feedback.textLog().split( '\n', Qt::SplitBehaviorFlags::SkipEmptyParts ) );
QCOMPARE( logs.size(), 11 );
QVERIFY( logs.first().startsWith( QLatin1String( "Error writing feature # 2" ) ) );
QVERIFY( logs.last().startsWith( QLatin1String( "There were 11 errors writing features" ) ) );
QCOMPARE( logs.size(), 1 );
QCOMPARE( logs.at( 0 ), QStringLiteral( "Cannot store existing FID values in temporary GeoPackage layer, these will be moved to \"OLD_FID\" instead." ) );

QgsVectorLayer vl( temporaryFile );
QVERIFY( vl.isValid() );

QCOMPARE( vl.featureCount(), 12 );
QCOMPARE( vl.fields().at( 0 ).name(), QStringLiteral( "fid" ) );
QCOMPARE( vl.fields().at( 1 ).name(), QStringLiteral( "OLD_FID" ) );
QCOMPARE( vl.fields().at( 2 ).name(), QStringLiteral( "name" ) );
}

void TestQgsProcessing::create()
Expand Down

0 comments on commit d154799

Please sign in to comment.