diff --git a/src/core/qgsvectorlayerexporter.cpp b/src/core/qgsvectorlayerexporter.cpp index 2c9cd7990d5d..ae7c84068073 100644 --- a/src/core/qgsvectorlayerexporter.cpp +++ b/src/core/qgsvectorlayerexporter.cpp @@ -62,11 +62,52 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri, { mProvider = nullptr; + QMap modifiedOptions( options ); + + // For GPKG/Spatialite, we explicitly ask not to create a spatial index at + // layer creation since this would slow down inserts. Defer its creation + // to end of exportLayer() or destruction of this object. + if ( geometryType != QgsWkbTypes::NoGeometry && + providerKey == QLatin1String( "ogr" ) && + options.contains( QStringLiteral( "driverName" ) ) && + ( options[ QStringLiteral( "driverName" ) ].toString().compare( QLatin1String( "GPKG" ), Qt::CaseInsensitive ) == 0 || + options[ QStringLiteral( "driverName" ) ].toString().compare( QLatin1String( "SQLite" ), Qt::CaseInsensitive ) == 0 ) ) + { + QStringList modifiedLayerOptions; + if ( options.contains( QStringLiteral( "layerOptions" ) ) ) + { + QStringList layerOptions = options.value( QStringLiteral( "layerOptions" ) ).toStringList(); + for ( const QString &layerOption : layerOptions ) + { + if ( layerOption.compare( QLatin1String( "SPATIAL_INDEX=YES" ), Qt::CaseInsensitive ) == 0 || + layerOption.compare( QLatin1String( "SPATIAL_INDEX=ON" ), Qt::CaseInsensitive ) == 0 || + layerOption.compare( QLatin1String( "SPATIAL_INDEX=TRUE" ), Qt::CaseInsensitive ) == 0 || + layerOption.compare( QLatin1String( "SPATIAL_INDEX=1" ), Qt::CaseInsensitive ) == 0 ) + { + // do nothing + } + else if ( layerOption.compare( QLatin1String( "SPATIAL_INDEX=NO" ), Qt::CaseInsensitive ) == 0 || + layerOption.compare( QLatin1String( "SPATIAL_INDEX=OFF" ), Qt::CaseInsensitive ) == 0 || + layerOption.compare( QLatin1String( "SPATIAL_INDEX=FALSE" ), Qt::CaseInsensitive ) == 0 || + layerOption.compare( QLatin1String( "SPATIAL_INDEX=0" ), Qt::CaseInsensitive ) == 0 ) + { + mCreateSpatialIndex = false; + } + else + { + modifiedLayerOptions << layerOption; + } + } + } + modifiedLayerOptions << QStringLiteral( "SPATIAL_INDEX=FALSE" ); + modifiedOptions[ QStringLiteral( "layerOptions" ) ] = modifiedLayerOptions; + } + // create an empty layer QString errMsg; QgsProviderRegistry *pReg = QgsProviderRegistry::instance(); mError = pReg->createEmptyLayer( providerKey, uri, fields, geometryType, crs, overwrite, mOldToNewAttrIdx, - errMsg, !options.isEmpty() ? &options : nullptr ); + errMsg, !modifiedOptions.isEmpty() ? &modifiedOptions : nullptr ); if ( errorCode() ) { mErrorMessage = errMsg; @@ -132,6 +173,12 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri, QgsVectorLayerExporter::~QgsVectorLayerExporter() { flushBuffer(); + + if ( mCreateSpatialIndex ) + { + createSpatialIndex(); + } + delete mProvider; } @@ -222,6 +269,7 @@ bool QgsVectorLayerExporter::flushBuffer() bool QgsVectorLayerExporter::createSpatialIndex() { + mCreateSpatialIndex = false; if ( mProvider && ( mProvider->capabilities() & QgsVectorDataProvider::CreateSpatialIndex ) != 0 ) { return mProvider->createSpatialIndex(); @@ -433,7 +481,7 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer, } int errors = writer->errorCount(); - if ( !writer->createSpatialIndex() ) + if ( writer->mCreateSpatialIndex && !writer->createSpatialIndex() ) { if ( writer->errorCode() && errorMessage ) { diff --git a/src/core/qgsvectorlayerexporter.h b/src/core/qgsvectorlayerexporter.h index edf03878298a..d249238a1316 100644 --- a/src/core/qgsvectorlayerexporter.h +++ b/src/core/qgsvectorlayerexporter.h @@ -164,6 +164,8 @@ class CORE_EXPORT QgsVectorLayerExporter : public QgsFeatureSink QgsFeatureList mFeatureBuffer; + bool mCreateSpatialIndex = true; + #ifdef SIP_RUN QgsVectorLayerExporter( const QgsVectorLayerExporter &rh ); #endif diff --git a/tests/src/python/test_provider_ogr_gpkg.py b/tests/src/python/test_provider_ogr_gpkg.py index 398026c3761a..c935136846d2 100644 --- a/tests/src/python/test_provider_ogr_gpkg.py +++ b/tests/src/python/test_provider_ogr_gpkg.py @@ -1851,6 +1851,79 @@ def testTransactionGroupCrash(self): feature.setAttributes([None, 'three']) self.assertTrue(vl.addFeature(feature)) + def _testVectorLayerExporterDeferredSpatialIndex(self, layerOptions, expectSpatialIndex): + """ Internal method """ + + tmpfile = '/vsimem/_testVectorLayerExporterDeferredSpatialIndex.gpkg' + options = {} + options['driverName'] = 'GPKG' + options['layerName'] = 'table1' + if layerOptions: + options['layerOptions'] = layerOptions + exporter = QgsVectorLayerExporter(tmpfile, "ogr", QgsFields(), QgsWkbTypes.Polygon, + QgsCoordinateReferenceSystem(3111), False, options) + self.assertFalse(exporter.errorCode(), + 'unexpected export error {}: {}'.format(exporter.errorCode(), exporter.errorMessage())) + + # Check that at that point the rtree is *not* created + ds = ogr.Open(tmpfile) + sql_lyr = ds.ExecuteSQL("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'gpkg_extensions'") + assert sql_lyr.GetNextFeature() is None + ds.ReleaseResultSet(sql_lyr) + del ds + + del exporter + + ds = gdal.OpenEx(tmpfile, gdal.OF_VECTOR) + if expectSpatialIndex: + # Check that at that point the rtree is created + sql_lyr = ds.ExecuteSQL("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'gpkg_extensions'") + assert sql_lyr.GetNextFeature() is not None + ds.ReleaseResultSet(sql_lyr) + sql_lyr = ds.ExecuteSQL("SELECT 1 FROM gpkg_extensions WHERE table_name = 'table1' AND extension_name = 'gpkg_rtree_index'") + assert sql_lyr.GetNextFeature() is not None + ds.ReleaseResultSet(sql_lyr) + else: + # Check that at that point the rtree is *still not* created + sql_lyr = ds.ExecuteSQL("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'gpkg_extensions'") + assert sql_lyr.GetNextFeature() is None + ds.ReleaseResultSet(sql_lyr) + return ds + + def testVectorLayerExporterDeferredSpatialIndexNoLayerOptions(self): + """ Check that a deferred spatial index is created when no layer creation options is provided """ + + ds = self._testVectorLayerExporterDeferredSpatialIndex(None, True) + filename = ds.GetDescription() + del ds + gdal.Unlink(filename) + + def testVectorLayerExporterDeferredSpatialIndexLayerOptions(self): + """ Check that a deferred spatial index is created when other layer creations options is provided """ + + ds = self._testVectorLayerExporterDeferredSpatialIndex(['GEOMETRY_NAME=my_geom'], True) + lyr = ds.GetLayer(0) + self.assertEqual(lyr.GetGeometryColumn(), 'my_geom') + filename = ds.GetDescription() + del ds + gdal.Unlink(filename) + + def testVectorLayerExporterDeferredSpatialIndexExplicitSpatialIndexAsked(self): + """ Check that a deferred spatial index is created when explicit asked """ + + ds = self._testVectorLayerExporterDeferredSpatialIndex(['SPATIAL_INDEX=YES'], True) + filename = ds.GetDescription() + del ds + gdal.Unlink(filename) + + def testVectorLayerExporterDeferredSpatialIndexSpatialIndexDisallowed(self): + """ Check that a deferred spatial index is NOT created when explicit disallowed """ + + ds = self._testVectorLayerExporterDeferredSpatialIndex(['SPATIAL_INDEX=NO'], False) + filename = ds.GetDescription() + del ds + gdal.Unlink(filename) + if __name__ == '__main__': unittest.main()