[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
This commit is contained in:
Alessandro Pasotti 2018-09-28 10:29:23 +02:00
parent b85db09b4f
commit a02d448ba6
4 changed files with 122 additions and 11 deletions

View File

@ -355,6 +355,8 @@ bool QgsSpatiaLiteFeatureIterator::prepareStatement( const QString &whereClause,
if ( limit >= 0 )
sql += QStringLiteral( " LIMIT %1" ).arg( limit );
qDebug() << sql;
if ( sqlite3_prepare_v2( mHandle->handle(), sql.toUtf8().constData(), -1, &sqliteStatement, nullptr ) != SQLITE_OK )
{
// some error occurred

View File

@ -1066,6 +1066,9 @@ void QgsSpatiaLiteProvider::loadFields()
if ( name == mPrimaryKey )
{
// Skip if ROWID has been added to the query by the provider
if ( mRowidInjectedInQuery )
continue;
pkCount++;
if ( mPrimaryKeyAttrs.isEmpty() )
pkName = name;
@ -4583,10 +4586,51 @@ bool QgsSpatiaLiteProvider::checkLayerType()
.arg( mQuery,
quotedIdentifier( alias ) );
sql = QStringLiteral( "SELECT 0 FROM %1 LIMIT 1" ).arg( mQuery );
sql = QStringLiteral( "SELECT 0, %1 FROM %2 LIMIT 1" ).arg( quotedIdentifier( mGeometryColumn ), mQuery );
ret = sqlite3_get_table( mSqliteHandle, sql.toUtf8().constData(), &results, &rows, &columns, &errMsg );
if ( ret == SQLITE_OK && rows == 1 )
{
// Check if we can get use the ROWID from the table that provides the geometry
sqlite3_stmt *stmt = nullptr;
// 1. find the table that provides geometry
if ( sqlite3_prepare_v2( mSqliteHandle, sql.toUtf8().constData(), -1, &stmt, nullptr ) == SQLITE_OK )
{
mQueryGeomTableName = sqlite3_column_table_name( stmt, 1 );
}
// 2. check if the table has a useable ROWID
if ( ! mQueryGeomTableName.isEmpty() )
{
sql = QStringLiteral( "SELECT ROWID FROM %1 WHERE ROWID IS NOT NULL LIMIT 1" ).arg( quotedIdentifier( mQueryGeomTableName ) );
ret = sqlite3_get_table( mSqliteHandle, sql.toUtf8().constData(), &results, &rows, &columns, &errMsg );
if ( ret != SQLITE_OK || rows != 1 )
{
mQueryGeomTableName = QString();
}
}
// 3. check if ROWID injection works
if ( ! mQueryGeomTableName.isEmpty() )
{
QString newSql( mQuery.replace( QStringLiteral( "SELECT " ),
QStringLiteral( "SELECT %1.%2, " )
.arg( quotedIdentifier( mQueryGeomTableName ), QStringLiteral( "ROWID" ) ),
Qt::CaseInsensitive ) );
sql = QStringLiteral( "SELECT ROWID FROM %1 WHERE ROWID IS NOT NULL LIMIT 1" ).arg( newSql );
ret = sqlite3_get_table( mSqliteHandle, sql.toUtf8().constData(), &results, &rows, &columns, &errMsg );
if ( ret == SQLITE_OK && rows == 1 )
{
mQuery = newSql;
mPrimaryKey = QStringLiteral( "ROWID" );
mRowidInjectedInQuery = true;
}
}
// 4. if it does not work, simply clear the message and fallback to the original behavior
if ( errMsg )
{
QgsMessageLog::logMessage( tr( "SQLite error while trying to inject ROWID: %2\nSQL: %1" ).arg( sql, errMsg ), tr( "SpatiaLite" ) );
sqlite3_free( errMsg );
errMsg = nullptr;
}
sqlite3_finalize( stmt );
mIsQuery = true;
mReadOnly = true;
count++;

View File

@ -230,6 +230,12 @@ class QgsSpatiaLiteProvider: public QgsVectorDataProvider
//! Flag indicating if the layer data source is based on a query
bool mIsQuery = false;
//! String containing the name of the table that provides the geometry if the layer data source is based on a query
QString mQueryGeomTableName;
//! Flag indicating if ROWID has been injected in the query
bool mRowidInjectedInQuery = false;
//! Flag indicating if the layer data source is based on a plain Table
bool mTableBased = false;

View File

@ -91,7 +91,7 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sql = "SELECT AddGeometryColumn('test_pg', 'geometry', 4326, 'POLYGON', 'XY')"
cur.execute(sql)
sql = "INSERT INTO test_pg (id, name, geometry) "
sql += "VALUES (1, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
sql += "VALUES (1, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
cur.execute(sql)
# table with Z dimension geometry
@ -100,7 +100,7 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sql = "SELECT AddGeometryColumn('test_z', 'geometry', 4326, 'POINT', 'XYZ')"
cur.execute(sql)
sql = "INSERT INTO test_z (id, name, geometry) "
sql += "VALUES (1, 'toto', GeomFromText('POINT Z (0 0 1)', 4326))"
sql += "VALUES (1, 'toto 2', GeomFromText('POINT Z (0 0 1)', 4326))"
cur.execute(sql)
# table with M value geometry
@ -109,7 +109,7 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sql = "SELECT AddGeometryColumn('test_m', 'geometry', 4326, 'POINT', 'XYM')"
cur.execute(sql)
sql = "INSERT INTO test_m (id, name, geometry) "
sql += "VALUES (1, 'toto', GeomFromText('POINT M (0 0 1)', 4326))"
sql += "VALUES (1, 'toto 3', GeomFromText('POINT M (0 0 1)', 4326))"
cur.execute(sql)
# table with Z dimension and M value geometry
@ -118,7 +118,7 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sql = "SELECT AddGeometryColumn('test_zm', 'geometry', 4326, 'POINT', 'XYZM')"
cur.execute(sql)
sql = "INSERT INTO test_zm (id, name, geometry) "
sql += "VALUES (1, 'toto', GeomFromText('POINT ZM (0 0 1 1)', 4326))"
sql += "VALUES (1, 'toto 1', GeomFromText('POINT ZM (0 0 1 1)', 4326))"
cur.execute(sql)
# table with multiple column primary key
@ -127,7 +127,7 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sql = "SELECT AddGeometryColumn('test_pg_mk', 'geometry', 4326, 'POLYGON', 'XY')"
cur.execute(sql)
sql = "INSERT INTO test_pg_mk (id, name, geometry) "
sql += "VALUES (1, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
sql += "VALUES (1, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
cur.execute(sql)
# simple table with primary key
@ -136,10 +136,10 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sql = "SELECT AddGeometryColumn('test_q', 'geometry', 4326, 'POLYGON', 'XY')"
cur.execute(sql)
sql = "INSERT INTO test_q (id, name, geometry) "
sql += "VALUES (11, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
sql += "VALUES (11, 'toto 11', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_q (id, name, geometry) "
sql += "VALUES (21, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
sql += "VALUES (21, 'toto 12', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
cur.execute(sql)
# simple table with a geometry column named 'Geometry'
@ -148,10 +148,10 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sql = "SELECT AddGeometryColumn('test_n', 'Geometry', 4326, 'POLYGON', 'XY')"
cur.execute(sql)
sql = "INSERT INTO test_n (id, name, geometry) "
sql += "VALUES (1, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
sql += "VALUES (1, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_n (id, name, geometry) "
sql += "VALUES (2, 'toto', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
sql += "VALUES (2, 'toto 1', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
cur.execute(sql)
# table with different array types, stored as JSON
@ -357,7 +357,7 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
sum_id1 = sum(f.id() for f in l.getFeatures())
# the attribute 'id' works
sum_id2 = sum(f.attributes()[0] for f in l.getFeatures())
self.assertEqual(sum_id1, 3) # 1+2
self.assertEqual(sum_id1, 32) # 11 + 21
self.assertEqual(sum_id2, 32) # 11 + 21
# and now with an id declared
@ -789,6 +789,65 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
err, ok = vl.loadDefaultStyle()
self.assertTrue(ok)
def testPkLessQuery(self):
"""Test if features in queries with/without pk can be retrieved by id"""
# create test db
dbname = os.path.join(tempfile.gettempdir(), "test_pkless.sqlite")
if os.path.exists(dbname):
os.remove(dbname)
con = spatialite_connect(dbname, isolation_level=None)
cur = con.cursor()
cur.execute("BEGIN")
sql = "SELECT InitSpatialMetadata()"
cur.execute(sql)
# simple table with primary key
sql = "CREATE TABLE test_pk (id INTEGER NOT NULL PRIMARY KEY, name TEXT NOT NULL)"
cur.execute(sql)
sql = "SELECT AddGeometryColumn('test_pk', 'geometry', 4326, 'POINT', 'XY')"
cur.execute(sql)
for i in range(11, 21):
sql = "INSERT INTO test_pk (id, name, geometry) "
sql += "VALUES ({id}, 'name {id}', GeomFromText('POINT({id} {id})', 4326))".format(id=i)
cur.execute(sql)
# simple table without primary key
sql = "CREATE TABLE test_no_pk (name TEXT NOT NULL)"
cur.execute(sql)
sql = "SELECT AddGeometryColumn('test_no_pk', 'geometry', 4326, 'POINT', 'XY')"
cur.execute(sql)
for i in range(11, 21):
sql = "INSERT INTO test_no_pk (name, geometry) "
sql += "VALUES ('name {id}', GeomFromText('POINT({id} {id})', 4326))".format(id=i)
cur.execute(sql)
cur.execute("COMMIT")
con.close()
def _check_features(vl, offset):
self.assertEqual(vl.featureCount(), 10)
i = 11
for f in vl.getFeatures():
self.assertTrue(f.isValid())
self.assertTrue(vl.getFeature(i - offset).isValid())
self.assertEqual(vl.getFeature(i - offset)['name'], 'name {id}'.format(id=i))
self.assertEqual(f.id(), i - offset)
self.assertEqual(f['name'], 'name {id}'.format(id=i))
self.assertEqual(f.geometry().asWkt(), 'Point ({id} {id})'.format(id=i))
i += 1
vl_pk = QgsVectorLayer('dbname=\'%s\' table="(select * from test_pk)" (geometry) sql=' % dbname, 'pk', 'spatialite')
self.assertTrue(vl_pk.isValid())
_check_features(vl_pk, 0)
vl_no_pk = QgsVectorLayer('dbname=\'%s\' table="(select * from test_no_pk)" (geometry) sql=' % dbname, 'pk', 'spatialite')
self.assertTrue(vl_no_pk.isValid())
_check_features(vl_no_pk, 10)
if __name__ == '__main__':
unittest.main()