Skip to content

Commit

Permalink
Fix OGR provider cannot create attribute or spatial indexes for
Browse files Browse the repository at this point in the history 
GeoPackage/SQLite layers

Previously this capability was only exposed for shapefiles,
but was available in the spatialite provider. We don't use that
for GeoPackages, so I've ported the functionality across to
the OGR provider for these data sources.

Includes unit tests
  • Loading branch information
@nyalldawson
nyalldawson committed Dec 1, 2017
1 parent 31c79da commit 791eb91
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 19 deletions.
76 changes: 58 additions & 18 deletions src/providers/ogr/qgsogrprovider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1966,36 +1966,70 @@ bool QgsOgrProvider::createSpatialIndex()
if ( !doInitialActionsForEdition() )
return false;

if ( mGDALDriverName != QLatin1String( "ESRI Shapefile" ) )
return false;

QByteArray layerName = mOgrOrigLayer->name();
if ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) )
{
QByteArray sql = QByteArray( "CREATE SPATIAL INDEX ON " ) + quotedIdentifier( layerName ); // quote the layer name so spaces are handled
QgsDebugMsg( QString( "SQL: %1" ).arg( QString::fromUtf8( sql ) ) );
mOgrOrigLayer->ExecuteSQLNoReturn( sql );

QByteArray sql = QByteArray( "CREATE SPATIAL INDEX ON " ) + quotedIdentifier( layerName ); // quote the layer name so spaces are handled
QgsDebugMsg( QString( "SQL: %1" ).arg( QString::fromUtf8( sql ) ) );
mOgrOrigLayer->ExecuteSQLNoReturn( sql );
QFileInfo fi( mFilePath ); // to get the base name
//find out, if the .qix file is there
return QFileInfo::exists( fi.path().append( '/' ).append( fi.completeBaseName() ).append( ".qix" ) );
}
else if ( mGDALDriverName == QLatin1String( "GPKG" ) ||
mGDALDriverName == QLatin1String( "SQLite" ) )
{
QMutex *mutex = nullptr;
OGRLayerH layer = mOgrOrigLayer->getHandleAndMutex( mutex );
QMutexLocker locker( mutex );

QByteArray sql = QByteArray( "SELECT CreateSpatialIndex(" + quotedIdentifier( layerName ) + ","
+ quotedIdentifier( OGR_L_GetGeometryColumn( layer ) ) + ") " ); // quote the layer name so spaces are handled
mOgrOrigLayer->ExecuteSQLNoReturn( sql );
return true;
}
return false;
}

QFileInfo fi( mFilePath ); // to get the base name
//find out, if the .qix file is there
QFile indexfile( fi.path().append( '/' ).append( fi.completeBaseName() ).append( ".qix" ) );
return indexfile.exists();
QString createIndexName( QString tableName, QString field )
{
QRegularExpression safeExp( QStringLiteral( "[^a-zA-Z0-9]" ) );
tableName.replace( safeExp, QStringLiteral( "_" ) );
field.replace( safeExp, QStringLiteral( "_" ) );
return tableName + "_" + field + "_idx";
}

bool QgsOgrProvider::createAttributeIndex( int field )
{
if ( field < 0 || field >= mAttributeFields.count() )
return false;

if ( !doInitialActionsForEdition() )
return false;

QByteArray quotedLayerName = quotedIdentifier( mOgrOrigLayer->name() );
QByteArray dropSql = "DROP INDEX ON " + quotedLayerName;
mOgrOrigLayer->ExecuteSQLNoReturn( dropSql );
QByteArray createSql = "CREATE INDEX ON " + quotedLayerName + " USING " + textEncoding()->fromUnicode( fields().at( field ).name() );
mOgrOrigLayer->ExecuteSQLNoReturn( createSql );
if ( mGDALDriverName == QLatin1String( "GPKG" ) ||
mGDALDriverName == QLatin1String( "SQLite" ) )
{
QString indexName = createIndexName( mOgrOrigLayer->name(), fields().at( field ).name() );
QByteArray createSql = "CREATE INDEX IF NOT EXISTS " + textEncoding()->fromUnicode( indexName ) + " ON " + quotedLayerName + " (" + textEncoding()->fromUnicode( fields().at( field ).name() ) + ")";
mOgrOrigLayer->ExecuteSQLNoReturn( createSql );
return true;
}
else
{
QByteArray dropSql = "DROP INDEX ON " + quotedLayerName;
mOgrOrigLayer->ExecuteSQLNoReturn( dropSql );
QByteArray createSql = "CREATE INDEX ON " + quotedLayerName + " USING " + textEncoding()->fromUnicode( fields().at( field ).name() );
mOgrOrigLayer->ExecuteSQLNoReturn( createSql );

QFileInfo fi( mFilePath ); // to get the base name
//find out, if the .idm file is there
QFile indexfile( fi.path().append( '/' ).append( fi.completeBaseName() ).append( ".idm" ) );
return indexfile.exists();
QFileInfo fi( mFilePath ); // to get the base name
//find out, if the .idm/.ind file is there
QString idmFile( fi.path().append( '/' ).append( fi.completeBaseName() ).append( ".idm" ) );
QString indFile( fi.path().append( '/' ).append( fi.completeBaseName() ).append( ".ind" ) );
return QFile::exists( idmFile ) || QFile::exists( indFile );
}
}

bool QgsOgrProvider::deleteFeatures( const QgsFeatureIds &id )
Expand Down Expand Up @@ -2241,6 +2275,12 @@ void QgsOgrProvider::computeCapabilities()
ability &= ~( AddAttributes | DeleteFeatures );
}
}
else if ( mGDALDriverName == QLatin1String( "GPKG" ) ||
mGDALDriverName == QLatin1String( "SQLite" ) )
{
ability |= CreateSpatialIndex;
ability |= CreateAttributeIndex;
}

