Ignore FID field when merging vector layers to geopackage

qgis · Oct 31, 2018 · 8a46059 · 8a46059
1 parent b91c04a
commit 8a46059
Show file tree

Hide file tree

Showing 18 changed files with 103 additions and 35 deletions.
diff --git a/python/core/auto_generated/processing/qgsprocessingalgorithm.sip.in b/python/core/auto_generated/processing/qgsprocessingalgorithm.sip.in
@@ -611,7 +611,7 @@ Evaluates the parameter with matching ``name`` to a static boolean value.
 %End
 
     QgsFeatureSink *parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier /Out/,
-                                     const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem() ) const /Factory/;
+                                     const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(), QgsFeatureSink::SinkFlags options = 0 ) const /Factory/;
 %Docstring
 Evaluates the parameter with matching ``name`` to a feature sink.
 

diff --git a/python/core/auto_generated/processing/qgsprocessingparameters.sip.in b/python/core/auto_generated/processing/qgsprocessingparameters.sip.in
@@ -588,7 +588,7 @@ Evaluates the parameter with matching ``definition`` and ``value`` to a static b
 
     static QgsFeatureSink *parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariantMap &parameters,
                                             const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
-                                            QgsProcessingContext &context, QString &destinationIdentifier /Out/ ) /Factory/;
+                                            QgsProcessingContext &context, QString &destinationIdentifier /Out/, QgsFeatureSink::SinkFlags options = 0 ) /Factory/;
 %Docstring
 Evaluates the parameter with matching ``definition`` to a feature sink.
 
@@ -605,7 +605,7 @@ This function creates a new object and the caller takes responsibility for delet
 
     static QgsFeatureSink *parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariant &value,
                                             const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
-                                            QgsProcessingContext &context, QString &destinationIdentifier /Out/ ) /Factory/;
+                                            QgsProcessingContext &context, QString &destinationIdentifier /Out/, QgsFeatureSink::SinkFlags options = 0 ) /Factory/;
 %Docstring
 Evaluates the parameter with matching ``definition`` and ``value`` to a feature sink.
 

diff --git a/python/core/auto_generated/qgsfeaturesink.sip.in b/python/core/auto_generated/qgsfeaturesink.sip.in
@@ -22,6 +22,14 @@ An interface for objects which accept features via addFeature(s) methods.
 %End
   public:
 
+    enum SinkFlag
+    {
+
+      RegeneratePrimaryKey,
+    };
+    typedef QFlags<QgsFeatureSink::SinkFlag> SinkFlags;
+
+
     enum Flag
     {
 

diff --git a/python/core/auto_generated/qgsvectorfilewriter.sip.in b/python/core/auto_generated/qgsvectorfilewriter.sip.in
@@ -408,7 +408,8 @@ Writes a layer out to a vector file.
                          const QStringList &datasourceOptions = QStringList(),
                          const QStringList &layerOptions = QStringList(),
                          QString *newFilename = 0,
-                         QgsVectorFileWriter::SymbologyExport symbologyExport = QgsVectorFileWriter::NoSymbology
+                         QgsVectorFileWriter::SymbologyExport symbologyExport = QgsVectorFileWriter::NoSymbology,
+                         QgsFeatureSink::SinkFlags sinkFlags = 0
                        );
 
 

diff --git a/python/core/auto_generated/qgsvectorlayerexporter.sip.in b/python/core/auto_generated/qgsvectorlayerexporter.sip.in
@@ -77,7 +77,8 @@ Writes the contents of vector layer to a different datasource.
                             QgsWkbTypes::Type geometryType,
                             const QgsCoordinateReferenceSystem &crs,
                             bool overwrite = false,
-                            const QMap<QString, QVariant> &options = QMap<QString, QVariant>() );
+                            const QMap<QString, QVariant> &options = QMap<QString, QVariant>(),
+                            QgsFeatureSink::SinkFlags sinkOptions = 0 );
 %Docstring
 Constructor for QgsVectorLayerExporter.
 

diff --git a/src/analysis/processing/qgsalgorithmmergevector.cpp b/src/analysis/processing/qgsalgorithmmergevector.cpp
@@ -178,7 +178,7 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame
   }
 
   QString dest;
