Skip to content

Commit

Permalink
Merge pull request #4642 from nyalldawson/mssql
Browse files Browse the repository at this point in the history
MSSQL provider fixes
  • Loading branch information
nyalldawson committed Jun 3, 2017
2 parents 6f42b78 + 62af54e commit 0b76352
Show file tree
Hide file tree
Showing 8 changed files with 264 additions and 16 deletions.
9 changes: 9 additions & 0 deletions src/providers/mssql/qgsmssqlexpressioncompiler.cpp
Expand Up @@ -77,3 +77,12 @@ QString QgsMssqlExpressionCompiler::quotedValue( const QVariant& value, bool& ok
return QgsSqlExpressionCompiler::quotedValue( value, ok );
}
}

QString QgsMssqlExpressionCompiler::quotedIdentifier( const QString &identifier )
{
QString quoted = identifier;
quoted.replace( '[', "[[" );
quoted.replace( ']', "]]" );
quoted = quoted.prepend( '[' ).append( ']' );
return quoted;
}
1 change: 1 addition & 0 deletions src/providers/mssql/qgsmssqlexpressioncompiler.h
Expand Up @@ -29,6 +29,7 @@ class QgsMssqlExpressionCompiler : public QgsSqlExpressionCompiler
protected:
virtual Result compileNode( const QgsExpression::Node* node, QString& result ) override;
virtual QString quotedValue( const QVariant& value, bool& ok ) override;
virtual QString quotedIdentifier( const QString& identifier ) override;

};

Expand Down
42 changes: 33 additions & 9 deletions src/providers/mssql/qgsmssqlfeatureiterator.cpp
Expand Up @@ -109,7 +109,7 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
mStatement += QString( ",[%1]" ).arg( mSource->mGeometryColName );
}

mStatement += QString( "FROM [%1].[%2]" ).arg( mSource->mSchemaName, mSource->mTableName );
mStatement += QString( " FROM [%1].[%2]" ).arg( mSource->mSchemaName, mSource->mTableName );

bool filterAdded = false;
// set spatial filter
Expand All @@ -127,7 +127,7 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
<< qgsDoubleToString( request.filterRect().xMinimum() ) << ' ' << qgsDoubleToString( request.filterRect().yMaximum() ) << ", "
<< qgsDoubleToString( request.filterRect().xMinimum() ) << ' ' << qgsDoubleToString( request.filterRect().yMinimum() );

mStatement += QString( " where [%1].STIsValid() = 1 AND [%1].STIntersects([%2]::STGeomFromText('POLYGON((%3))',%4)) = 1" ).arg(
mStatement += QString( " where [%1].STIsValid() = 1 AND [%1].Filter([%2]::STGeomFromText('POLYGON((%3))',%4)) = 1" ).arg(
mSource->mGeometryColName, mSource->mGeometryColType, r, QString::number( mSource->mSRId ) );
filterAdded = true;
}
Expand Down Expand Up @@ -247,7 +247,7 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
mOrderByCompiled = false;
}

if ( !mOrderByCompiled )
if ( !mOrderByCompiled && !request.orderBy().isEmpty() )
limitAtProvider = false;

if ( request.limit() >= 0 && limitAtProvider )
Expand Down Expand Up @@ -301,8 +301,25 @@ bool QgsMssqlFeatureIterator::fetchFeature( QgsFeature& feature )
{
QVariant v = mQuery->value( i );
const QgsField &fld = mSource->mFields.at( mAttributesToFetch.at( i ) );
if ( v.type() != fld.type() )

// special handling for time fields
if ( fld.type() == QVariant::Time && v.type() == QVariant::ByteArray )
{
QList<QByteArray> parts = v.toByteArray().split( '\0' );
if ( parts.count() >= 3 )
{
int hours = QString( parts.at( 0 ) ).at( 0 ).toAscii();
int minutes = QString( parts.at( 1 ) ).at( 0 ).toAscii();
int seconds = QString( parts.at( 2 ) ).at( 0 ).toAscii();
v = QTime( hours, minutes, seconds );
}
else
v = QgsVectorDataProvider::convertValue( fld.type(), v.toString() );
}
else if ( v.type() != fld.type() )
{
v = QgsVectorDataProvider::convertValue( fld.type(), v.toString() );
}
feature.setAttribute( mAttributesToFetch.at( i ), v );
}

