Merge pull request #4410 from rouault/fix_12695_qgis2_18

DBManager: fix importing a new layer in a GeoPackage (#16295)

Author: rouault

python/plugins/db_manager/dlg_import_vector.py

@@ -317,9 +317,16 @@ def accept(self):
 
             # get output params, update output URI
             self.outUri.setDataSource(schema, table, geom, "", pk)
-            uri = self.outUri.uri(False)
-
+            typeName = self.db.dbplugin().typeName()
             providerName = self.db.dbplugin().providerName()
+            if typeName == 'gpkg':
+                uri = self.outUri.database()
+                options['update'] = True
+                options['driverName'] = 'GPKG'
+                options['layerName'] = table
+            else:
+                uri = self.outUri.uri(False)
+
             if self.chkDropTable.isChecked():
                 options['overwrite'] = True
 
@@ -367,6 +374,8 @@ def accept(self):
         if self.chkSpatialIndex.isEnabled() and self.chkSpatialIndex.isChecked():
             self.db.connector.createSpatialIndex((schema, table), geom)
 
+        self.db.connection().reconnect()
+        self.db.refresh()
         QMessageBox.information(self, self.tr("Import to database"), self.tr("Import was successful."))
         return QDialog.accept(self)
 

src/core/qgsvectorfilewriter.h

@@ -433,6 +433,37 @@ class CORE_EXPORT QgsVectorFileWriter
                          SymbologyExport symbologyExport = NoSymbology
                        );
 
+    /** Create a new vector file writer.
+     * \param vectorFileName file name to write to
+     * \param fileEncoding encoding to use
+     * \param fields fields to write
+     * \param geometryType geometry type of output file
+     * \param srs spatial reference system of output file
+     * \param driverName OGR driver to use
+     * \param datasourceOptions list of OGR data source creation options
+     * \param layerOptions list of OGR layer creation options
+     * \param newFilename potentially modified file name (output parameter)
+     * \param symbologyExport symbology to export
+     * \param fieldValueConverter field value converter (added in QGIS 2.16)
+     * \param layerName layer name. If let empty, it will be derived from the filename (added in QGIS 3.0)
+     * \param action action on existing file (added in QGIS 3.0)
+     * \note not available in Python bindings
+     */
+    QgsVectorFileWriter( const QString &vectorFileName,
+                         const QString &fileEncoding,
+                         const QgsFields &fields,
+                         QgsWKBTypes::Type geometryType,
+                         const QgsCoordinateReferenceSystem* srs,
+                         const QString &driverName,
+                         const QStringList &datasourceOptions,
+                         const QStringList &layerOptions,
+                         QString *newFilename,
+                         SymbologyExport symbologyExport,
+                         FieldValueConverter *fieldValueConverter,
+                         const QString &layerName,
+                         ActionOnExistingFile action
+                       );
+
     /** Returns map with format filter string as key and OGR format key as value*/
     static QMap< QString, QString> supportedFiltersAndFormats();
 
@@ -548,42 +579,13 @@ class CORE_EXPORT QgsVectorFileWriter
 
   private:
 
-    /** Create a new vector file writer.
-     * @param vectorFileName file name to write to
-     * @param fileEncoding encoding to use
-     * @param fields fields to write
-     * @param geometryType geometry type of output file
-     * @param srs spatial reference system of output file
-     * @param driverName OGR driver to use
-     * @param datasourceOptions list of OGR data source creation options
-     * @param layerOptions list of OGR layer creation options
-     * @param newFilename potentially modified file name (output parameter)
-     * @param symbologyExport symbology to export
-     * @param fieldValueConverter field value converter (added in QGIS 2.16)
-     * @param layerName layer name. If let empty, it will be derived from the filename (added in QGIS 3.0)
-     * @param action action on existing file (added in QGIS 3.0)
-     */
-    QgsVectorFileWriter( const QString& vectorFileName,
-                         const QString& fileEncoding,
-                         const QgsFields& fields,
-                         QgsWKBTypes::Type geometryType,
-                         const QgsCoordinateReferenceSystem* srs,
-                         const QString& driverName,
-                         const QStringList &datasourceOptions,
-                         const QStringList &layerOptions,
-                         QString *newFilename,
-                         SymbologyExport symbologyExport,
-                         FieldValueConverter* fieldValueConverter,
-                         const QString& layerName,
-                         ActionOnExistingFile action
-                       );
-
     void init( QString vectorFileName, QString fileEncoding, const QgsFields& fields,
                QgsWKBTypes::Type geometryType, const QgsCoordinateReferenceSystem* srs,
                const QString& driverName, QStringList datasourceOptions,
                QStringList layerOptions, QString* newFilename,
                FieldValueConverter* fieldValueConverter,
                const QString& layerName,
+
                ActionOnExistingFile action );
     void resetMap( const QgsAttributeList &attributes );
 