-  std::unique_ptr< QgsFeatureSink > sink( parameterAsSink( parameters, QStringLiteral( "OUTPUT" ), context, dest, outputFields, outputType, outputCrs ) );
+  std::unique_ptr< QgsFeatureSink > sink( parameterAsSink( parameters, QStringLiteral( "OUTPUT" ), context, dest, outputFields, outputType, outputCrs, QgsFeatureSink::RegeneratePrimaryKey ) );
   if ( !sink )
     throw QgsProcessingException( invalidSinkError( parameters, QStringLiteral( "OUTPUT" ) ) );
 

diff --git a/src/app/qgsgeometryvalidationservice.cpp b/src/app/qgsgeometryvalidationservice.cpp
@@ -348,6 +348,9 @@ void QgsGeometryValidationService::processFeature( QgsVectorLayer *layer, QgsFea
   if ( !allErrors.empty() )
     mLayerChecks[layer].singleFeatureCheckErrors.insert( fid, allErrors );
 
+  if ( !mLayerChecks[layer].singleFeatureCheckErrors.empty() )
+    layer->setAllowCommit( false );
+
   emit geometryCheckCompleted( layer, fid, allErrors );
 }
 
@@ -432,7 +435,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
   connect( futureWatcher, &QFutureWatcherBase::finished, this, [&allErrors, layer, feedbacks, futureWatcher, this]()
   {
     QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Read );
-    layer->setAllowCommit( allErrors.empty() );
+    layer->setAllowCommit( allErrors.empty() && mLayerChecks[layer].singleFeatureCheckErrors.empty() );
     errorLocker.unlock();
     qDeleteAll( feedbacks.values() );
     futureWatcher->deleteLater();

diff --git a/src/core/processing/qgsprocessingalgorithm.cpp b/src/core/processing/qgsprocessingalgorithm.cpp
@@ -586,9 +586,9 @@ bool QgsProcessingAlgorithm::parameterAsBool( const QVariantMap &parameters, con
   return QgsProcessingParameters::parameterAsBool( parameterDefinition( name ), parameters, context );
 }
 
-QgsFeatureSink *QgsProcessingAlgorithm::parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs ) const
+QgsFeatureSink *QgsProcessingAlgorithm::parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, QgsFeatureSink::SinkFlags options ) const
 {
-  return QgsProcessingParameters::parameterAsSink( parameterDefinition( name ), parameters, fields, geometryType, crs, context, destinationIdentifier );
+  return QgsProcessingParameters::parameterAsSink( parameterDefinition( name ), parameters, fields, geometryType, crs, context, destinationIdentifier, options );
 }
 
 QgsProcessingFeatureSource *QgsProcessingAlgorithm::parameterAsSource( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context ) const

diff --git a/src/core/processing/qgsprocessingalgorithm.h b/src/core/processing/qgsprocessingalgorithm.h
@@ -619,7 +619,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
      * This function creates a new object and the caller takes responsibility for deleting the returned object.
      */
     QgsFeatureSink *parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier SIP_OUT,
