Merge pull request #8832 from troopa81/fix_20547

fixes #20547 : Fix error when using a spatialite layer with a non-int primary key an a spatial index
qgis · Jan 11, 2019 · b61d6f8 · b61d6f8
2 parents 7b664ea + 8efc689
commit b61d6f8
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 3 deletions.
diff --git a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp b/src/providers/spatialite/qgsspatialitefeatureiterator.cpp
@@ -423,7 +423,7 @@ QString QgsSpatiaLiteFeatureIterator::whereClauseRect()
       mbrFilter += QStringLiteral( "ymax >= %1" ).arg( qgsDoubleToString( mFilterRect.yMinimum() ) );
       QString idxName = QStringLiteral( "idx_%1_%2" ).arg( mSource->mIndexTable, mSource->mIndexGeometry );
       whereClause += QStringLiteral( "%1 IN (SELECT pkid FROM %2 WHERE %3)" )
-                     .arg( quotedPrimaryKey(),
+                     .arg( QStringLiteral( "ROWID" ),
                            QgsSqliteUtils::quotedIdentifier( idxName ),
                            mbrFilter );
     }
@@ -432,7 +432,7 @@ QString QgsSpatiaLiteFeatureIterator::whereClauseRect()
       // using the MbrCache spatial index
       QString idxName = QStringLiteral( "cache_%1_%2" ).arg( mSource->mIndexTable, mSource->mIndexGeometry );
       whereClause += QStringLiteral( "%1 IN (SELECT rowid FROM %2 WHERE mbr = FilterMbrIntersects(%3))" )
-                     .arg( quotedPrimaryKey(),
+                     .arg( QStringLiteral( "ROWID" ),
                            QgsSqliteUtils::quotedIdentifier( idxName ),
                            mbr( mFilterRect ) );
     }
@@ -506,7 +506,7 @@ bool QgsSpatiaLiteFeatureIterator::getFeature( sqlite3_stmt *stmt, QgsFeature &f
   {
     if ( ic == 0 )
     {
-      if ( mHasPrimaryKey )
+      if ( mHasPrimaryKey && sqlite3_column_type( stmt, ic ) == SQLITE_INTEGER )
       {
         // first column always contains the ROWID (or the primary key)
         QgsFeatureId fid = sqlite3_column_int64( stmt, ic );

diff --git a/tests/src/python/test_provider_spatialite.py b/tests/src/python/test_provider_spatialite.py
@@ -31,6 +31,8 @@
                        QgsVectorLayerUtils,
                        QgsSettings,
                        QgsDefaultValue,
+                       QgsFeatureRequest,
+                       QgsRectangle,
                        QgsWkbTypes)
 
 from qgis.testing import start_app, unittest
@@ -752,6 +754,58 @@ def testDecodeUri(self):
         components = registry.decodeUri('spatialite', uri)
         self.assertEqual(components['path'], filename)
 
+    def testPKNotInt(self):
+        """ Check when primary key is not an integer """
+        # create test db
+        dbname = os.path.join(tempfile.mkdtemp(), "test_pknotint.sqlite")
+        con = spatialite_connect(dbname, isolation_level=None)
+        cur = con.cursor()
+
+        # try the two different types of index creation
+        for index_creation_method in ['CreateSpatialIndex', 'CreateMbrCache']:
+
+            table_name = "pk_is_string_{}".format(index_creation_method)
+
+            cur.execute("BEGIN")
+            sql = "SELECT InitSpatialMetadata()"
+            cur.execute(sql)
+
+            # create table with spatial index and pk is string
+            sql = "CREATE TABLE {}(id VARCHAR PRIMARY KEY NOT NULL, name TEXT NOT NULL);"
+            cur.execute(sql.format(table_name))
+
+            sql = "SELECT AddGeometryColumn('{}', 'geometry',  4326, 'POINT', 'XY')"
+            cur.execute(sql.format(table_name))
+
+            sql = "SELECT {}('{}', 'geometry')"
+            cur.execute(sql.format(index_creation_method, table_name))
+
+            sql = "insert into {} ('id', 'name', 'geometry') values( 'test_id', 'test_name', st_geomfromtext('POINT(1 2)', 4326))"
+            cur.execute(sql.format(table_name))
+
+            cur.execute("COMMIT")
+
+            testPath = "dbname={} table='{}' (geometry)".format(dbname, table_name)
+            vl = QgsVectorLayer(testPath, 'test', 'spatialite')
+            self.assertTrue(vl.isValid())
+            self.assertEqual(vl.featureCount(), 1)
+
+            # make spatial request to force the index use
+            request = QgsFeatureRequest(QgsRectangle(0, 0, 2, 3))
+            feature = next(vl.getFeatures(request), None)
+            self.assertTrue(feature)
+
+            self.assertEqual(feature.id(), 1)
+            point = feature.geometry().asPoint()
+            self.assertTrue(point)
+            self.assertEqual(point.x(), 1)
+            self.assertEqual(point.y(), 2)
+
+        con.close()
+
+        basepath, filename = os.path.split(dbname)
+        shutil.rmtree(basepath)
+
     def testLoadStyle(self):
         """Check that we can store and load a style"""