/* Curve geometries are available in some drivers starting with GDAL 2.0 */
if ( mOgrLayer->TestCapability( "CurveGeometries" ) )
Expand Down
61 changes: 60 additions & 1 deletion tests/src/python/test_provider_ogr_gpkg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@
QgsPointXY,
QgsProject,
QgsWkbTypes,
QgsDataProvider)
QgsDataProvider,
QgsVectorDataProvider)
from qgis.PyQt.QtCore import QCoreApplication, QVariant
from qgis.testing import start_app, unittest
from qgis.utils import spatialite_connect


def GDAL_COMPUTE_VERSION(maj, min, rev):
Expand Down Expand Up @@ -771,6 +773,63 @@ def test_SplitFeature(self):
self.assertEqual([f for f in layer.getFeatures()][0].geometry().asWkt(), 'Polygon ((0.5 0, 0.5 1, 1 1, 1 0, 0.5 0))')
self.assertEqual([f for f in layer.getFeatures()][1].geometry().asWkt(), 'Polygon ((0.5 1, 0.5 0, 0 0, 0 1, 0.5 1))')

def testCreateAttributeIndex(self):
tmpfile = os.path.join(self.basetestpath, 'testGeopackageAttributeIndex.gpkg')
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPolygon)
lyr.CreateField(ogr.FieldDefn('str_field', ogr.OFTString))
lyr.CreateField(ogr.FieldDefn('str_field2', ogr.OFTString))
f = None
ds = None

vl = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
self.assertTrue(vl.isValid())
self.assertTrue(vl.dataProvider().capabilities() & QgsVectorDataProvider.CreateAttributeIndex)
self.assertFalse(vl.dataProvider().createAttributeIndex(-1))
self.assertFalse(vl.dataProvider().createAttributeIndex(100))
self.assertTrue(vl.dataProvider().createAttributeIndex(1))

con = spatialite_connect(tmpfile, isolation_level=None)
cur = con.cursor()
rs = cur.execute("SELECT * FROM sqlite_master WHERE type='index' AND tbl_name='test'")
res = [row for row in rs]
self.assertEqual(len(res), 1)
index_name = res[0][1]
rs = cur.execute("PRAGMA index_info({})".format(index_name))
res = [row for row in rs]
self.assertEqual(len(res), 1)
self.assertEqual(res[0][2], 'str_field')

# second index
self.assertTrue(vl.dataProvider().createAttributeIndex(2))
rs = cur.execute("SELECT * FROM sqlite_master WHERE type='index' AND tbl_name='test'")
res = [row for row in rs]
self.assertEqual(len(res), 2)
indexed_columns = []
for row in res:
index_name = row[1]
rs = cur.execute("PRAGMA index_info({})".format(index_name))
res = [row for row in rs]
self.assertEqual(len(res), 1)
indexed_columns.append(res[0][2])

self.assertCountEqual(indexed_columns, ['str_field', 'str_field2'])
con.close()

def testCreateSpatialIndex(self):
tmpfile = os.path.join(self.basetestpath, 'testGeopackageSpatialIndex.gpkg')
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPolygon)
lyr.CreateField(ogr.FieldDefn('str_field', ogr.OFTString))
lyr.CreateField(ogr.FieldDefn('str_field2', ogr.OFTString))
f = None
ds = None

vl = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
self.assertTrue(vl.isValid())
self.assertTrue(vl.dataProvider().capabilities() & QgsVectorDataProvider.CreateSpatialIndex)
self.assertTrue(vl.dataProvider().createSpatialIndex())


if __name__ == '__main__':
unittest.main()
28 changes: 28 additions & 0 deletions tests/src/python/test_provider_shapefile.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,34 @@ def testOpenWithFilter(self):
# force close of data provider
vl.setDataSource('', 'test', 'ogr')

def testCreateAttributeIndex(self):
tmpdir = tempfile.mkdtemp()
self.dirs_to_cleanup.append(tmpdir)
srcpath = os.path.join(TEST_DATA_DIR, 'provider')
for file in glob.glob(os.path.join(srcpath, 'shapefile.*')):
shutil.copy(os.path.join(srcpath, file), tmpdir)
datasource = os.path.join(tmpdir, 'shapefile.shp')

vl = QgsVectorLayer('{}|layerid=0'.format(datasource), 'test', 'ogr')
self.assertTrue(vl.isValid())
self.assertTrue(vl.dataProvider().capabilities() & QgsVectorDataProvider.CreateAttributeIndex)
self.assertFalse(vl.dataProvider().createAttributeIndex(-1))
self.assertFalse(vl.dataProvider().createAttributeIndex(100))
self.assertTrue(vl.dataProvider().createAttributeIndex(1))

def testCreateSpatialIndex(self):
tmpdir = tempfile.mkdtemp()
self.dirs_to_cleanup.append(tmpdir)
srcpath = os.path.join(TEST_DATA_DIR, 'provider')
for file in glob.glob(os.path.join(srcpath, 'shapefile.*')):
shutil.copy(os.path.join(srcpath, file), tmpdir)
datasource = os.path.join(tmpdir, 'shapefile.shp')

vl = QgsVectorLayer('{}|layerid=0'.format(datasource), 'test', 'ogr')
self.assertTrue(vl.isValid())
self.assertTrue(vl.dataProvider().capabilities() & QgsVectorDataProvider.CreateSpatialIndex)
self.assertTrue(vl.dataProvider().createSpatialIndex())


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

0 comments on commit 791eb91

Please sign in to comment.