-                                     const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem() ) const SIP_FACTORY;
+                                     const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(), QgsFeatureSink::SinkFlags options = nullptr ) const SIP_FACTORY;
 
     /**
      * Evaluates the parameter with matching \a name to a feature source.

diff --git a/src/core/processing/qgsprocessingparameters.cpp b/src/core/processing/qgsprocessingparameters.cpp
@@ -350,18 +350,18 @@ bool QgsProcessingParameters::parameterAsBool( const QgsProcessingParameterDefin
 
 QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariantMap &parameters, const QgsFields &fields,
     QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
-    QgsProcessingContext &context, QString &destinationIdentifier )
+    QgsProcessingContext &context, QString &destinationIdentifier, QgsFeatureSink::SinkFlags options )
 {
   QVariant val;
   if ( definition )
   {
     val = parameters.value( definition->name() );
   }
 
-  return parameterAsSink( definition, val, fields, geometryType, crs, context, destinationIdentifier );
+  return parameterAsSink( definition, val, fields, geometryType, crs, context, destinationIdentifier, options );
 }
 
-QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariant &value, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, QgsProcessingContext &context, QString &destinationIdentifier )
+QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariant &value, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, QgsProcessingContext &context, QString &destinationIdentifier, QgsFeatureSink::SinkFlags options )
 {
   QVariant val = value;
 
@@ -374,6 +374,7 @@ QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingPar
     QgsProcessingOutputLayerDefinition fromVar = qvariant_cast<QgsProcessingOutputLayerDefinition>( val );
     destinationProject = fromVar.destinationProject;
     createOptions = fromVar.createOptions;
+
     val = fromVar.sink;
     destName = fromVar.destinationName;
   }
@@ -401,7 +402,7 @@ QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingPar
   if ( dest.isEmpty() )
     return nullptr;
 
-  std::unique_ptr< QgsFeatureSink > sink( QgsProcessingUtils::createFeatureSink( dest, context, fields, geometryType, crs, createOptions ) );
+  std::unique_ptr< QgsFeatureSink > sink( QgsProcessingUtils::createFeatureSink( dest, context, fields, geometryType, crs, createOptions, options ) );
   destinationIdentifier = dest;
 
   if ( destinationProject )

diff --git a/src/core/processing/qgsprocessingparameters.h b/src/core/processing/qgsprocessingparameters.h
@@ -24,6 +24,7 @@
 #include "qgsproperty.h"
 #include "qgscoordinatereferencesystem.h"
 #include "qgsfeaturesource.h"
+#include "qgsprocessingutils.h"
 #include <QMap>
 #include <limits>
 
@@ -662,7 +663,7 @@ class CORE_EXPORT QgsProcessingParameters
      */
     static QgsFeatureSink *parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariantMap &parameters,
                                             const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
-                                            QgsProcessingContext &context, QString &destinationIdentifier SIP_OUT ) SIP_FACTORY;
+                                            QgsProcessingContext &context, QString &destinationIdentifier SIP_OUT, QgsFeatureSink::SinkFlags options = nullptr ) SIP_FACTORY;
 
     /**
      * Evaluates the parameter with matching \a definition and \a value to a feature sink.
@@ -681,7 +682,7 @@ class CORE_EXPORT QgsProcessingParameters
      */
     static QgsFeatureSink *parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariant &value,
                                             const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
