Fix resolving geometry types when invalid geometries present

This commit is contained in:
Nyall Dawson 2025-02-20 08:41:17 +10:00 committed by github-actions[bot]
parent 4dd7d7e969
commit 2116c3dc46
9 changed files with 225 additions and 29 deletions

View File

@ -234,7 +234,7 @@ QVector<QgsDataItem *> 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 );

View File

@ -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>( "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<QgsMssqlDatabase> 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' );

View File

@ -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<QgsMssqlLayerProperty> 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<QgsMssqlLayerProperty> layerProperties;
};

View File

@ -354,6 +354,7 @@ QList<QgsMssqlProviderConnection::TableProperty> 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::TableProperty> 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() };

View File

@ -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 );

View File

@ -121,7 +121,7 @@ class QgsMssqlSourceSelect : public QgsAbstractDbSourceSelect
typedef QList<geomPair> 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.

View File

@ -31,9 +31,9 @@ struct QgsMssqlLayerProperty
QString geometryColName;
QStringList pkCols;
QString srid;
bool isGeography;
bool isGeography = false;
QString sql;
bool isView;
bool isView = false;
};

View File

@ -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<QgsMssqlLayerProperty> 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<QgsMssqlLayerProperty> 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<QgsMssqlLayerProperty> 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<QgsMssqlLayerProperty> 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"

View File

@ -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()