Expand All @@ -311,12 +328,19 @@ bool QgsMssqlFeatureIterator::fetchFeature( QgsFeature& feature )
if ( mSource->isSpatial() )
{
QByteArray ar = mQuery->record().value( mSource->mGeometryColName ).toByteArray();
unsigned char* wkb = mParser.ParseSqlGeometry(( unsigned char* )ar.data(), ar.size() );
if ( wkb )
if ( !ar.isEmpty() )
{
QgsGeometry *g = new QgsGeometry();
g->fromWkb( wkb, mParser.GetWkbLen() );
feature.setGeometry( g );
unsigned char* wkb = mParser.ParseSqlGeometry(( unsigned char* )ar.data(), ar.size() );
if ( wkb )
{
QgsGeometry *g = new QgsGeometry();
g->fromWkb( wkb, mParser.GetWkbLen() );
feature.setGeometry( g );
}
else
{
feature.setGeometry( nullptr );
}
}
else
{
Expand Down
44 changes: 41 additions & 3 deletions src/providers/mssql/qgsmssqlprovider.cpp
Expand Up @@ -350,6 +350,11 @@ void QgsMssqlProvider::loadMetadata()
mSRId = 0;
mWkbType = QGis::WKBUnknown;

if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}

QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );
if ( !query.exec( QString( "select f_geometry_column, coord_dimension, srid, geometry_type from geometry_columns where f_table_schema = '%1' and f_table_name = '%2'" ).arg( mSchemaName, mTableName ) ) )
Expand All @@ -368,7 +373,12 @@ void QgsMssqlProvider::loadFields()
{
mAttributeFields.clear();
mDefaultValues.clear();

// get field spec
if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );
if ( !query.exec( QString( "exec sp_columns @table_name = N'%1', @table_owner = '%2'" ).arg( mTableName, mSchemaName ) ) )
Expand Down Expand Up @@ -532,6 +542,10 @@ QVariant QgsMssqlProvider::minimumValue( int index )
sql += QString( " where (%1)" ).arg( mSqlWhereClause );
}

if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );

Expand Down Expand Up @@ -563,6 +577,10 @@ QVariant QgsMssqlProvider::maximumValue( int index )
sql += QString( " where (%1)" ).arg( mSqlWhereClause );
}

if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );

Expand Down Expand Up @@ -603,6 +621,10 @@ void QgsMssqlProvider::uniqueValues( int index, QList<QVariant> &uniqueValues, i
sql += QString( " where (%1)" ).arg( mSqlWhereClause );
}

if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );

Expand Down Expand Up @@ -631,6 +653,10 @@ void QgsMssqlProvider::UpdateStatistics( bool estimate )
// get features to calculate the statistics
QString statement;

if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );

Expand Down Expand Up @@ -664,14 +690,14 @@ void QgsMssqlProvider::UpdateStatistics( bool estimate )
if ( estimate )
{
if ( mGeometryColType == "geometry" )
statement = QString( "select min([%1].MakeValid().STPointN(1).STX), min([%1].MakeValid().STPointN(1).STY), max([%1].MakeValid().STPointN(1).STX), max([%1].MakeValid().STPointN(1).STY)" ).arg( mGeometryColName );
statement = QString( "select min(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).STX else NULL end), min(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).STY else NULL end), max(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).STX else NULL end), max(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).STY else NULL end)" ).arg( mGeometryColName );
else
statement = QString( "select min([%1].MakeValid().STPointN(1).Long), min([%1].MakeValid().STPointN(1).Lat), max([%1].MakeValid().STPointN(1).Long), max([%1].MakeValid().STPointN(1).Lat)" ).arg( mGeometryColName );
statement = QString( "select min(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).Long else NULL end), min(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).Lat else NULL end), max(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).Long else NULL end), max(case when ([%1].STIsValid() = 1) THEN [%1].STPointN(1).Lat else NULL end)" ).arg( mGeometryColName );
}
else
{
if ( mGeometryColType == "geometry" )
statement = QString( "select min([%1].MakeValid().STEnvelope().STPointN(1).STX), min([%1].MakeValid().STEnvelope().STPointN(1).STY), max([%1].MakeValid().STEnvelope().STPointN(3).STX), max([%1].MakeValid().STEnvelope().STPointN(3).STY)" ).arg( mGeometryColName );
statement = QString( "select min(case when ([%1].STIsValid() = 1) THEN [%1].STEnvelope().STPointN(1).STX else NULL end), min(case when ([%1].STIsValid() = 1) THEN [%1].STEnvelope().STPointN(1).STY else NULL end), max(case when ([%1].STIsValid() = 1) THEN [%1].STEnvelope().STPointN(3).STX else NULL end), max(case when ([%1].STIsValid() = 1) THEN [%1].STEnvelope().STPointN(3).STY else NULL end)" ).arg( mGeometryColName );
else
{
statement = QString( "select [%1]" ).arg( mGeometryColName );
Expand Down Expand Up @@ -758,6 +784,10 @@ long QgsMssqlProvider::featureCount() const

// If there is no subset set we can get the count from the system tables.
// Which is faster then doing select count(*)
if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );

Expand Down Expand Up @@ -1419,6 +1449,10 @@ QgsCoordinateReferenceSystem QgsMssqlProvider::crs()
return mCrs;

// try to load crs from the database tables as a fallback
if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );
bool execOk = query.exec( QString( "select srtext from spatial_ref_sys where srid = %1" ).arg( QString::number( mSRId ) ) );
Expand Down Expand Up @@ -1472,6 +1506,10 @@ bool QgsMssqlProvider::setSubsetString( const QString& theSQL, bool )
sql += QString( " where %1" ).arg( mSqlWhereClause );
}

if ( !mDatabase.isOpen() )
{
mDatabase = GetDatabase( mService, mHost, mDatabaseName, mUserName, mPassword );
}
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );
if ( !query.exec( sql ) )
Expand Down
2 changes: 1 addition & 1 deletion src/providers/mssql/qgsmssqlprovider.h
Expand Up @@ -290,7 +290,7 @@ class QgsMssqlProvider : public QgsVectorDataProvider
QGis::WkbType mWkbType;

// The database object
QSqlDatabase mDatabase;
mutable QSqlDatabase mDatabase;

// The current sql query
QSqlQuery mQuery;
Expand Down
4 changes: 2 additions & 2 deletions src/providers/mssql/qgsmssqlsourceselect.cpp
Expand Up @@ -495,7 +495,7 @@ void QgsMssqlSourceSelect::on_btnConnect_clicked()

bool allowGeometrylessTables = cbxAllowGeometrylessTables->isChecked();

bool estimateMetadata = settings.value( key + "/estimatedMetadata", true ).toBool();
mUseEstimatedMetadata = settings.value( key + "/estimatedMetadata", true ).toBool();