src/core/qgsvectorlayerimport.cpp

@@ -97,7 +97,20 @@ QgsVectorLayerImport::QgsVectorLayerImport( const QString &uri,
 
   QgsDebugMsg( "Created empty layer" );
 
-  QgsVectorDataProvider *vectorProvider = dynamic_cast< QgsVectorDataProvider* >( pReg->provider( providerKey, uri ) );
+  QString uriUpdated( uri );
+  // HACK sorry...
+  if ( providerKey == "ogr" )
+  {
+    QString layerName;
+    if ( options->contains( "layerName" ) )
+      layerName = options->value( "layerName" ).toString();
+    if ( !layerName.isEmpty() )
+    {
+      uriUpdated += "|layername=";
+      uriUpdated += layerName;
+    }
+  }
+  QgsVectorDataProvider *vectorProvider = dynamic_cast< QgsVectorDataProvider * >( pReg->provider( providerKey, uriUpdated ) );
   if ( !vectorProvider || !vectorProvider->isValid() || ( vectorProvider->capabilities() & QgsVectorDataProvider::AddFeatures ) == 0 )
   {
     mError = ErrInvalidLayer;

src/providers/ogr/qgsogrprovider.cpp

@@ -230,6 +230,7 @@ QgsVectorLayerImport::ImportError QgsOgrProvider::createEmptyLayer(
   QString encoding;
   QString driverName = "ESRI Shapefile";
   QStringList dsOptions, layerOptions;
+  QString layerName;
 
   if ( options )
   {
@@ -244,14 +245,45 @@ QgsVectorLayerImport::ImportError QgsOgrProvider::createEmptyLayer(
 
     if ( options->contains( "layerOptions" ) )
       layerOptions << options->value( "layerOptions" ).toStringList();
+
+    if ( options->contains( "layerName" ) )
+      layerName = options->value( "layerName" ).toString();
   }
 
   if ( oldToNewAttrIdxMap )
     oldToNewAttrIdxMap->clear();
   if ( errorMessage )
     errorMessage->clear();
 
-  if ( !overwrite )
+  QgsVectorFileWriter::ActionOnExistingFile action( QgsVectorFileWriter::CreateOrOverwriteFile );
+
+  bool update = false;
+  if ( options->contains( "update" ) )
+  {
+    update = options->value( "update" ).toBool();
+    if ( update )
+    {
+      if ( !overwrite && !layerName.isEmpty() )
+      {
+        OGRDataSourceH hDS = OGROpen( uri.toUtf8().constData(), TRUE, nullptr );
+        if ( hDS )
+        {
+          if ( OGR_DS_GetLayerByName( hDS, layerName.toUtf8().constData() ) )
+          {
+            OGR_DS_Destroy( hDS );
+            if ( errorMessage )
+              *errorMessage += QObject::tr( "Layer %2 of %1 exists and overwrite flag is false." )
+                               .arg( uri ).arg( layerName );
+            return QgsVectorLayerImport::ErrCreateDataSource;
+          }
+          OGR_DS_Destroy( hDS );
+        }
+      }
+      action = QgsVectorFileWriter::CreateOrOverwriteLayer;
+    }
+  }
+
+  if ( !overwrite && !update )
   {
     QFileInfo fi( uri );
     if ( fi.exists() )
@@ -264,8 +296,10 @@ QgsVectorLayerImport::ImportError QgsOgrProvider::createEmptyLayer(
   }
 
   QgsVectorFileWriter *writer = new QgsVectorFileWriter(
-    uri, encoding, fields, wkbType,
-    srs, driverName, dsOptions, layerOptions );
+    uri, encoding, fields, QGis::fromOldWkbType( wkbType ),
+    srs, driverName, dsOptions, layerOptions, nullptr,
+    QgsVectorFileWriter::NoSymbology, nullptr,
+    layerName, action );
 
   QgsVectorFileWriter::WriterError error = writer->hasError();
   if ( error )
@@ -277,16 +311,37 @@ QgsVectorLayerImport::ImportError QgsOgrProvider::createEmptyLayer(
     return ( QgsVectorLayerImport::ImportError ) error;
   }
 
+  QMap<int, int> attrIdxMap = writer->attrIdxToOgrIdx();
+  delete writer;
+
   if ( oldToNewAttrIdxMap )
   {
-    QMap<int, int> attrIdxMap = writer->attrIdxToOgrIdx();
+    bool firstFieldIsFid = false;
+    if ( !layerName.isEmpty() )
+    {
+      OGRDataSourceH hDS = OGROpen( uri.toUtf8().constData(), TRUE, nullptr );
+      if ( hDS )
+      {
+        OGRLayerH hLayer = OGR_DS_GetLayerByName( hDS, layerName.toUtf8().constData() );
+        if ( hLayer )
+        {
+          // Expose the OGR FID if it comes from a "real" column (typically GPKG)
+          // and make sure that this FID column is not exposed as a regular OGR field (shouldn't happen normally)
+          firstFieldIsFid = !( EQUAL( OGR_L_GetFIDColumn( hLayer ), "" ) ) &&
+                            OGR_FD_GetFieldIndex( OGR_L_GetLayerDefn( hLayer ), OGR_L_GetFIDColumn( hLayer ) ) < 0 &&
+                            fields.indexFromName( OGR_L_GetFIDColumn( hLayer ) ) < 0;
+
+        }
+        OGR_DS_Destroy( hDS );
+      }
+    }
+
     for ( QMap<int, int>::const_iterator attrIt = attrIdxMap.begin(); attrIt != attrIdxMap.end(); ++attrIt )
     {
-      oldToNewAttrIdxMap->insert( attrIt.key(), *attrIt );
+      oldToNewAttrIdxMap->insert( attrIt.key(), *attrIt + ( firstFieldIsFid ? 1 : 0 ) );
     }
   }
 
-  delete writer;
   return QgsVectorLayerImport::NoError;
 }
 

tests/src/python/test_provider_ogr_gpkg.py

@@ -21,7 +21,7 @@
 from osgeo import gdal, ogr
 
 from qgis.PyQt.QtCore import QCoreApplication, QSettings
-from qgis.core import QgsVectorLayer, QgsFeature, QgsGeometry, QgsFeatureRequest, QgsRectangle
+from qgis.core import QgsVectorLayer, QgsVectorLayerImport, QgsFeature, QgsGeometry, QgsFeatureRequest, QgsRectangle
 from qgis.testing import start_app, unittest
 from utilities import unitTestDataPath
 
@@ -268,6 +268,62 @@ def testDisablewalForSqlite3(self):
 
         QSettings().setValue("/qgis/walForSqlite3", None)
 
+    def testSimulatedDBManagerImport(self):
+        uri = 'point?field=f1:int'
+        uri += '&field=f2:double(6,4)'
+        uri += '&field=f3:string(20)'
+        lyr = QgsVectorLayer(uri, "x", "memory")
+        self.assertTrue(lyr.isValid())
+        f = QgsFeature(lyr.fields())
+        f['f1'] = 1
+        f['f2'] = 123.456
+        f['f3'] = '12345678.90123456789'
+        f2 = QgsFeature(lyr.fields())
+        f2['f1'] = 2
+        lyr.dataProvider().addFeatures([f, f2])
+
+        tmpfile = os.path.join(self.basetestpath, 'testSimulatedDBManagerImport.gpkg')
+        ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
+        ds = None
+        options = {}
+        options['update'] = True
+        options['driverName'] = 'GPKG'
+        options['layerName'] = 'my_out_table'
+        err = QgsVectorLayerImport.importLayer(lyr, tmpfile, "ogr", lyr.crs(), False, False, options)
+        self.assertEqual(err[0], QgsVectorLayerImport.NoError,
+                         'unexpected import error {0}'.format(err))
+        lyr = QgsVectorLayer(tmpfile + "|layername=my_out_table", "y", "ogr")
+        self.assertTrue(lyr.isValid())
+        features = lyr.getFeatures()
+        f = next(features)
+        self.assertEqual(f['f1'], 1)
+        self.assertEqual(f['f2'], 123.456)
+        self.assertEqual(f['f3'], '12345678.90123456789')
+        f = next(features)
+        self.assertEqual(f['f1'], 2)
+        features = None
+
+        # Test overwriting without overwrite option
+        err = QgsVectorLayerImport.importLayer(lyr, tmpfile, "ogr", lyr.crs(), False, False, options)
+        self.assertEqual(err[0], QgsVectorLayerImport.ErrCreateDataSource)
+
+        # Test overwriting
+        lyr = QgsVectorLayer(uri, "x", "memory")
+        self.assertTrue(lyr.isValid())
+        f = QgsFeature(lyr.fields())
+        f['f1'] = 3
+        lyr.dataProvider().addFeatures([f])
+        options['overwrite'] = True
+        err = QgsVectorLayerImport.importLayer(lyr, tmpfile, "ogr", lyr.crs(), False, False, options)
+        self.assertEqual(err[0], QgsVectorLayerImport.NoError,
+                         'unexpected import error {0}'.format(err))
+        lyr = QgsVectorLayer(tmpfile + "|layername=my_out_table", "y", "ogr")
+        self.assertTrue(lyr.isValid())
+        features = lyr.getFeatures()
+        f = next(features)
+        self.assertEqual(f['f1'], 3)
+        features = None
+
 
 if __name__ == '__main__':
     unittest.main()