Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Handle fid and geom cols in GPKG exec sql
  • Loading branch information
elpaso committed Jul 6, 2021
1 parent cf26022 commit 34c5cdc
Show file tree
Hide file tree
Showing 11 changed files with 187 additions and 39 deletions.
Expand Up @@ -635,10 +635,12 @@ Returns the fields of a ``table`` and ``schema``.

.. note::

The default implementation creates a temporary vector layer, providers may
choose to override this method for a greater efficiency.
the default implementation creates a temporary vector layer, providers may
choose to override this method for a greater efficiency of to overcome provider's
behavior when the layer does not expose all fields (GPKG for example hides geometry
and primary key column).

:raises QgsProviderConnectionException: if any errors are encountered.
:raises QgsProviderConnectionException:

.. versionadded:: 3.16
%End
Expand Down
116 changes: 111 additions & 5 deletions src/core/providers/ogr/qgsgeopackageproviderconnection.cpp
Expand Up @@ -405,28 +405,64 @@ QgsAbstractDatabaseProviderConnection::QueryResult QgsGeoPackageProviderConnecti
if ( fet.reset( OGR_L_GetNextFeature( ogrLayer ) ), fet )
{

QgsFields fields { QgsOgrUtils::readOgrFields( fet.get(), QTextCodec::codecForName( "UTF-8" ) ) };
iterator->setFields( fields );
// May need to prepend PK and append geometries to the columns
thread_local const QRegularExpression pkRegExp { QStringLiteral( R"(^select\s+(\*|fid)[,\s+](.*)from)" ), QRegularExpression::PatternOption::CaseInsensitiveOption };
if ( pkRegExp.match( sql.trimmed() ).hasMatch() )
{
iterator->setPrimaryKeyColumnName( QStringLiteral( "fid" ) );
results.appendColumn( QStringLiteral( "fid" ) );
}

QgsFields fields { QgsOgrUtils::readOgrFields( fet.get(), QTextCodec::codecForName( "UTF-8" ) ) };
for ( const auto &f : std::as_const( fields ) )
{
results.appendColumn( f.name() );
}

// Geometry columns
OGRFeatureDefnH featureDef = OGR_F_GetDefnRef( fet.get() );

if ( featureDef )
{
QStringList geomColumnsList;
for ( int idx = 0; idx < OGR_F_GetGeomFieldCount( fet.get() ); ++idx )
{
OGRGeomFieldDefnH geomFldDef { OGR_F_GetGeomFieldDefnRef( fet.get(), idx ) };
if ( geomFldDef )
{
const QString geomColumnName { OGR_GFld_GetNameRef( geomFldDef ) };
geomColumnsList.append( geomColumnName );
results.appendColumn( geomColumnName );
}
}
iterator->setGeometryColumnNames( geomColumnsList );
}

iterator->setFields( fields );
}

// Check for errors
errCause = CPLGetLastErrorMsg( );
if ( CE_Failure == CPLGetLastErrorType() || CE_Fatal == CPLGetLastErrorType() )
{
errCause = CPLGetLastErrorMsg( );
}

if ( ! errCause.isEmpty() )
{
throw QgsProviderConnectionException( QObject::tr( "Error executing SQL %1: %2" ).arg( sql, errCause ) );
throw QgsProviderConnectionException( QObject::tr( "Error executing SQL statement %1: %2" ).arg( sql, errCause ) );
}

OGR_L_ResetReading( ogrLayer );
iterator->nextRow();
return results;
}
errCause = CPLGetLastErrorMsg( );

