Skip to content

Commit a02d448

Browse files
elpasoelpaso
committedSep 28, 2018
[spatialite] Fix pk-less queries cannot be retrieved by id
Fixes #1993 - Zoom to feature" does not work This actually fixes many more bugs related with the QGIS generated feature id (incremental) when using queries that was not consistent when using QgsFeature request with ids or bbox. This is not a regression: the same error is in 2.18
1 parent b85db09 commit a02d448

File tree

4 files changed

+122
-11
lines changed

4 files changed

+122
-11
lines changed
 

‎src/providers/spatialite/qgsspatialitefeatureiterator.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,8 @@ bool QgsSpatiaLiteFeatureIterator::prepareStatement( const QString &whereClause,
355355
if ( limit >= 0 )
356356
sql += QStringLiteral( " LIMIT %1" ).arg( limit );
357357

358+
qDebug() << sql;
359+
358360
if ( sqlite3_prepare_v2( mHandle->handle(), sql.toUtf8().constData(), -1, &sqliteStatement, nullptr ) != SQLITE_OK )
359361
{
360362
// some error occurred

‎src/providers/spatialite/qgsspatialiteprovider.cpp

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,9 @@ void QgsSpatiaLiteProvider::loadFields()
10661066

10671067
if ( name == mPrimaryKey )
10681068
{
1069+
// Skip if ROWID has been added to the query by the provider
1070+
if ( mRowidInjectedInQuery )
1071+
continue;
10691072
pkCount++;
10701073
if ( mPrimaryKeyAttrs.isEmpty() )
10711074
pkName = name;
@@ -4583,10 +4586,51 @@ bool QgsSpatiaLiteProvider::checkLayerType()
45834586
.arg( mQuery,
45844587
quotedIdentifier( alias ) );
45854588

4586-
sql = QStringLiteral( "SELECT 0 FROM %1 LIMIT 1" ).arg( mQuery );
4589+
sql = QStringLiteral( "SELECT 0, %1 FROM %2 LIMIT 1" ).arg( quotedIdentifier( mGeometryColumn ), mQuery );
45874590
ret = sqlite3_get_table( mSqliteHandle, sql.toUtf8().constData(), &results, &rows, &columns, &errMsg );
45884591
if ( ret == SQLITE_OK && rows == 1 )
45894592
{
4593+
// Check if we can get use the ROWID from the table that provides the geometry
4594+
sqlite3_stmt *stmt = nullptr;
4595+
// 1. find the table that provides geometry
4596+
if ( sqlite3_prepare_v2( mSqliteHandle, sql.toUtf8().constData(), -1, &stmt, nullptr ) == SQLITE_OK )
4597+
{
4598+
mQueryGeomTableName = sqlite3_column_table_name( stmt, 1 );
4599+
}
4600+
// 2. check if the table has a useable ROWID
4601+
if ( ! mQueryGeomTableName.isEmpty() )
4602+
{
4603+
sql = QStringLiteral( "SELECT ROWID FROM %1 WHERE ROWID IS NOT NULL LIMIT 1" ).arg( quotedIdentifier( mQueryGeomTableName ) );
4604+
ret = sqlite3_get_table( mSqliteHandle, sql.toUtf8().constData(), &results, &rows, &columns, &errMsg );
4605+
if ( ret != SQLITE_OK || rows != 1 )
4606+
{
4607+
mQueryGeomTableName = QString();
4608+
}
4609+
}
4610+
// 3. check if ROWID injection works
4611+
if ( ! mQueryGeomTableName.isEmpty() )
4612+
{
4613+
QString newSql( mQuery.replace( QStringLiteral( "SELECT " ),
4614+
QStringLiteral( "SELECT %1.%2, " )
4615+
.arg( quotedIdentifier( mQueryGeomTableName ), QStringLiteral( "ROWID" ) ),
4616+
Qt::CaseInsensitive ) );
4617+
sql = QStringLiteral( "SELECT ROWID FROM %1 WHERE ROWID IS NOT NULL LIMIT 1" ).arg( newSql );
4618+
ret = sqlite3_get_table( mSqliteHandle, sql.toUtf8().constData(), &results, &rows, &columns, &errMsg );
4619+
if ( ret == SQLITE_OK && rows == 1 )
4620+
{
4621+
mQuery = newSql;
4622+
mPrimaryKey = QStringLiteral( "ROWID" );
4623+
mRowidInjectedInQuery = true;
4624+
}
4625+
}
4626+
// 4. if it does not work, simply clear the message and fallback to the original behavior
4627+
if ( errMsg )
4628+
{
4629+
QgsMessageLog::logMessage( tr( "SQLite error while trying to inject ROWID: %2\nSQL: %1" ).arg( sql, errMsg ), tr( "SpatiaLite" ) );
4630+
sqlite3_free( errMsg );
4631+
errMsg = nullptr;
4632+
}
4633+
sqlite3_finalize( stmt );
45904634
mIsQuery = true;
45914635
mReadOnly = true;
45924636
count++;

‎src/providers/spatialite/qgsspatialiteprovider.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,12 @@ class QgsSpatiaLiteProvider: public QgsVectorDataProvider
230230
//! Flag indicating if the layer data source is based on a query
231231
bool mIsQuery = false;
232232

233+
//! String containing the name of the table that provides the geometry if the layer data source is based on a query
234+
QString mQueryGeomTableName;
235+
236+
//! Flag indicating if ROWID has been injected in the query
237+
bool mRowidInjectedInQuery = false;
238+
233239
//! Flag indicating if the layer data source is based on a plain Table
234240
bool mTableBased = false;
235241

‎tests/src/python/test_provider_spatialite.py

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ def setUpClass(cls):
9191
sql = "SELECT AddGeometryColumn('test_pg', 'geometry', 4326, 'POLYGON', 'XY')"
9292
cur.execute(sql)
9393
sql = "INSERT INTO test_pg (id, name, geometry) "
94-
sql += "VALUES (1, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
94+
sql += "VALUES (1, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
9595
cur.execute(sql)
9696

9797
# table with Z dimension geometry
@@ -100,7 +100,7 @@ def setUpClass(cls):
100100
sql = "SELECT AddGeometryColumn('test_z', 'geometry', 4326, 'POINT', 'XYZ')"
101101
cur.execute(sql)
102102
sql = "INSERT INTO test_z (id, name, geometry) "
103-
sql += "VALUES (1, 'toto', GeomFromText('POINT Z (0 0 1)', 4326))"
103+
sql += "VALUES (1, 'toto 2', GeomFromText('POINT Z (0 0 1)', 4326))"
104104
cur.execute(sql)
105105

106106
# table with M value geometry
@@ -109,7 +109,7 @@ def setUpClass(cls):
109109
sql = "SELECT AddGeometryColumn('test_m', 'geometry', 4326, 'POINT', 'XYM')"
110110
cur.execute(sql)
111111
sql = "INSERT INTO test_m (id, name, geometry) "
112-
sql += "VALUES (1, 'toto', GeomFromText('POINT M (0 0 1)', 4326))"
112+
sql += "VALUES (1, 'toto 3', GeomFromText('POINT M (0 0 1)', 4326))"
113113
cur.execute(sql)
114114

115115
# table with Z dimension and M value geometry
@@ -118,7 +118,7 @@ def setUpClass(cls):
118118
sql = "SELECT AddGeometryColumn('test_zm', 'geometry', 4326, 'POINT', 'XYZM')"
119119
cur.execute(sql)
120120
sql = "INSERT INTO test_zm (id, name, geometry) "
121-
sql += "VALUES (1, 'toto', GeomFromText('POINT ZM (0 0 1 1)', 4326))"
121+
sql += "VALUES (1, 'toto 1', GeomFromText('POINT ZM (0 0 1 1)', 4326))"
122122
cur.execute(sql)
123123

124124
# table with multiple column primary key
@@ -127,7 +127,7 @@ def setUpClass(cls):
127127
sql = "SELECT AddGeometryColumn('test_pg_mk', 'geometry', 4326, 'POLYGON', 'XY')"
128128
cur.execute(sql)
129129
sql = "INSERT INTO test_pg_mk (id, name, geometry) "
130-
sql += "VALUES (1, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
130+
sql += "VALUES (1, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
131131
cur.execute(sql)
132132

133133
# simple table with primary key
@@ -136,10 +136,10 @@ def setUpClass(cls):
136136
sql = "SELECT AddGeometryColumn('test_q', 'geometry', 4326, 'POLYGON', 'XY')"
137137
cur.execute(sql)
138138
sql = "INSERT INTO test_q (id, name, geometry) "
139-
sql += "VALUES (11, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
139+
sql += "VALUES (11, 'toto 11', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
140140
cur.execute(sql)
141141
sql = "INSERT INTO test_q (id, name, geometry) "
142-
sql += "VALUES (21, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
142+
sql += "VALUES (21, 'toto 12', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
143143
cur.execute(sql)
144144

145145
# simple table with a geometry column named 'Geometry'
@@ -148,10 +148,10 @@ def setUpClass(cls):
148148
sql = "SELECT AddGeometryColumn('test_n', 'Geometry', 4326, 'POLYGON', 'XY')"
149149
cur.execute(sql)
150150
sql = "INSERT INTO test_n (id, name, geometry) "
151-
sql += "VALUES (1, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
151+
sql += "VALUES (1, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
152152
cur.execute(sql)
153153
sql = "INSERT INTO test_n (id, name, geometry) "
154-
sql += "VALUES (2, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
154+
sql += "VALUES (2, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
155155
cur.execute(sql)
156156

157157
# table with different array types, stored as JSON
@@ -357,7 +357,7 @@ def test_queries(self):
357357
sum_id1 = sum(f.id() for f in l.getFeatures())
358358
# the attribute 'id' works
359359
sum_id2 = sum(f.attributes()[0] for f in l.getFeatures())
360-
self.assertEqual(sum_id1, 3) # 1+2
360+
self.assertEqual(sum_id1, 32) # 11 + 21
361361
self.assertEqual(sum_id2, 32) # 11 + 21
362362

363363
# and now with an id declared
@@ -789,6 +789,65 @@ def testLoadStyle(self):
789789
err, ok = vl.loadDefaultStyle()
790790
self.assertTrue(ok)
791791

792+
def testPkLessQuery(self):
793+
"""Test if features in queries with/without pk can be retrieved by id"""
794+
# create test db
795+
dbname = os.path.join(tempfile.gettempdir(), "test_pkless.sqlite")
796+
if os.path.exists(dbname):
797+
os.remove(dbname)
798+
con = spatialite_connect(dbname, isolation_level=None)
799+
cur = con.cursor()
800+
cur.execute("BEGIN")
801+
sql = "SELECT InitSpatialMetadata()"
802+
cur.execute(sql)
803+
804+
# simple table with primary key
805+
sql = "CREATE TABLE test_pk (id INTEGER NOT NULL PRIMARY KEY, name TEXT NOT NULL)"
806+
cur.execute(sql)
807+
808+
sql = "SELECT AddGeometryColumn('test_pk', 'geometry', 4326, 'POINT', 'XY')"
809+
cur.execute(sql)
810+
811+
for i in range(11, 21):
812+
sql = "INSERT INTO test_pk (id, name, geometry) "
813+
sql += "VALUES ({id}, 'name {id}', GeomFromText('POINT({id} {id})', 4326))".format(id=i)
814+
cur.execute(sql)
815+
816+
# simple table without primary key
817+
sql = "CREATE TABLE test_no_pk (name TEXT NOT NULL)"
818+
cur.execute(sql)
819+
820+
sql = "SELECT AddGeometryColumn('test_no_pk', 'geometry', 4326, 'POINT', 'XY')"
821+
cur.execute(sql)
822+
823+
for i in range(11, 21):
824+
sql = "INSERT INTO test_no_pk (name, geometry) "
825+
sql += "VALUES ('name {id}', GeomFromText('POINT({id} {id})', 4326))".format(id=i)
826+
cur.execute(sql)
827+
828+
cur.execute("COMMIT")
829+
con.close()
830+
831+
def _check_features(vl, offset):
832+
self.assertEqual(vl.featureCount(), 10)
833+
i = 11
834+
for f in vl.getFeatures():
835+
self.assertTrue(f.isValid())
836+
self.assertTrue(vl.getFeature(i - offset).isValid())
837+
self.assertEqual(vl.getFeature(i - offset)['name'], 'name {id}'.format(id=i))
838+
self.assertEqual(f.id(), i - offset)
839+
self.assertEqual(f['name'], 'name {id}'.format(id=i))
840+
self.assertEqual(f.geometry().asWkt(), 'Point ({id} {id})'.format(id=i))
841+
i += 1
842+
843+
vl_pk = QgsVectorLayer('dbname=\'%s\' table="(select * from test_pk)" (geometry) sql=' % dbname, 'pk', 'spatialite')
844+
self.assertTrue(vl_pk.isValid())
845+
_check_features(vl_pk, 0)
846+
847+
vl_no_pk = QgsVectorLayer('dbname=\'%s\' table="(select * from test_no_pk)" (geometry) sql=' % dbname, 'pk', 'spatialite')
848+
self.assertTrue(vl_no_pk.isValid())
849+
_check_features(vl_no_pk, 10)
850+
792851

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

0 commit comments

Comments
 (0)