-                                            QgsProcessingContext &context, QString &destinationIdentifier SIP_OUT ) SIP_FACTORY;
+                                            QgsProcessingContext &context, QString &destinationIdentifier SIP_OUT, QgsFeatureSink::SinkFlags options = nullptr ) SIP_FACTORY;
 
     /**
      * Evaluates the parameter with matching \a definition to a feature source.

diff --git a/src/core/processing/qgsprocessingutils.cpp b/src/core/processing/qgsprocessingutils.cpp
@@ -373,6 +373,7 @@ void QgsProcessingUtils::parseDestinationString( QString &destination, QString &
           options.insert( QStringLiteral( "layerName" ), layerName );
         }
         uri = dsUri.database();
+        format = QgsVectorFileWriter::driverForExtension( QFileInfo( uri ).completeSuffix() );
       }
       options.insert( QStringLiteral( "update" ), true );
     }
@@ -402,7 +403,7 @@ void QgsProcessingUtils::parseDestinationString( QString &destination, QString &
   }
 }
 
-QgsFeatureSink *QgsProcessingUtils::createFeatureSink( QString &destination, QgsProcessingContext &context, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, const QVariantMap &createOptions )
+QgsFeatureSink *QgsProcessingUtils::createFeatureSink( QString &destination, QgsProcessingContext &context, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, const QVariantMap &createOptions, QgsFeatureSink::SinkFlags sinkFlags )
 {
   QVariantMap options = createOptions;
   if ( !options.contains( QStringLiteral( "fileEncoding" ) ) )
@@ -445,13 +446,14 @@ QgsFeatureSink *QgsProcessingUtils::createFeatureSink( QString &destination, Qgs
     bool useWriter = false;
     parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
 
+    QgsFields newFields = fields;
     if ( useWriter && providerKey == QLatin1String( "ogr" ) )
     {
       // use QgsVectorFileWriter for OGR destinations instead of QgsVectorLayerImport, as that allows
       // us to use any OGR format which supports feature addition
       QString finalFileName;
-      std::unique_ptr< QgsVectorFileWriter > writer = qgis::make_unique< QgsVectorFileWriter >( destination, options.value( QStringLiteral( "fileEncoding" ) ).toString(), fields, geometryType, crs, format, QgsVectorFileWriter::defaultDatasetOptions( format ),
-          QgsVectorFileWriter::defaultLayerOptions( format ), &finalFileName );
+      std::unique_ptr< QgsVectorFileWriter > writer = qgis::make_unique< QgsVectorFileWriter >( destination, options.value( QStringLiteral( "fileEncoding" ) ).toString(), newFields, geometryType, crs, format, QgsVectorFileWriter::defaultDatasetOptions( format ),
+          QgsVectorFileWriter::defaultLayerOptions( format ), &finalFileName, QgsVectorFileWriter::NoSymbology, sinkFlags );
 
       if ( writer->hasError() )
       {
@@ -463,7 +465,7 @@ QgsFeatureSink *QgsProcessingUtils::createFeatureSink( QString &destination, Qgs
     else
     {
       //create empty layer
-      std::unique_ptr< QgsVectorLayerExporter > exporter( new QgsVectorLayerExporter( uri, providerKey, fields, geometryType, crs, true, options ) );
+      std::unique_ptr< QgsVectorLayerExporter > exporter( new QgsVectorLayerExporter( uri, providerKey, newFields, geometryType, crs, true, options, sinkFlags ) );
       if ( exporter->errorCode() )
       {
         throw QgsProcessingException( QObject::tr( "Could not create layer %1: %2" ).arg( destination, exporter->errorMessage() ) );

diff --git a/src/core/processing/qgsprocessingutils.h b/src/core/processing/qgsprocessingutils.h
@@ -44,7 +44,6 @@ class QgsProcessingFeatureSource;
  */
 class CORE_EXPORT QgsProcessingUtils
 {
-
   public:
 
     /**
@@ -154,13 +153,13 @@ class CORE_EXPORT QgsProcessingUtils
      * The caller takes responsibility for deleting the returned sink.
      */
 #ifndef SIP_RUN
-    static QgsFeatureSink *createFeatureSink(
-      QString &destination,
-      QgsProcessingContext &context,
-      const QgsFields &fields,
-      QgsWkbTypes::Type geometryType,
-      const QgsCoordinateReferenceSystem &crs,
-      const QVariantMap &createOptions = QVariantMap() ) SIP_FACTORY;
+    static QgsFeatureSink *createFeatureSink( QString &destination,
+        QgsProcessingContext &context,
+        const QgsFields &fields,
+        QgsWkbTypes::Type geometryType,
+        const QgsCoordinateReferenceSystem &crs,
+        const QVariantMap &createOptions = QVariantMap(),
+        QgsFeatureSink::SinkFlags options = nullptr ) SIP_FACTORY;
 #endif
 
     /**

diff --git a/src/core/qgsfeaturesink.h b/src/core/qgsfeaturesink.h
@@ -34,6 +34,28 @@ class CORE_EXPORT QgsFeatureSink
 {
   public:
 
+    /**
+     * Flags that can be set on a QgsFeatureSink. Not all sinks may implement all flags.
+     *
+     * \since QGIS 3.4
+     */
+    enum SinkFlag
+    {
+
+      /**
+       * This flag indicates, that a primary key field cannot be guaranteed to be unique and
+       * the sink should ignore it if somehow possible.
+       * This should for example be set for a geopackage file if the field "fid" has a risk
+       * to contain duplicate entries. In this case sinks like QgsVectorFileWriter or
+       * QgsVectorLayerExporter will prefer to regenerate the fid instead of trying to reuse
+       * the fids provided in addFeature calls.
+       *
+       * \since QGIS 3.4
+       */
+      RegeneratePrimaryKey = 1 << 1,
+    };
+    Q_DECLARE_FLAGS( SinkFlags, SinkFlag )
+
     //! Flags controlling how features are added to a sink.
     enum Flag
     {

diff --git a/src/core/qgsvectorfilewriter.cpp b/src/core/qgsvectorfilewriter.cpp
@@ -102,6 +102,7 @@ QgsVectorFileWriter::QgsVectorFileWriter(
   const QStringList &layerOptions,
   QString *newFilename,
   SymbologyExport symbologyExport,
+  QgsFeatureSink::SinkFlags sinkFlags,
   QString *newLayer
 )
   : mError( NoError )
@@ -111,7 +112,7 @@ QgsVectorFileWriter::QgsVectorFileWriter(
 {
   init( vectorFileName, fileEncoding, fields,  geometryType,
         srs, driverName, datasourceOptions, layerOptions, newFilename, nullptr,
-        QString(), CreateOrOverwriteFile, newLayer );
+        QString(), CreateOrOverwriteFile, newLayer, sinkFlags );
 }
 
 QgsVectorFileWriter::QgsVectorFileWriter( const QString &vectorFileName,
@@ -135,7 +136,7 @@ QgsVectorFileWriter::QgsVectorFileWriter( const QString &vectorFileName,
 {
   init( vectorFileName, fileEncoding, fields, geometryType, srs, driverName,
         datasourceOptions, layerOptions, newFilename, fieldValueConverter,
-        layerName, action, newLayer );
+        layerName, action, newLayer, nullptr );
 }
 
 bool QgsVectorFileWriter::supportsFeatureStyles( const QString &driverName )
@@ -171,7 +172,7 @@ void QgsVectorFileWriter::init( QString vectorFileName,
                                 FieldValueConverter *fieldValueConverter,
                                 const QString &layerNameIn,
                                 ActionOnExistingFile action,
-                                QString *newLayer )
+                                QString *newLayer, SinkFlags sinkFlags )
 {
   mRenderContext.setRendererScale( mSymbologyScale );
 
@@ -310,6 +311,7 @@ void QgsVectorFileWriter::init( QString vectorFileName,
     }
     options[ datasourceOptions.size()] = nullptr;
   }
+  mAttrIdxToOgrIdx.remove( 0 );
 
   // create the data source
   if ( action == CreateOrOverwriteFile )
@@ -687,6 +689,16 @@ void QgsVectorFileWriter::init( QString vectorFileName,
     }
   }
 
+  // Geopackages require a unique feature id. If the input feature stream cannot guarantee
+  // the uniqueness of the FID column, we drop it and let OGR generate new ones
+  if ( sinkFlags.testFlag( QgsFeatureSink::RegeneratePrimaryKey ) && driverName == QLatin1String( "GPKG" ) )
+  {
+    int fidIdx = fields.lookupField( QStringLiteral( "FID" ) );
+
+    if ( fidIdx >= 0 )
+      mAttrIdxToOgrIdx.remove( fidIdx );
+  }
+
   QgsDebugMsg( QStringLiteral( "Done creating fields" ) );
 
   mWkbType = geometryType;

diff --git a/src/core/qgsvectorfilewriter.h b/src/core/qgsvectorfilewriter.h
@@ -553,7 +553,8 @@ class CORE_EXPORT QgsVectorFileWriter : public QgsFeatureSink
                          const QStringList &datasourceOptions = QStringList(),
                          const QStringList &layerOptions = QStringList(),
                          QString *newFilename = nullptr,
-                         QgsVectorFileWriter::SymbologyExport symbologyExport = QgsVectorFileWriter::NoSymbology
+                         QgsVectorFileWriter::SymbologyExport symbologyExport = QgsVectorFileWriter::NoSymbology,
+                         QgsFeatureSink::SinkFlags sinkFlags = nullptr
 #ifndef SIP_RUN
                              , QString *newLayer = nullptr
 #endif
@@ -880,7 +881,7 @@ class CORE_EXPORT QgsVectorFileWriter : public QgsFeatureSink
                QStringList layerOptions, QString *newFilename,
                QgsVectorFileWriter::FieldValueConverter *fieldValueConverter,
                const QString &layerName,
-               QgsVectorFileWriter::ActionOnExistingFile action, QString *newLayer );
+               QgsVectorFileWriter::ActionOnExistingFile action, QString *newLayer, QgsFeatureSink::SinkFlags sinkFlags );
     void resetMap( const QgsAttributeList &attributes );
 
     std::unique_ptr< QgsFeatureRenderer > mRenderer;