Skip to content
Permalink
Browse files

QgsVectorLayerExporter: defer GPKG spatial index creation at file clo…

…sing

Should fix the issue raised in
https://lists.osgeo.org/pipermail/qgis-developer/2020-October/062422.html

The difference with QgsVectorFileWriter is that QgsVectorFileWriter does not
close and re-open the layer, so the defered spatial index creation mechanism
that is built-in in the GPKG driver works. But in the case of
QgsVectorLayerExporter, we close and re-open and thus the spatial index gets
created immediately.
  • Loading branch information
rouault authored and nyalldawson committed Oct 17, 2020
1 parent 501c63f commit d8c6a25abe6c1de930aeb65c4c575ba60c99a805
@@ -62,11 +62,52 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri,
{
mProvider = nullptr;

QMap<QString, QVariant> 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 )
{
@@ -164,6 +164,8 @@ class CORE_EXPORT QgsVectorLayerExporter : public QgsFeatureSink

QgsFeatureList mFeatureBuffer;

bool mCreateSpatialIndex = true;

#ifdef SIP_RUN
QgsVectorLayerExporter( const QgsVectorLayerExporter &rh );
#endif
@@ -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()

0 comments on commit d8c6a25

Please sign in to comment.
You can’t perform that action at this time.