// Check for errors
if ( CE_Failure == CPLGetLastErrorType() || CE_Fatal == CPLGetLastErrorType() )
{
errCause = CPLGetLastErrorMsg( );
}

}
else
{
Expand Down Expand Up @@ -456,6 +492,12 @@ QVariantList QgsGeoPackageProviderResultIterator::nextRowInternal()
gdal::ogr_feature_unique_ptr fet;
if ( fet.reset( OGR_L_GetNextFeature( mOgrLayer ) ), fet )
{
// PK
if ( ! mPrimaryKeyColumnName.isEmpty() )
{
row.push_back( OGR_F_GetFID( fet.get() ) );
}

if ( ! mFields.isEmpty() )
{
QgsFeature f { QgsOgrUtils::readOgrFeature( fet.get(), mFields, QTextCodec::codecForName( "UTF-8" ) ) };
Expand All @@ -464,6 +506,13 @@ QVariantList QgsGeoPackageProviderResultIterator::nextRowInternal()
{
row.push_back( attribute );
}

// Geoms go last
for ( const auto &geomColName : std::as_const( mGeometryColumnNames ) )
{
row.push_back( f.geometry().asWkb() );
}

}
else // Fallback to strings
{
Expand Down Expand Up @@ -493,6 +542,16 @@ void QgsGeoPackageProviderResultIterator::setFields( const QgsFields &fields )
mFields = fields;
}

void QgsGeoPackageProviderResultIterator::setGeometryColumnNames( const QStringList &geometryColumnNames )
{
mGeometryColumnNames = geometryColumnNames;
}

void QgsGeoPackageProviderResultIterator::setPrimaryKeyColumnName( const QString &primaryKeyColumnName )
{
mPrimaryKeyColumnName = primaryKeyColumnName;
}

QList<QgsVectorDataProvider::NativeType> QgsGeoPackageProviderConnection::nativeTypes() const
{
QgsVectorLayer::LayerOptions options { false, true };
Expand All @@ -508,6 +567,53 @@ QList<QgsVectorDataProvider::NativeType> QgsGeoPackageProviderConnection::native
return vl.dataProvider()->nativeTypes();
}

QgsFields QgsGeoPackageProviderConnection::fields( const QString &schema, const QString &table ) const
{

Q_UNUSED( schema )

// Get fields from layer
QgsFields fieldList;

// Prepend PK fid
fieldList.append( QgsField{ QStringLiteral( "fid" ), QVariant::LongLong } );

QgsVectorLayer::LayerOptions options { false, true };
options.skipCrsValidation = true;
QgsVectorLayer vl { tableUri( schema, table ), QStringLiteral( "temp_layer" ), mProviderKey, options };
if ( vl.isValid() )
{
const auto parentFields { vl.fields() };
for ( const auto &pField : std::as_const( parentFields ) )
{
fieldList.append( pField );
}
// Append name of the geometry column, the data provider does not expose this information so we need an extra query:/
const QString sql = QStringLiteral( "SELECT g.column_name "
"FROM gpkg_contents c LEFT JOIN gpkg_geometry_columns g ON (c.table_name = g.table_name) "
"WHERE c.table_name = %1" ).arg( QgsSqliteUtils::quotedString( table ) );
try
{
const auto results = executeSql( sql );
if ( ! results.isEmpty() )
{
fieldList.append( QgsField{ results.first().first().toString(), QVariant::String, QStringLiteral( "geometry" ) } );
}
}
catch ( QgsProviderConnectionException &ex )
{
throw QgsProviderConnectionException( QObject::tr( "Error retrieving fields information for uri %1: %2" ).arg( vl.publicSource(), ex.what() ) );
}

}
else
{
throw QgsProviderConnectionException( QObject::tr( "Error retrieving fields information for uri: %1" ).arg( vl.publicSource() ) );
}

return fieldList;
}

QgsGeoPackageProviderResultIterator::~QgsGeoPackageProviderResultIterator()
{
if ( mHDS )
Expand Down
8 changes: 8 additions & 0 deletions src/core/providers/ogr/qgsgeopackageproviderconnection.h
Expand Up @@ -27,6 +27,7 @@

struct QgsGeoPackageProviderResultIterator: public QgsAbstractDatabaseProviderConnection::QueryResult::QueryResultIterator
{

QgsGeoPackageProviderResultIterator( gdal::ogr_datasource_unique_ptr hDS, OGRLayerH ogrLayer )
: mHDS( std::move( hDS ) )
, mOgrLayer( ogrLayer )
Expand All @@ -35,13 +36,17 @@ struct QgsGeoPackageProviderResultIterator: public QgsAbstractDatabaseProviderCo
~QgsGeoPackageProviderResultIterator();

void setFields( const QgsFields &fields );
void setGeometryColumnNames( const QStringList &geometryColumnNames );
void setPrimaryKeyColumnName( const QString &primaryKeyColumnName );

private:

gdal::ogr_datasource_unique_ptr mHDS;
OGRLayerH mOgrLayer;
QgsFields mFields;
QVariantList mNextRow;
QStringList mGeometryColumnNames;
QString mPrimaryKeyColumnName;

QVariantList nextRowPrivate() override;
QVariantList nextRowInternal();
Expand Down Expand Up @@ -76,12 +81,15 @@ class QgsGeoPackageProviderConnection : public QgsAbstractDatabaseProviderConnec
const TableFlags &flags = TableFlags() ) const override;
QIcon icon() const override;
QList<QgsVectorDataProvider::NativeType> nativeTypes() const override;
QgsFields fields( const QString &schema, const QString &table ) const override;

private:

void setDefaultCapabilities();
//! Use GDAL to execute SQL
QueryResult executeGdalSqlPrivate( const QString &sql, QgsFeedback *feedback = nullptr ) const;


};


Expand Down
Expand Up @@ -301,6 +301,7 @@ QgsFields QgsAbstractDatabaseProviderConnection::fields( const QString &schema,
QgsVectorLayer vl { tableUri( schema, tableName ), QStringLiteral( "temp_layer" ), mProviderKey, options };
if ( vl.isValid() )
{
// Note: this implementation works for providers that do not hide any "special" field (geometry or PKs)
return vl.fields();
}
else
Expand Down
8 changes: 5 additions & 3 deletions src/core/providers/qgsabstractdatabaseproviderconnection.h
Expand Up @@ -736,9 +736,11 @@ class CORE_EXPORT QgsAbstractDatabaseProviderConnection : public QgsAbstractProv
/**
* Returns the fields of a \a table and \a schema.
*
* \note The default implementation creates a temporary vector layer, providers may
* choose to override this method for a greater efficiency.
* \throws QgsProviderConnectionException if any errors are encountered.
* \note the default implementation creates a temporary vector layer, providers may
* choose to override this method for a greater efficiency of to overcome provider's
* behavior when the layer does not expose all fields (GPKG for example hides geometry
* and primary key column).
* \throws QgsProviderConnectionException
* \since QGIS 3.16
*/
virtual QgsFields fields( const QString &schema, const QString &table ) const SIP_THROW( QgsProviderConnectionException );
Expand Down
22 changes: 20 additions & 2 deletions src/providers/spatialite/qgsspatialiteproviderconnection.cpp
Expand Up @@ -319,15 +319,17 @@ QList<QgsSpatiaLiteProviderConnection::TableProperty> QgsSpatiaLiteProviderConne

// Need to store it here because provider (and underlying gaia library) returns views as spatial table if they have geometries
QStringList viewNames;
for ( const auto &tn : executeSqlPrivate( QStringLiteral( "SELECT name FROM sqlite_master WHERE type = 'view'" ) ).rows() )
const auto viewRows { executeSqlPrivate( QStringLiteral( "SELECT name FROM sqlite_master WHERE type = 'view'" ) ).rows() };
for ( const auto &tn : std::as_const( viewRows ) )
{
viewNames.push_back( tn.first().toString() );
}

// Another weirdness: table names are converted to lowercase when out of spatialite gaia functions, let's get them back to their real case here,
// may need LAUNDER on open, but let's try to make it consistent with how GPKG works.
QgsStringMap tableNotLowercaseNames;
for ( const auto &tn : executeSqlPrivate( QStringLiteral( "SELECT name FROM sqlite_master WHERE LOWER(name) != name" ) ).rows() )
const auto lowerTables { executeSqlPrivate( QStringLiteral( "SELECT name FROM sqlite_master WHERE LOWER(name) != name" ) ).rows() };
for ( const auto &tn : std::as_const( lowerTables ) )
{
const QString tName { tn.first().toString() };
tableNotLowercaseNames.insert( tName.toLower(), tName );
Expand Down Expand Up @@ -357,10 +359,26 @@ QList<QgsSpatiaLiteProviderConnection::TableProperty> QgsSpatiaLiteProviderConne
property.setGeometryColumnTypes( {{ QgsWkbTypes::NoGeometry, QgsCoordinateReferenceSystem() }} );
property.setFlag( QgsSpatiaLiteProviderConnection::TableFlag::Aspatial );
}

if ( viewNames.contains( tableName ) )
{
property.setFlag( QgsSpatiaLiteProviderConnection::TableFlag::View );
}

const auto constPkIdxs { vl->dataProvider()->pkAttributeIndexes() };
QStringList pkNames;
for ( const auto &pkIdx : std::as_const( constPkIdxs ) )
{
// Better safe than sorry
if ( pkIdx < vl->fields().count() )
pkNames.append( vl->fields().at( pkIdx ).name() );
}

if ( ! pkNames.isEmpty() )
{
property.setPrimaryKeyColumns( pkNames );
}

tableInfo.push_back( property );
}
else
Expand Down
41 changes: 32 additions & 9 deletions tests/src/python/test_qgsproviderconnection_ogr_gpkg.py
Expand Up @@ -16,6 +16,7 @@
import os
import shutil
from test_qgsproviderconnection_base import TestPyQgsProviderConnectionBase
from qgis.PyQt.QtCore import QTemporaryDir
from qgis.core import (
QgsWkbTypes,
QgsAbstractDatabaseProviderConnection,
Expand Down Expand Up @@ -56,17 +57,13 @@ def setUpClass(cls):
"""Run before all tests"""
TestPyQgsProviderConnectionBase.setUpClass()
gpkg_original_path = '{}/qgis_server/test_project_wms_grouped_layers.gpkg'.format(TEST_DATA_DIR)
cls.gpkg_path = '{}/qgis_server/test_project_wms_grouped_layers_test.gpkg'.format(TEST_DATA_DIR)
cls.temp_dir = QTemporaryDir()
cls.gpkg_path = '{}/test_project_wms_grouped_layers.gpkg'.format(cls.temp_dir.path())
shutil.copy(gpkg_original_path, cls.gpkg_path)
vl = QgsVectorLayer('{}|layername=cdb_lines'.format(cls.gpkg_path), 'test', 'ogr')
assert vl.isValid()
cls.uri = cls.gpkg_path

@classmethod
def tearDownClass(cls):
"""Run after all tests"""
os.unlink(cls.gpkg_path)

def test_gpkg_connections_from_uri(self):
"""Create a connection from a layer uri and retrieve it"""

Expand All @@ -85,8 +82,8 @@ def test_gpkg_table_uri(self):
self.assertTrue(vl.isValid())

# Test table(), throws if not found
table_info = conn.table('', 'osm')
table_info = conn.table('', 'cdb_lines')
conn.table('', 'osm')
conn.table('', 'cdb_lines')

self.assertEqual(conn.tableUri('', 'osm'), "GPKG:%s:osm" % self.uri)
rl = QgsRasterLayer(conn.tableUri('', 'osm'), 'r', 'gdal')
Expand Down Expand Up @@ -136,7 +133,10 @@ def test_gpkg_fields(self):
md = QgsProviderRegistry.instance().providerMetadata('ogr')
conn = md.createConnection(self.uri, {})
fields = conn.fields('', 'cdb_lines')
self.assertEqual(fields.names(), ['fid', 'id', 'typ', 'name', 'ortsrat', 'id_long'])
table_info = conn.table('', 'cdb_lines')
self.assertIn(table_info.geometryColumn(), fields.names())
self.assertIn(table_info.primaryKeyColumns()[0], fields.names())
self.assertEqual(fields.names(), ['fid', 'id', 'typ', 'name', 'ortsrat', 'id_long', 'geom'])

def test_create_vector_layer(self):
"""Test query layers"""
Expand All @@ -153,6 +153,29 @@ def test_create_vector_layer(self):
self.assertEqual(len(features), 2)
self.assertEqual(features[0].attributes(), [8, 'Sülfeld'])

def test_execute_sql_pk_geoms(self):
"""OGR hides fid and geom from attributes, check if we can still get them"""

md = QgsProviderRegistry.instance().providerMetadata('ogr')
conn = md.createConnection(self.uri, {})

# Check errors
with self.assertRaises(QgsProviderConnectionException):
sql = 'SELECT not_exists, name, geom FROM cdb_lines WHERE name LIKE \'S%\' LIMIT 2'
results = conn.executeSql(sql)

sql = 'SELECT fid, name, geom FROM cdb_lines WHERE name LIKE \'S%\' LIMIT 2'
results = conn.executeSql(sql)
self.assertEqual(results[0][:2], [8, 'Sülfeld'])
self.assertEqual(results[1][:2], [16, 'Steimker Berg'])
self.assertEqual(results[0][2][:20], b'\x01\x03\x00\x00\x00\x01\x00\x00\x00/\x00\x00\x00\xf6\x88\x16Y\xad\xb2"')
self.assertEqual(results[1][2][:20], b'\x01\x03\x00\x00\x00\x01\x00\x00\x00F\x00\x00\x00 \xc1\x9f\xda\xb4\xfb"')

sql = 'SELECT name, st_astext(geom) FROM cdb_lines WHERE name LIKE \'S%\' LIMIT 2'
results = conn.executeSql(sql)
self.assertEqual(results[0], ['Sülfeld',
'POLYGON((612694.674 5807839.658, 612668.715 5808176.815, 612547.354 5808414.452, 612509.527 5808425.73, 612522.932 5808473.02, 612407.901 5808519.082, 612505.836 5808632.763, 612463.449 5808781.115, 612433.57 5808819.061, 612422.685 5808980.281999, 612473.423 5808995.424999, 612333.856 5809647.731, 612307.316 5809781.446, 612267.099 5809852.803, 612308.221 5810040.995, 613920.397 5811079.478, 613947.16 5811129.3, 614022.726 5811154.456, 614058.436 5811260.36, 614194.037 5811331.972, 614307.176 5811360.06, 614343.842 5811323.238, 614443.449 5811363.03, 614526.199 5811059.031, 614417.83 5811057.603, 614787.296 5809648.422, 614772.062 5809583.246, 614981.93 5809245.35, 614811.885 5809138.271, 615063.452 5809100.954, 615215.476 5809029.413, 615469.441 5808883.282, 615569.846 5808829.522, 615577.239 5808806.242, 615392.964 5808736.873, 615306.34 5808662.171, 615335.445 5808290.588, 615312.192 5808290.397, 614890.582 5808077.956, 615018.854 5807799.895, 614837.326 5807688.363, 614435.698 5807646.847, 614126.351 5807661.841, 613555.813 5807814.801, 612826.66 5807964.828, 612830.113 5807856.315, 612694.674 5807839.658))'])


if __name__ == '__main__':
unittest.main()
2 changes: 1 addition & 1 deletion tests/src/python/test_qgsproviderconnection_postgres.py
Expand Up @@ -339,7 +339,7 @@ def test_fields(self):

conn.executeSql(sql)
fields = conn.fields('qgis_test', 'gh_37666')
self.assertEqual([f.name() for f in fields], ['id', 'geom', 'geog'])
self.assertEqual(fields.names(), ['id', 'geom', 'geog'])
self.assertEqual([f.typeName() for f in fields], ['int4', 'geometry', 'geography'])
table = conn.table('qgis_test', 'gh_37666')
self.assertEqual(table.primaryKeyColumns(), ['id'])
Expand Down

0 comments on commit 34c5cdc

Please sign in to comment.