From 1560d9aea89498afa2c2488a2a5351db34ff55db Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 15 Sep 2020 11:07:50 +0200 Subject: [PATCH 1/4] PG unrestricted geometry: trust SRID from geometry_columns Fixes #38567 where the SRID was not detected if the table was empty. Also: do not override the SRID passed explicitly in the data source URI. Note: the logic in the provider is becoming a bit convoluted, there are too many corner cases, at least the test coverage is pretty good. --- src/providers/postgres/qgspostgresprovider.cpp | 11 ++++++++--- tests/src/python/test_provider_postgres.py | 12 ++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index a7ea9c00616d..2c9094a23496 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -3874,6 +3874,12 @@ bool QgsPostgresProvider::getGeometryDetails() { detectedType = result.PQgetvalue( 0, 0 ); + // Do not override the SRID if set in the data source URI + if ( detectedSrid.isEmpty() ) + { + detectedSrid = result.PQgetvalue( 0, 1 ); + } + QString dim = result.PQgetvalue( 0, 2 ); if ( dim == QLatin1String( "3" ) && !detectedType.endsWith( 'M' ) ) detectedType += QLatin1String( "Z" ); @@ -4047,7 +4053,6 @@ bool QgsPostgresProvider::getGeometryDetails() if ( mDetectedGeomType == QgsWkbTypes::Unknown ) { - mDetectedSrid.clear(); QgsPostgresLayerProperty layerProperty; if ( !mIsQuery ) @@ -4077,8 +4082,8 @@ bool QgsPostgresProvider::getGeometryDetails() if ( layerProperty.size() == 0 ) { - // no data - so take what's requested - if ( mRequestedGeomType == QgsWkbTypes::Unknown || mRequestedSrid.isEmpty() ) + // no data - so take what's requested/detected + if ( mRequestedGeomType == QgsWkbTypes::Unknown || mDetectedSrid.isEmpty() ) { QgsMessageLog::logMessage( tr( "Geometry type and srid for empty column %1 of %2 undefined." ).arg( mGeometryColumn, mQuery ) ); } diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py index 9e058e5e5f0f..7086fdf81868 100644 --- a/tests/src/python/test_provider_postgres.py +++ b/tests/src/python/test_provider_postgres.py @@ -2972,6 +2972,18 @@ def testUnrestrictedGeometryType(self): with self.assertRaises(StopIteration): next(polygons.getFeatures()) + # Test regression GH #38567 (no SRID requested in the data source URI) + # Cleanup if needed + conn.executeSql('DELETE FROM "qgis_test"."test_unrestricted_geometry" WHERE \'t\'') + + points = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'gid\' type=POINT table="qgis_test"."test_unrestricted_geometry" (geom) sql=', 'test_points', 'postgres') + lines = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'gid\' type=LINESTRING table="qgis_test"."test_unrestricted_geometry" (geom) sql=', 'test_lines', 'postgres') + polygons = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'gid\' type=POLYGON table="qgis_test"."test_unrestricted_geometry" (geom) sql=', 'test_polygons', 'postgres') + + self.assertTrue(points.isValid()) + self.assertTrue(lines.isValid()) + self.assertTrue(polygons.isValid()) + if __name__ == '__main__': unittest.main() From 7094614f79c2a7ff674b4cea49f9dbc07b642790 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 15 Sep 2020 16:59:04 +0200 Subject: [PATCH 2/4] Typo --- src/providers/postgres/qgspostgresconn.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp index c6904e64e561..0335093d4b52 100644 --- a/src/providers/postgres/qgspostgresconn.cpp +++ b/src/providers/postgres/qgspostgresconn.cpp @@ -1848,7 +1848,7 @@ void QgsPostgresConn::retrieveLayerTypes( QVector &l } else // vectors { - // our estimatation ignores that a where clause might restrict the feature type or srid + // our estimation ignores that a where clause might restrict the feature type or srid if ( useEstimatedMetadata ) { table = QStringLiteral( "(SELECT %1 FROM %2 WHERE %3%1 IS NOT NULL LIMIT %4) AS t" ) From 194305b418aea3512b2040083f7499ab21d38699 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 15 Sep 2020 16:59:44 +0200 Subject: [PATCH 3/4] Do not set detected SRID from rquested --- src/providers/postgres/qgspostgresprovider.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index 2c9094a23496..078143481ac4 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -3857,7 +3857,7 @@ bool QgsPostgresProvider::getGeometryDetails() } QString detectedType; - QString detectedSrid = mRequestedSrid; + QString detectedSrid; if ( !schemaName.isEmpty() ) { // check geometry columns From 235864fbc357ad105e69817fe54eecc488acafa3 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 15 Sep 2020 19:46:42 +0200 Subject: [PATCH 4/4] Fix test data (add SRID) --- tests/testdata/provider/testdata_pg.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testdata/provider/testdata_pg.sql b/tests/testdata/provider/testdata_pg.sql index fc3d0e1fb122..3fb14de1974f 100644 --- a/tests/testdata/provider/testdata_pg.sql +++ b/tests/testdata/provider/testdata_pg.sql @@ -714,10 +714,10 @@ CREATE TABLE test_table_default_values ( CREATE TABLE IF NOT EXISTS "test_check_constraint" ( "id" SERIAL PRIMARY KEY, - "geom" geometry(POINT), + "geom" geometry(POINT, 4326), "i_will_fail_on_no_name" TEXT CHECK ("i_will_fail_on_no_name" != 'no name') ); INSERT INTO test_check_constraint (geom, i_will_fail_on_no_name) -VALUES ('POINT(9 45)'::geometry, 'I have a name'), ('POINT(10 46)'::geometry, 'I have a name too'); +VALUES ('SRID=4326;POINT(9 45)'::geometry, 'I have a name'), ('SRID=4326;POINT(10 46)'::geometry, 'I have a name too');