mConnInfo = "dbname='" + database + '\'';
if ( !host.isEmpty() )
Expand Down Expand Up @@ -585,7 +585,7 @@ void QgsMssqlSourceSelect::on_btnConnect_clicked()
{
if ( type == "GEOMETRY" || type.isNull() || srid.isEmpty() )
{
addSearchGeometryColumn( connectionName, layer, estimateMetadata );
addSearchGeometryColumn( connectionName, layer, mUseEstimatedMetadata );
type = "";
srid = "";
}
Expand Down
75 changes: 74 additions & 1 deletion tests/src/python/test_provider_mssql.py
Expand Up @@ -16,7 +16,7 @@

import os

from qgis.core import QgsVectorLayer, QgsFeatureRequest
from qgis.core import QgsVectorLayer, QgsFeatureRequest, QgsRectangle

from qgis.PyQt.QtCore import QSettings, QDate, QTime, QDateTime, QVariant

Expand Down Expand Up @@ -56,6 +56,50 @@ def enableCompiler(self):
def disableCompiler(self):
QSettings().setValue(u'/qgis/compileExpressions', False)

def uncompiledFilters(self):
filters = set(['"name" NOT LIKE \'Ap%\'',
'"name" IS NULL',
'"name" IS NOT NULL',
'"name" NOT ILIKE \'QGIS\'',
'"name" NOT ILIKE \'pEAR\'',
'name <> \'Apple\'',
'"name" <> \'apple\'',
'(name = \'Apple\') is not null',
'"name" || \' \' || "cnt" = \'Orange 100\'',
'\'x\' || "name" IS NOT NULL',
'\'x\' || "name" IS NULL',
'"name" ~ \'[OP]ra[gne]+\'',
'false and NULL',
'true and NULL',
'NULL and false',
'NULL and true',
'NULL and NULL',
'false or NULL',
'true or NULL',
'NULL or false',
'NULL or true',
'NULL or NULL',
'not null',
'not name IS NULL',
'not name = \'Apple\'',
'not name = \'Apple\' or name = \'Apple\'',
'not name = \'Apple\' or not name = \'Apple\'',
'not name = \'Apple\' and pk = 4',
'not name = \'Apple\' and not pk = 4',
'intersects($geometry,geom_from_wkt( \'Polygon ((-72.2 66.1, -65.2 66.1, -65.2 72.0, -72.2 72.0, -72.2 66.1))\'))'])
return filters

def partiallyCompiledFilters(self):
return set(['name ILIKE \'QGIS\'',
'name = \'Apple\'',
'name = \'apple\'',
'name LIKE \'Apple\'',
'name LIKE \'aPple\'',
'"name"="name2"',
'name ILIKE \'aPple\'',
'name ILIKE \'%pp%\'',
'"name" || \' \' || "name" = \'Orange Orange\''])

# HERE GO THE PROVIDER SPECIFIC TESTS
def testDateTimeTypes(self):
vl = QgsVectorLayer('%s table="qgis_test"."date_times" sql=' %
Expand Down Expand Up @@ -83,5 +127,34 @@ def testDateTimeTypes(self):
self.assertEqual(f.attributes()[datetime_idx], QDateTime(
QDate(2004, 3, 4), QTime(13, 41, 52)))

def testInvalidGeometries(self):
""" Test what happens when SQL Server is a POS and throws an exception on encountering an invalid geometry """
vl = QgsVectorLayer('%s srid=4167 type=POLYGON table="qgis_test"."invalid_polys" (ogr_geometry) sql=' %
(self.dbconn), "testinvalid", "mssql")
assert(vl.isValid())

self.assertEqual(vl.dataProvider().extent().toString(1), QgsRectangle(173.953, -41.513, 173.967, -41.502).toString(1))

#burn through features - don't want SQL server to trip up on the invalid ones
count = 0
for f in vl.dataProvider().getFeatures():
count += 1
self.assertEqual(count, 39)

count = 0

for f in vl.dataProvider().getFeatures(QgsFeatureRequest(QgsRectangle(173, -42, 174, -41))):
count += 1
# two invalid geometry features
self.assertEqual(count, 37)
# sorry... you get NO chance to see these features exist and repair them... because SQL server. Use PostGIS instead and live a happier life!

# with estimated metadata
vl = QgsVectorLayer('%s srid=4167 type=POLYGON estimatedmetadata=true table="qgis_test"."invalid_polys" (ogr_geometry) sql=' %
(self.dbconn), "testinvalid", "mssql")
assert(vl.isValid())
self.assertEqual(vl.dataProvider().extent().toString(1), QgsRectangle(173.954, -41.513, 173.967, -41.502).toString(1))


if __name__ == '__main__':
unittest.main()

0 comments on commit 0b76352

Please sign in to comment.