diff --git a/src/providers/mssql/qgsmssqldataitems.cpp b/src/providers/mssql/qgsmssqldataitems.cpp index 027ca1c00e7..1eda011c991 100644 --- a/src/providers/mssql/qgsmssqldataitems.cpp +++ b/src/providers/mssql/qgsmssqldataitems.cpp @@ -234,7 +234,7 @@ QVector QgsMssqlConnectionItem::createChildren() { if ( !mColumnTypeThread ) { - mColumnTypeThread = new QgsMssqlGeomColumnTypeThread( mService, mHost, mDatabase, mUsername, mPassword, true /* use estimated metadata */ ); + mColumnTypeThread = new QgsMssqlGeomColumnTypeThread( mService, mHost, mDatabase, mUsername, mPassword, true /* use estimated metadata */, disableInvalidGeometryHandling ); connect( mColumnTypeThread, &QgsMssqlGeomColumnTypeThread::setLayerType, this, &QgsMssqlConnectionItem::setLayerType ); connect( this, &QgsMssqlConnectionItem::addGeometryColumn, mColumnTypeThread, &QgsMssqlGeomColumnTypeThread::addGeometryColumn ); diff --git a/src/providers/mssql/qgsmssqlgeomcolumntypethread.cpp b/src/providers/mssql/qgsmssqlgeomcolumntypethread.cpp index fd5e927adbb..3e9a8f0710b 100644 --- a/src/providers/mssql/qgsmssqlgeomcolumntypethread.cpp +++ b/src/providers/mssql/qgsmssqlgeomcolumntypethread.cpp @@ -22,14 +22,14 @@ #include "qgsmssqlprovider.h" #include "qgsmssqldatabase.h" -QgsMssqlGeomColumnTypeThread::QgsMssqlGeomColumnTypeThread( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, bool useEstimatedMetadata ) +QgsMssqlGeomColumnTypeThread::QgsMssqlGeomColumnTypeThread( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, bool useEstimatedMetadata, bool disableInvalidGeometryHandling ) : mService( service ) , mHost( host ) , mDatabase( database ) , mUsername( username ) , mPassword( password ) , mUseEstimatedMetadata( useEstimatedMetadata ) - , mStopped( false ) + , mDisableInvalidGeometryHandling( disableInvalidGeometryHandling ) { qRegisterMetaType( "QgsMssqlLayerProperty" ); } @@ -59,15 +59,31 @@ void QgsMssqlGeomColumnTypeThread::run() const QString table = QStringLiteral( "%1[%2]" ) .arg( layerProperty.schemaName.isEmpty() ? QString() : QStringLiteral( "[%1]." ).arg( layerProperty.schemaName ), layerProperty.tableName ); - const QString query = QStringLiteral( "SELECT %3" - " UPPER([%1].STGeometryType())," - " [%1].STSrid," - " [%1].HasZ," - " [%1].HasM" - " FROM %2" - " WHERE [%1] IS NOT NULL %4" - " GROUP BY [%1].STGeometryType(), [%1].STSrid, [%1].HasZ, [%1].HasM" ) - .arg( layerProperty.geometryColName, table, mUseEstimatedMetadata ? "TOP 1" : "", layerProperty.sql.isEmpty() ? QString() : QStringLiteral( " AND %1" ).arg( layerProperty.sql ) ); + QString query; + if ( mDisableInvalidGeometryHandling ) + { + query = QStringLiteral( "SELECT %3" + " UPPER([%1].STGeometryType())," + " [%1].STSrid," + " [%1].HasZ," + " [%1].HasM" + " FROM %2" + " WHERE [%1] IS NOT NULL %4" + " GROUP BY [%1].STGeometryType(), [%1].STSrid, [%1].HasZ, [%1].HasM" ) + .arg( layerProperty.geometryColName, table, mUseEstimatedMetadata ? "TOP 1" : "", layerProperty.sql.isEmpty() ? QString() : QStringLiteral( " AND %1" ).arg( layerProperty.sql ) ); + } + else + { + query = QStringLiteral( + R"raw( + SELECT type, srid, hasz, hasm FROM + (SELECT %3 UPPER((CASE WHEN [%1].STIsValid() = 0 THEN [%1].MakeValid() ELSE [%1] END).STGeometryType()) as type, + [%1].STSrid as srid, [%1].HasZ as hasz, [%1].HasM as hasm FROM %2 WHERE [%1] IS NOT NULL %4) AS a + GROUP BY type, srid, hasz, hasm + )raw" + ) + .arg( layerProperty.geometryColName, table, mUseEstimatedMetadata ? "TOP 1" : "", layerProperty.sql.isEmpty() ? QString() : QStringLiteral( " AND %1" ).arg( layerProperty.sql ) ); + } // issue the sql query std::shared_ptr db = QgsMssqlDatabase::connectDb( mService, mHost, mDatabase, mUsername, mPassword ); @@ -98,6 +114,9 @@ void QgsMssqlGeomColumnTypeThread::run() const bool hasM { q.value( 3 ).toString() == '1' }; const int dimensions { 2 + ( ( hasZ && hasM ) ? 2 : ( ( hasZ || hasM ) ? 1 : 0 ) ) }; QString typeName { q.value( 0 ).toString().toUpper() }; + if ( typeName.isEmpty() ) + continue; + if ( hasM && !typeName.endsWith( 'M' ) ) { typeName.append( 'M' ); diff --git a/src/providers/mssql/qgsmssqlgeomcolumntypethread.h b/src/providers/mssql/qgsmssqlgeomcolumntypethread.h index 6f169323877..d51d8d77e8a 100644 --- a/src/providers/mssql/qgsmssqlgeomcolumntypethread.h +++ b/src/providers/mssql/qgsmssqlgeomcolumntypethread.h @@ -27,12 +27,17 @@ class QgsMssqlGeomColumnTypeThread : public QThread { Q_OBJECT public: - QgsMssqlGeomColumnTypeThread( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, bool useEstimatedMetadata ); + QgsMssqlGeomColumnTypeThread( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, bool useEstimatedMetadata, bool disableInvalidGeometryHandling ); // These functions get the layer types and pass that information out // by emitting the setLayerType() signal. void run() override; + /** + * Returns the results of the run. + */ + QList results() const { return layerProperties; } + signals: void setLayerType( const QgsMssqlLayerProperty &layerProperty ); @@ -48,8 +53,9 @@ class QgsMssqlGeomColumnTypeThread : public QThread QString mDatabase; QString mUsername; QString mPassword; - bool mUseEstimatedMetadata; - bool mStopped; + bool mUseEstimatedMetadata = false; + bool mDisableInvalidGeometryHandling = false; + bool mStopped = false; QList layerProperties; }; diff --git a/src/providers/mssql/qgsmssqlproviderconnection.cpp b/src/providers/mssql/qgsmssqlproviderconnection.cpp index 5f2594ccfeb..4fdbd52a207 100644 --- a/src/providers/mssql/qgsmssqlproviderconnection.cpp +++ b/src/providers/mssql/qgsmssqlproviderconnection.cpp @@ -354,6 +354,7 @@ QList QgsMssqlProviderConnection::tab { allowGeometrylessTables = flags.testFlag( TableFlag::Aspatial ); } + const bool disableInvalidGeometryHandling = dsUri.hasParam( QStringLiteral( "disableInvalidGeometryHandling" ) ) && dsUri.param( QStringLiteral( "disableInvalidGeometryHandling" ) ).toInt(); QString query { QStringLiteral( "SELECT " ) }; @@ -422,18 +423,32 @@ QList QgsMssqlProviderConnection::tab if ( !table.geometryColumn().isEmpty() ) { // Fetch geom cols - const QString geomColSql { - QStringLiteral( R"raw( - SELECT %4 UPPER( %1.STGeometryType()), %1.STSrid, + QString geomColSql; + + if ( disableInvalidGeometryHandling ) + { + // this query will fail if the table contains invalid geometries + geomColSql = QStringLiteral( R"raw( +SELECT %4 UPPER( %1.STGeometryType()), %1.STSrid, %1.HasZ, %1.HasM FROM %2.%3 WHERE %1 IS NOT NULL GROUP BY %1.STGeometryType(), %1.STSrid, %1.HasZ, %1.HasM - )raw" ) - .arg( QgsMssqlProvider::quotedIdentifier( table.geometryColumn() ), QgsMssqlProvider::quotedIdentifier( table.schema() ), QgsMssqlProvider::quotedIdentifier( table.tableName() ), useEstimatedMetadata ? "TOP 1" : "" ) - }; - // This may fail for invalid geometries + )raw" ) + .arg( QgsMssqlProvider::quotedIdentifier( table.geometryColumn() ), QgsMssqlProvider::quotedIdentifier( table.schema() ), QgsMssqlProvider::quotedIdentifier( table.tableName() ), useEstimatedMetadata ? "TOP 1" : "" ); + } + else + { + geomColSql = QStringLiteral( R"raw( + SELECT type, srid, hasz, hasm FROM + (SELECT %4 UPPER((CASE WHEN %1.STIsValid() = 0 THEN %1.MakeValid() ELSE %1 END).STGeometryType()) as type, + %1.STSrid as srid, %1.HasZ as hasz, %1.HasM as hasm FROM %2.%3 WHERE %1 IS NOT NULL) AS a + GROUP BY type, srid, hasz, hasm + )raw" ) + .arg( QgsMssqlProvider::quotedIdentifier( table.geometryColumn() ), QgsMssqlProvider::quotedIdentifier( table.schema() ), QgsMssqlProvider::quotedIdentifier( table.tableName() ), useEstimatedMetadata ? "TOP 1" : "" ); + } + try { const auto geomColResults { executeSqlPrivate( geomColSql ).rows() }; diff --git a/src/providers/mssql/qgsmssqlsourceselect.cpp b/src/providers/mssql/qgsmssqlsourceselect.cpp index c1a977d32a9..6c45e249ea4 100644 --- a/src/providers/mssql/qgsmssqlsourceselect.cpp +++ b/src/providers/mssql/qgsmssqlsourceselect.cpp @@ -384,6 +384,7 @@ void QgsMssqlSourceSelect::btnConnect_clicked() bool useGeometryColumns = QgsMssqlConnection::geometryColumnsOnly( cmbConnections->currentText() ); const bool allowGeometrylessTables = cbxAllowGeometrylessTables->isChecked(); const bool estimateMetadata = QgsMssqlConnection::useEstimatedMetadata( cmbConnections->currentText() ); + const bool disableInvalidGeometryHandling = QgsMssqlConnection::isInvalidGeometryHandlingDisabled( cmbConnections->currentText() ); mConnInfo = "dbname='" + database + '\''; if ( !host.isEmpty() ) @@ -455,7 +456,7 @@ void QgsMssqlSourceSelect::btnConnect_clicked() { if ( type == QLatin1String( "GEOMETRY" ) || type.isNull() || srid.isEmpty() ) { - addSearchGeometryColumn( service, host, database, username, password, layer, estimateMetadata ); + addSearchGeometryColumn( service, host, database, username, password, layer, estimateMetadata, disableInvalidGeometryHandling ); type.clear(); srid.clear(); } @@ -562,12 +563,12 @@ void QgsMssqlSourceSelect::setSql( const QModelIndex &index ) } } -void QgsMssqlSourceSelect::addSearchGeometryColumn( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, const QgsMssqlLayerProperty &layerProperty, bool estimateMetadata ) +void QgsMssqlSourceSelect::addSearchGeometryColumn( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, const QgsMssqlLayerProperty &layerProperty, bool estimateMetadata, bool disableInvalidGeometryHandling ) { // store the column details and do the query in a thread if ( !mColumnTypeThread ) { - mColumnTypeThread = new QgsMssqlGeomColumnTypeThread( service, host, database, username, password, estimateMetadata ); + mColumnTypeThread = new QgsMssqlGeomColumnTypeThread( service, host, database, username, password, estimateMetadata, disableInvalidGeometryHandling ); connect( mColumnTypeThread, &QgsMssqlGeomColumnTypeThread::setLayerType, this, &QgsMssqlSourceSelect::setLayerType ); connect( this, &QgsMssqlSourceSelect::addGeometryColumn, mColumnTypeThread, &QgsMssqlGeomColumnTypeThread::addGeometryColumn ); diff --git a/src/providers/mssql/qgsmssqlsourceselect.h b/src/providers/mssql/qgsmssqlsourceselect.h index 137e1a83e54..4e46964dcbe 100644 --- a/src/providers/mssql/qgsmssqlsourceselect.h +++ b/src/providers/mssql/qgsmssqlsourceselect.h @@ -121,7 +121,7 @@ class QgsMssqlSourceSelect : public QgsAbstractDbSourceSelect typedef QList geomCol; // queue another query for the thread - void addSearchGeometryColumn( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, const QgsMssqlLayerProperty &layerProperty, bool estimateMetadata ); + void addSearchGeometryColumn( const QString &service, const QString &host, const QString &database, const QString &username, const QString &password, const QgsMssqlLayerProperty &layerProperty, bool estimateMetadata, bool disableInvalidGeometryHandling ); // Set the position of the database connection list to the last // used one. diff --git a/src/providers/mssql/qgsmssqltablemodel.h b/src/providers/mssql/qgsmssqltablemodel.h index 79e17fdf3d3..27a5bd31087 100644 --- a/src/providers/mssql/qgsmssqltablemodel.h +++ b/src/providers/mssql/qgsmssqltablemodel.h @@ -31,9 +31,9 @@ struct QgsMssqlLayerProperty QString geometryColName; QStringList pkCols; QString srid; - bool isGeography; + bool isGeography = false; QString sql; - bool isView; + bool isView = false; }; diff --git a/tests/src/providers/testqgsmssqlprovider.cpp b/tests/src/providers/testqgsmssqlprovider.cpp index 03357342b01..f5cc91a29d0 100644 --- a/tests/src/providers/testqgsmssqlprovider.cpp +++ b/tests/src/providers/testqgsmssqlprovider.cpp @@ -26,8 +26,8 @@ #include "qgsmssqltransaction.h" #include "qgsmssqlconnection.h" #include "qgsvectorlayer.h" -//#include "qgsmssqldatabase.h" #include "qgsproject.h" +#include "qgsmssqlgeomcolumntypethread.h" /** * \ingroup UnitTests @@ -49,6 +49,10 @@ class TestQgsMssqlProvider : public QObject void transactionTwoLayers(); void transactionUndoRedo(); + void testGeomTypeResolutionValid(); + void testGeomTypeResolutionValidNoWorkaround(); + void testGeomTypeResolutionInvalid(); + void testGeomTypeResolutionInvalidNoWorkaround(); private: QString mDbConn; @@ -283,5 +287,120 @@ void TestQgsMssqlProvider::transactionUndoRedo() // 2. with transaction, try to add a feature to the first layer -> both layers are affected } +void TestQgsMssqlProvider::testGeomTypeResolutionValid() +{ + QgsDataSourceUri uri( mDbConn ); + + // with invalid geometry handling workaround: + QgsMssqlGeomColumnTypeThread thread( uri.service(), uri.host(), uri.database(), uri.username(), uri.password(), false, false ); + + QgsMssqlLayerProperty layerProperty; + layerProperty.schemaName = QStringLiteral( "qgis_test" ); + layerProperty.tableName = QStringLiteral( "someData" ); + layerProperty.geometryColName = QStringLiteral( "geom" ); + thread.addGeometryColumn( layerProperty ); + + thread.run(); + const QList results = thread.results(); + QCOMPARE( results.size(), 1 ); + const QgsMssqlLayerProperty result = results.at( 0 ); + QCOMPARE( result.type, QStringLiteral( "POINT" ) ); + QCOMPARE( result.schemaName, QStringLiteral( "qgis_test" ) ); + QCOMPARE( result.tableName, QStringLiteral( "someData" ) ); + QCOMPARE( result.geometryColName, QStringLiteral( "geom" ) ); + QCOMPARE( result.pkCols.size(), 0 ); + QCOMPARE( result.srid, QStringLiteral( "4326" ) ); + QCOMPARE( result.isGeography, false ); + QCOMPARE( result.sql, QString() ); + QCOMPARE( result.isView, false ); +} + +void TestQgsMssqlProvider::testGeomTypeResolutionValidNoWorkaround() +{ + QgsDataSourceUri uri( mDbConn ); + + // WITHOUT invalid geometry handling workaround: + QgsMssqlGeomColumnTypeThread thread( uri.service(), uri.host(), uri.database(), uri.username(), uri.password(), false, true ); + + QgsMssqlLayerProperty layerProperty; + layerProperty.schemaName = QStringLiteral( "qgis_test" ); + layerProperty.tableName = QStringLiteral( "someData" ); + layerProperty.geometryColName = QStringLiteral( "geom" ); + thread.addGeometryColumn( layerProperty ); + + thread.run(); + const QList results = thread.results(); + QCOMPARE( results.size(), 1 ); + const QgsMssqlLayerProperty result = results.at( 0 ); + QCOMPARE( result.type, QStringLiteral( "POINT" ) ); + QCOMPARE( result.schemaName, QStringLiteral( "qgis_test" ) ); + QCOMPARE( result.tableName, QStringLiteral( "someData" ) ); + QCOMPARE( result.geometryColName, QStringLiteral( "geom" ) ); + QCOMPARE( result.pkCols.size(), 0 ); + QCOMPARE( result.srid, QStringLiteral( "4326" ) ); + QCOMPARE( result.isGeography, false ); + QCOMPARE( result.sql, QString() ); + QCOMPARE( result.isView, false ); +} + + +void TestQgsMssqlProvider::testGeomTypeResolutionInvalid() +{ + QgsDataSourceUri uri( mDbConn ); + + // with invalid geometry handling workaround: + QgsMssqlGeomColumnTypeThread thread( uri.service(), uri.host(), uri.database(), uri.username(), uri.password(), false, false ); + + QgsMssqlLayerProperty layerProperty; + layerProperty.schemaName = QStringLiteral( "qgis_test" ); + layerProperty.tableName = QStringLiteral( "invalid_polys" ); + layerProperty.geometryColName = QStringLiteral( "ogr_geometry" ); + thread.addGeometryColumn( layerProperty ); + + thread.run(); + const QList results = thread.results(); + QCOMPARE( results.size(), 1 ); + const QgsMssqlLayerProperty result = results.at( 0 ); + QCOMPARE( result.type, QStringLiteral( "MULTIPOLYGON,POLYGON" ) ); + QCOMPARE( result.schemaName, QStringLiteral( "qgis_test" ) ); + QCOMPARE( result.tableName, QStringLiteral( "invalid_polys" ) ); + QCOMPARE( result.geometryColName, QStringLiteral( "ogr_geometry" ) ); + QCOMPARE( result.pkCols.size(), 0 ); + QCOMPARE( result.srid, QStringLiteral( "4167,4167" ) ); + QCOMPARE( result.isGeography, false ); + QCOMPARE( result.sql, QString() ); + QCOMPARE( result.isView, false ); +} + +void TestQgsMssqlProvider::testGeomTypeResolutionInvalidNoWorkaround() +{ + QgsDataSourceUri uri( mDbConn ); + + // WITHOUT invalid geometry handling workaround. + // We expect this to fail + QgsMssqlGeomColumnTypeThread thread( uri.service(), uri.host(), uri.database(), uri.username(), uri.password(), false, true ); + + QgsMssqlLayerProperty layerProperty; + layerProperty.schemaName = QStringLiteral( "qgis_test" ); + layerProperty.tableName = QStringLiteral( "invalid_polys" ); + layerProperty.geometryColName = QStringLiteral( "ogr_geometry" ); + thread.addGeometryColumn( layerProperty ); + + thread.run(); + const QList results = thread.results(); + QCOMPARE( results.size(), 1 ); + const QgsMssqlLayerProperty result = results.at( 0 ); + // geometry type resolution will fail because of unhandled exception raised by SQL Server + QCOMPARE( result.type, QString() ); + QCOMPARE( result.schemaName, QStringLiteral( "qgis_test" ) ); + QCOMPARE( result.tableName, QStringLiteral( "invalid_polys" ) ); + QCOMPARE( result.geometryColName, QStringLiteral( "ogr_geometry" ) ); + QCOMPARE( result.pkCols.size(), 0 ); + QCOMPARE( result.srid, QString() ); + QCOMPARE( result.isGeography, false ); + QCOMPARE( result.sql, QString() ); + QCOMPARE( result.isView, false ); +} + QGSTEST_MAIN( TestQgsMssqlProvider ) #include "testqgsmssqlprovider.moc" diff --git a/tests/src/python/test_qgsproviderconnection_mssql.py b/tests/src/python/test_qgsproviderconnection_mssql.py index 3a893cd6150..d3ce88df559 100644 --- a/tests/src/python/test_qgsproviderconnection_mssql.py +++ b/tests/src/python/test_qgsproviderconnection_mssql.py @@ -206,6 +206,42 @@ class TestPyQgsProviderConnectionMssql( conn.dropVectorTable("qgis_test", "test_zm") + def test_invalid_geometries(self): + """Test retrieving details for a table containing invalid geometries""" + + md = QgsProviderRegistry.instance().providerMetadata("mssql") + conn = md.createConnection(self.uri, {}) + + tb = conn.table("qgis_test", "invalid_polys") + self.assertEqual(tb.tableName(), "invalid_polys") + self.assertEqual(tb.schema(), "qgis_test") + self.assertEqual(tb.geometryColumn(), "ogr_geometry") + self.assertEqual(tb.primaryKeyColumns(), []) + self.assertEqual(len(tb.geometryColumnTypes()), 1) + gct = tb.geometryColumnTypes()[0] + self.assertEqual(gct.wkbType, Qgis.WkbType.MultiPolygon) + self.assertEqual(tb.maxCoordinateDimensions(), 2) + self.assertEqual(gct.crs, QgsCoordinateReferenceSystem("EPSG:4167")) + + def test_invalid_geometries_disable_workaround(self): + """Test retrieving details for a table containing invalid geometries, + with invalid geometry workarounds disabled + """ + + md = QgsProviderRegistry.instance().providerMetadata("mssql") + uri = QgsDataSourceUri(self.uri) + uri.setParam("disableInvalidGeometryHandling", "1") + conn = md.createConnection(uri.uri(), {}) + + tb = conn.table("qgis_test", "invalid_polys") + self.assertEqual(tb.tableName(), "invalid_polys") + self.assertEqual(tb.schema(), "qgis_test") + self.assertEqual(tb.geometryColumn(), "ogr_geometry") + self.assertEqual(tb.primaryKeyColumns(), []) + # we can't retrieve this, as SQL server will have raised an exception + # due to the invalid geometries in this table + self.assertEqual(len(tb.geometryColumnTypes()), 0) + if __name__ == "__main__": unittest.main()