Skip to content

Commit d8c6a25

Browse files
rouaultnyalldawson
authored andcommittedOct 17, 2020
QgsVectorLayerExporter: defer GPKG spatial index creation at file closing
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.
1 parent 501c63f commit d8c6a25

File tree

3 files changed

+125
-2
lines changed

3 files changed

+125
-2
lines changed
 

‎src/core/qgsvectorlayerexporter.cpp

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,52 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri,
6262
{
6363
mProvider = nullptr;
6464

65+
QMap<QString, QVariant> modifiedOptions( options );
66+
67+
// For GPKG/Spatialite, we explicitly ask not to create a spatial index at
68+
// layer creation since this would slow down inserts. Defer its creation
69+
// to end of exportLayer() or destruction of this object.
70+
if ( geometryType != QgsWkbTypes::NoGeometry &&
71+
providerKey == QLatin1String( "ogr" ) &&
72+
options.contains( QStringLiteral( "driverName" ) ) &&
73+
( options[ QStringLiteral( "driverName" ) ].toString().compare( QLatin1String( "GPKG" ), Qt::CaseInsensitive ) == 0 ||
74+
options[ QStringLiteral( "driverName" ) ].toString().compare( QLatin1String( "SQLite" ), Qt::CaseInsensitive ) == 0 ) )
75+
{
76+
QStringList modifiedLayerOptions;
77+
if ( options.contains( QStringLiteral( "layerOptions" ) ) )
78+
{
79+
QStringList layerOptions = options.value( QStringLiteral( "layerOptions" ) ).toStringList();
80+
for ( const QString &layerOption : layerOptions )
81+
{
82+
if ( layerOption.compare( QLatin1String( "SPATIAL_INDEX=YES" ), Qt::CaseInsensitive ) == 0 ||
83+
layerOption.compare( QLatin1String( "SPATIAL_INDEX=ON" ), Qt::CaseInsensitive ) == 0 ||
84+
layerOption.compare( QLatin1String( "SPATIAL_INDEX=TRUE" ), Qt::CaseInsensitive ) == 0 ||
85+
layerOption.compare( QLatin1String( "SPATIAL_INDEX=1" ), Qt::CaseInsensitive ) == 0 )
86+
{
87+
// do nothing
88+
}
89+
else if ( layerOption.compare( QLatin1String( "SPATIAL_INDEX=NO" ), Qt::CaseInsensitive ) == 0 ||
90+
layerOption.compare( QLatin1String( "SPATIAL_INDEX=OFF" ), Qt::CaseInsensitive ) == 0 ||
91+
layerOption.compare( QLatin1String( "SPATIAL_INDEX=FALSE" ), Qt::CaseInsensitive ) == 0 ||
92+
layerOption.compare( QLatin1String( "SPATIAL_INDEX=0" ), Qt::CaseInsensitive ) == 0 )
93+
{
94+
mCreateSpatialIndex = false;
95+
}
96+
else
97+
{
98+
modifiedLayerOptions << layerOption;
99+
}
100+
}
101+
}
102+
modifiedLayerOptions << QStringLiteral( "SPATIAL_INDEX=FALSE" );
103+
modifiedOptions[ QStringLiteral( "layerOptions" ) ] = modifiedLayerOptions;
104+
}
105+
65106
// create an empty layer
66107
QString errMsg;
67108
QgsProviderRegistry *pReg = QgsProviderRegistry::instance();
68109
mError = pReg->createEmptyLayer( providerKey, uri, fields, geometryType, crs, overwrite, mOldToNewAttrIdx,
69-
errMsg, !options.isEmpty() ? &options : nullptr );
110+
errMsg, !modifiedOptions.isEmpty() ? &modifiedOptions : nullptr );
70111
if ( errorCode() )
71112
{
72113
mErrorMessage = errMsg;
@@ -132,6 +173,12 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri,
132173
QgsVectorLayerExporter::~QgsVectorLayerExporter()
133174
{
134175
flushBuffer();
176+
177+
if ( mCreateSpatialIndex )
178+
{
179+
createSpatialIndex();
180+
}
181+
135182
delete mProvider;
136183
}
137184

@@ -222,6 +269,7 @@ bool QgsVectorLayerExporter::flushBuffer()
222269

223270
bool QgsVectorLayerExporter::createSpatialIndex()
224271
{
272+
mCreateSpatialIndex = false;
225273
if ( mProvider && ( mProvider->capabilities() & QgsVectorDataProvider::CreateSpatialIndex ) != 0 )
226274
{
227275
return mProvider->createSpatialIndex();
@@ -433,7 +481,7 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer,
433481
}
434482
int errors = writer->errorCount();
435483

436-
if ( !writer->createSpatialIndex() )
484+
if ( writer->mCreateSpatialIndex && !writer->createSpatialIndex() )
437485
{
438486
if ( writer->errorCode() && errorMessage )
439487
{

‎src/core/qgsvectorlayerexporter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ class CORE_EXPORT QgsVectorLayerExporter : public QgsFeatureSink
164164

165165
QgsFeatureList mFeatureBuffer;
166166

167+
bool mCreateSpatialIndex = true;
168+
167169
#ifdef SIP_RUN
168170
QgsVectorLayerExporter( const QgsVectorLayerExporter &rh );
169171
#endif

‎tests/src/python/test_provider_ogr_gpkg.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,6 +1851,79 @@ def testTransactionGroupCrash(self):
18511851
feature.setAttributes([None, 'three'])
18521852
self.assertTrue(vl.addFeature(feature))
18531853

1854+
def _testVectorLayerExporterDeferredSpatialIndex(self, layerOptions, expectSpatialIndex):
1855+
""" Internal method """
1856+
1857+
tmpfile = '/vsimem/_testVectorLayerExporterDeferredSpatialIndex.gpkg'
1858+
options = {}
1859+
options['driverName'] = 'GPKG'
1860+
options['layerName'] = 'table1'
1861+
if layerOptions:
1862+
options['layerOptions'] = layerOptions
1863+
exporter = QgsVectorLayerExporter(tmpfile, "ogr", QgsFields(), QgsWkbTypes.Polygon,
1864+
QgsCoordinateReferenceSystem(3111), False, options)
1865+
self.assertFalse(exporter.errorCode(),
1866+
'unexpected export error {}: {}'.format(exporter.errorCode(), exporter.errorMessage()))
1867+
1868+
# Check that at that point the rtree is *not* created
1869+
ds = ogr.Open(tmpfile)
1870+
sql_lyr = ds.ExecuteSQL("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'gpkg_extensions'")
1871+
assert sql_lyr.GetNextFeature() is None
1872+
ds.ReleaseResultSet(sql_lyr)
1873+
del ds
1874+
1875+
del exporter
1876+
1877+
ds = gdal.OpenEx(tmpfile, gdal.OF_VECTOR)
1878+
if expectSpatialIndex:
1879+
# Check that at that point the rtree is created
1880+
sql_lyr = ds.ExecuteSQL("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'gpkg_extensions'")
1881+
assert sql_lyr.GetNextFeature() is not None
1882+
ds.ReleaseResultSet(sql_lyr)
1883+
sql_lyr = ds.ExecuteSQL("SELECT 1 FROM gpkg_extensions WHERE table_name = 'table1' AND extension_name = 'gpkg_rtree_index'")
1884+
assert sql_lyr.GetNextFeature() is not None
1885+
ds.ReleaseResultSet(sql_lyr)
1886+
else:
1887+
# Check that at that point the rtree is *still not* created
1888+
sql_lyr = ds.ExecuteSQL("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'gpkg_extensions'")
1889+
assert sql_lyr.GetNextFeature() is None
1890+
ds.ReleaseResultSet(sql_lyr)
1891+
return ds
1892+
1893+
def testVectorLayerExporterDeferredSpatialIndexNoLayerOptions(self):
1894+
""" Check that a deferred spatial index is created when no layer creation options is provided """
1895+
1896+
ds = self._testVectorLayerExporterDeferredSpatialIndex(None, True)
1897+
filename = ds.GetDescription()
1898+
del ds
1899+
gdal.Unlink(filename)
1900+
1901+
def testVectorLayerExporterDeferredSpatialIndexLayerOptions(self):
1902+
""" Check that a deferred spatial index is created when other layer creations options is provided """
1903+
1904+
ds = self._testVectorLayerExporterDeferredSpatialIndex(['GEOMETRY_NAME=my_geom'], True)
1905+
lyr = ds.GetLayer(0)
1906+
self.assertEqual(lyr.GetGeometryColumn(), 'my_geom')
1907+
filename = ds.GetDescription()
1908+
del ds
1909+
gdal.Unlink(filename)
1910+
1911+
def testVectorLayerExporterDeferredSpatialIndexExplicitSpatialIndexAsked(self):
1912+
""" Check that a deferred spatial index is created when explicit asked """
1913+
1914+
ds = self._testVectorLayerExporterDeferredSpatialIndex(['SPATIAL_INDEX=YES'], True)
1915+
filename = ds.GetDescription()
1916+
del ds
1917+
gdal.Unlink(filename)
1918+
1919+
def testVectorLayerExporterDeferredSpatialIndexSpatialIndexDisallowed(self):
1920+
""" Check that a deferred spatial index is NOT created when explicit disallowed """
1921+
1922+
ds = self._testVectorLayerExporterDeferredSpatialIndex(['SPATIAL_INDEX=NO'], False)
1923+
filename = ds.GetDescription()
1924+
del ds
1925+
gdal.Unlink(filename)
1926+
18541927

18551928
if __name__ == '__main__':
18561929
unittest.main()

0 commit comments

Comments
 (0)
Please sign in to comment.