Address review comments

nyalldawson · nyalldawson · commit aaa18e0b89dc · 2017-12-02T09:01:25.000+10:00
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -1982,8 +1982,6 @@ bool QgsOgrProvider::createSpatialIndex()
   {
     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 );
@@ -2012,6 +2010,12 @@ bool QgsOgrProvider::createAttributeIndex( int field )
   if ( mGDALDriverName == QLatin1String( "GPKG" ) ||
        mGDALDriverName == QLatin1String( "SQLite" ) )
   {
+    if ( field == 0 && mFirstFieldIsFid )
+    {
+      // already an index on this field, no need to re-created
+      return false;
+    }
+
     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 );
diff --git a/tests/src/python/test_provider_ogr_gpkg.py b/tests/src/python/test_provider_ogr_gpkg.py
@@ -787,6 +787,10 @@ def testCreateAttributeIndex(self):
         self.assertTrue(vl.dataProvider().capabilities() & QgsVectorDataProvider.CreateAttributeIndex)
         self.assertFalse(vl.dataProvider().createAttributeIndex(-1))
         self.assertFalse(vl.dataProvider().createAttributeIndex(100))
+
+        # should not be allowed - there's already a index on the primary key
+        self.assertFalse(vl.dataProvider().createAttributeIndex(0))
+
         self.assertTrue(vl.dataProvider().createAttributeIndex(1))
 
         con = spatialite_connect(tmpfile, isolation_level=None)
@@ -819,7 +823,7 @@ def testCreateAttributeIndex(self):
     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 = ds.CreateLayer('test', geom_type=ogr.wkbPolygon, options=['SPATIAL_INDEX=NO'])
         lyr.CreateField(ogr.FieldDefn('str_field', ogr.OFTString))
         lyr.CreateField(ogr.FieldDefn('str_field2', ogr.OFTString))
         f = None
