Skip to content

Commit

Permalink
[server] Proper convertion of literals in Filters
Browse files Browse the repository at this point in the history 
Convert OGC Filter's literals accordings to field type.
This can have a huge impact on performance in some cases.
For example for a filter like "num_char" = '+2' converted to "num_char" = 2,
this result with PostgreSQL provider in a fallback to client side evaluation for the whole filter,
including the bbox if present.
  • Loading branch information
@arnaud-morvan
arnaud-morvan committed Jun 5, 2018
1 parent facf7a2 commit b07c334
Show file tree
Hide file tree
Showing 14 changed files with 126 additions and 62 deletions.
2 changes: 1 addition & 1 deletion python/core/auto_generated/qgsogcutils.sip.in
View file
Expand Up @@ -128,7 +128,7 @@ Exports the rectangle to GML3 Envelope
Parse XML with OGC fill into QColor
%End

static QgsExpression *expressionFromOgcFilter( const QDomElement &element ) /Factory/;
static QgsExpression *expressionFromOgcFilter( const QDomElement &element, QgsVectorLayer *layer = 0 ) /Factory/;
%Docstring
Parse XML with OGC filter into QGIS expression
%End
Expand Down
56 changes: 40 additions & 16 deletions src/core/qgsogcutils.cpp
View file
Expand Up @@ -1598,7 +1598,7 @@ QColor QgsOgcUtils::colorFromOgcFill( const QDomElement &fillElement )
}


QgsExpression *QgsOgcUtils::expressionFromOgcFilter( const QDomElement &element )
QgsExpression *QgsOgcUtils::expressionFromOgcFilter( const QDomElement &element, QgsVectorLayer *layer )
{
if ( element.isNull() || !element.hasChildNodes() )
return nullptr;
Expand All @@ -1619,7 +1619,7 @@ QgsExpression *QgsOgcUtils::expressionFromOgcFilter( const QDomElement &element
while ( !childElem.isNull() )
{
QString errorMsg;
QgsExpressionNode *node = nodeFromOgcFilter( childElem, errorMsg );
QgsExpressionNode *node = nodeFromOgcFilter( childElem, errorMsg, layer );
if ( !node )
{
// invalid expression, parser error
Expand Down Expand Up @@ -1702,15 +1702,15 @@ static bool isSpatialOperator( const QString &tagName )



QgsExpressionNode *QgsOgcUtils::nodeFromOgcFilter( QDomElement &element, QString &errorMessage )
QgsExpressionNode *QgsOgcUtils::nodeFromOgcFilter( QDomElement &element, QString &errorMessage, QgsVectorLayer *layer )
{
if ( element.isNull() )
return nullptr;

// check for binary operators
if ( isBinaryOperator( element.tagName() ) )
{
return nodeBinaryOperatorFromOgcFilter( element, errorMessage );
return nodeBinaryOperatorFromOgcFilter( element, errorMessage, layer );
}

// check for spatial operators
Expand All @@ -1731,7 +1731,7 @@ QgsExpressionNode *QgsOgcUtils::nodeFromOgcFilter( QDomElement &element, QString
}
else if ( element.tagName() == QLatin1String( "Literal" ) )
{
return nodeLiteralFromOgcFilter( element, errorMessage );
return nodeLiteralFromOgcFilter( element, errorMessage, layer );
}
else if ( element.tagName() == QLatin1String( "Function" ) )
{
Expand All @@ -1752,7 +1752,7 @@ QgsExpressionNode *QgsOgcUtils::nodeFromOgcFilter( QDomElement &element, QString



QgsExpressionNodeBinaryOperator *QgsOgcUtils::nodeBinaryOperatorFromOgcFilter( QDomElement &element, QString &errorMessage )
QgsExpressionNodeBinaryOperator *QgsOgcUtils::nodeBinaryOperatorFromOgcFilter( QDomElement &element, QString &errorMessage, QgsVectorLayer *layer )
{
if ( element.isNull() )
return nullptr;
Expand All @@ -1771,7 +1771,7 @@ QgsExpressionNodeBinaryOperator *QgsOgcUtils::nodeBinaryOperatorFromOgcFilter( Q
}

QDomElement operandElem = element.firstChildElement();
QgsExpressionNode *expr = nodeFromOgcFilter( operandElem, errorMessage ), *leftOp = expr;
QgsExpressionNode *expr = nodeFromOgcFilter( operandElem, errorMessage, layer ), *leftOp = expr;
if ( !expr )
{
if ( errorMessage.isEmpty() )
Expand All @@ -1781,7 +1781,7 @@ QgsExpressionNodeBinaryOperator *QgsOgcUtils::nodeBinaryOperatorFromOgcFilter( Q

for ( operandElem = operandElem.nextSiblingElement(); !operandElem.isNull(); operandElem = operandElem.nextSiblingElement() )
{
QgsExpressionNode *opRight = nodeFromOgcFilter( operandElem, errorMessage );
QgsExpressionNode *opRight = nodeFromOgcFilter( operandElem, errorMessage, layer );
if ( !opRight )
{
if ( errorMessage.isEmpty() )
Expand Down Expand Up @@ -1960,7 +1960,7 @@ QgsExpressionNodeFunction *QgsOgcUtils::nodeFunctionFromOgcFilter( QDomElement &



QgsExpressionNode *QgsOgcUtils::nodeLiteralFromOgcFilter( QDomElement &element, QString &errorMessage )
QgsExpressionNode *QgsOgcUtils::nodeLiteralFromOgcFilter( QDomElement &element, QString &errorMessage, QgsVectorLayer *layer )
{
if ( element.isNull() || element.tagName() != QLatin1String( "Literal" ) )
{
Expand All @@ -1980,7 +1980,7 @@ QgsExpressionNode *QgsOgcUtils::nodeLiteralFromOgcFilter( QDomElement &element,
{
// found a element node (e.g. PropertyName), convert it
QDomElement operandElem = childNode.toElement();
operand = nodeFromOgcFilter( operandElem, errorMessage );
operand = nodeFromOgcFilter( operandElem, errorMessage, layer );
if ( !operand )
{
delete root;
Expand All @@ -1994,12 +1994,36 @@ QgsExpressionNode *QgsOgcUtils::nodeLiteralFromOgcFilter( QDomElement &element,
// probably a text/CDATA node
QVariant value = childNode.nodeValue();

// try to convert the node content to number if possible,
// otherwise let's use it as string
bool ok;
double d = value.toDouble( &ok );
if ( ok )
value = d;
bool converted = false;

// try to convert the node content to corresponding field type if possible
if ( layer != nullptr )
{
QDomElement propertyNameElement = element.previousSiblingElement( QLatin1String( "PropertyName" ) );
if ( propertyNameElement.isNull() || propertyNameElement.tagName() != QLatin1String( "PropertyName" ) )
{
propertyNameElement = element.nextSiblingElement( QLatin1String( "PropertyName" ) );
}
if ( !propertyNameElement.isNull() || propertyNameElement.tagName() == QLatin1String( "PropertyName" ) )
{
int fieldIndex = layer->fields().indexOf( propertyNameElement.firstChild().nodeValue() );
if ( fieldIndex != -1 )
{
QgsField field = layer->fields().field( propertyNameElement.firstChild().nodeValue() );
field.convertCompatible( value );
converted = true;
}
}
}
if ( !converted )
{
// try to convert the node content to number if possible,
// otherwise let's use it as string
bool ok;
double d = value.toDouble( &ok );
if ( ok )
value = d;
}

operand = new QgsExpressionNodeLiteral( value );
if ( !operand )
Expand Down
9 changes: 5 additions & 4 deletions src/core/qgsogcutils.h
View file
Expand Up @@ -37,6 +37,7 @@ class QgsRectangle;
#include "qgsexpressionnode.h"
#include "qgsexpressionnodeimpl.h"
#include "qgssqlstatement.h"
#include "qgsvectorlayer.h"

/**
* \ingroup core
Expand Down Expand Up @@ -140,7 +141,7 @@ class CORE_EXPORT QgsOgcUtils
static QColor colorFromOgcFill( const QDomElement &fillElement );

//! Parse XML with OGC filter into QGIS expression
static QgsExpression *expressionFromOgcFilter( const QDomElement &element ) SIP_FACTORY;
static QgsExpression *expressionFromOgcFilter( const QDomElement &element, QgsVectorLayer *layer = nullptr ) SIP_FACTORY;

/**
* Creates OGC filter XML element. Supports minimum standard filter
Expand Down Expand Up @@ -298,17 +299,17 @@ class CORE_EXPORT QgsOgcUtils
static QDomElement createGMLPositions( const QgsPolylineXY &points, QDomDocument &doc );

//! handle a generic sub-expression
static QgsExpressionNode *nodeFromOgcFilter( QDomElement &element, QString &errorMessage );
static QgsExpressionNode *nodeFromOgcFilter( QDomElement &element, QString &errorMessage, QgsVectorLayer *layer = nullptr );
//! handle a generic binary operator
static QgsExpressionNodeBinaryOperator *nodeBinaryOperatorFromOgcFilter( QDomElement &element, QString &errorMessage );
static QgsExpressionNodeBinaryOperator *nodeBinaryOperatorFromOgcFilter( QDomElement &element, QString &errorMessage, QgsVectorLayer *layer = nullptr );
//! handles various spatial operation tags (\verbatim <Intersects> \endverbatim, \verbatim <Touches> \endverbatim etc.)
static QgsExpressionNodeFunction *nodeSpatialOperatorFromOgcFilter( QDomElement &element, QString &errorMessage );
//! handle \verbatim <Not> \endverbatim tag
static QgsExpressionNodeUnaryOperator *nodeNotFromOgcFilter( QDomElement &element, QString &errorMessage );
//! handles \verbatim <Function> \endverbatim tag
static QgsExpressionNodeFunction *nodeFunctionFromOgcFilter( QDomElement &element, QString &errorMessage );
//! handles \verbatim <Literal> \endverbatim tag
static QgsExpressionNode *nodeLiteralFromOgcFilter( QDomElement &element, QString &errorMessage );
static QgsExpressionNode *nodeLiteralFromOgcFilter( QDomElement &element, QString &errorMessage, QgsVectorLayer *layer = nullptr );
//! handles \verbatim <PropertyName> \endverbatim tag
static QgsExpressionNodeColumnRef *nodeColumnRefFromOgcFilter( QDomElement &element, QString &errorMessage );
//! handles \verbatim <PropertyIsBetween> \endverbatim tag
Expand Down
1 change: 1 addition & 0 deletions src/server/qgsserverprojectutils.h
View file
Expand Up @@ -20,6 +20,7 @@

#include "qgis_server.h"
#include "qgsproject.h"
#include "qgsvectorlayer.h"

#ifdef SIP_RUN
% ModuleHeaderCode
Expand Down
10 changes: 2 additions & 8 deletions src/server/services/wfs/qgswfsdescribefeaturetype.cpp
View file
Expand Up @@ -139,10 +139,7 @@ namespace QgsWfs
continue;
}

QString name = layer->name();
if ( !layer->shortName().isEmpty() )
name = layer->shortName();
name = name.replace( ' ', '_' );
QString name = layerTypeName( layer );

if ( !typeNameList.isEmpty() && !typeNameList.contains( name ) )
{
Expand Down Expand Up @@ -180,10 +177,7 @@ namespace QgsWfs
return;
}

QString typeName = layer->name();
if ( !layer->shortName().isEmpty() )
typeName = layer->shortName();
typeName = typeName.replace( ' ', '_' );
QString typeName = layerTypeName( layer );

//xsd:element
QDomElement elementElem = doc.createElement( QStringLiteral( "element" )/*xsd:element*/ );
Expand Down
6 changes: 1 addition & 5 deletions src/server/services/wfs/qgswfsgetcapabilities.cpp
View file
Expand Up @@ -438,11 +438,7 @@ namespace QgsWfs

//create Name
QDomElement nameElem = doc.createElement( QStringLiteral( "Name" ) );
QString typeName = layer->name();
if ( !layer->shortName().isEmpty() )
typeName = layer->shortName();
typeName = typeName.replace( QLatin1String( " " ), QLatin1String( "_" ) );
QDomText nameText = doc.createTextNode( typeName );
QDomText nameText = doc.createTextNode( layerTypeName( layer ) );
nameElem.appendChild( nameText );
layerElem.appendChild( nameElem );

Expand Down
21 changes: 9 additions & 12 deletions src/server/services/wfs/qgswfsgetfeature.cpp
View file
Expand Up @@ -101,11 +101,11 @@ namespace QgsWfs
if ( doc.setContent( mRequestParameters.value( QStringLiteral( "REQUEST_BODY" ) ), true, &errorMsg ) )
{
QDomElement docElem = doc.documentElement();
aRequest = parseGetFeatureRequestBody( docElem );
aRequest = parseGetFeatureRequestBody( docElem, project );
}
else
{
aRequest = parseGetFeatureParameters();
aRequest = parseGetFeatureParameters( project );
}

// store typeName
Expand Down Expand Up @@ -141,10 +141,7 @@ namespace QgsWfs
continue;
}

QString name = layer->name();
if ( !layer->shortName().isEmpty() )
name = layer->shortName();
name = name.replace( ' ', '_' );
QString name = layerTypeName( layer );

if ( typeNameList.contains( name ) )
{
Expand Down Expand Up @@ -433,7 +430,7 @@ namespace QgsWfs

}

getFeatureRequest parseGetFeatureParameters()
getFeatureRequest parseGetFeatureParameters( const QgsProject *project )
{
getFeatureRequest request;
request.maxFeatures = mWfsParameters.maxFeaturesAsInt();;
Expand Down Expand Up @@ -767,7 +764,7 @@ namespace QgsWfs
}

QDomElement filterElem = filter.firstChildElement();
query.featureRequest = parseFilterElement( query.typeName, filterElem );
query.featureRequest = parseFilterElement( query.typeName, filterElem, project );

if ( filterIt != filterList.constEnd() )
{
Expand Down Expand Up @@ -821,7 +818,7 @@ namespace QgsWfs
return request;
}

getFeatureRequest parseGetFeatureRequestBody( QDomElement &docElem )
getFeatureRequest parseGetFeatureRequestBody( QDomElement &docElem, const QgsProject *project )
{
getFeatureRequest request;
request.maxFeatures = mWfsParameters.maxFeaturesAsInt();;
Expand All @@ -833,7 +830,7 @@ namespace QgsWfs
for ( int i = 0; i < queryNodes.size(); i++ )
{
queryElem = queryNodes.at( i ).toElement();
getFeatureQuery query = parseQueryElement( queryElem );
getFeatureQuery query = parseQueryElement( queryElem, project );
request.queries.append( query );
}
return request;
Expand Down Expand Up @@ -887,7 +884,7 @@ namespace QgsWfs
}
}

getFeatureQuery parseQueryElement( QDomElement &queryElem )
getFeatureQuery parseQueryElement( QDomElement &queryElem, const QgsProject *project )
{
QString typeName = queryElem.attribute( QStringLiteral( "typeName" ), QLatin1String( "" ) );
if ( typeName.contains( ':' ) )
Expand Down Expand Up @@ -923,7 +920,7 @@ namespace QgsWfs
}
else if ( queryChildElem.tagName() == QLatin1String( "Filter" ) )
{
featureRequest = parseFilterElement( typeName, queryChildElem );
featureRequest = parseFilterElement( typeName, queryChildElem, project );
}
else if ( queryChildElem.tagName() == QLatin1String( "SortBy" ) )
{
Expand Down
6 changes: 3 additions & 3 deletions src/server/services/wfs/qgswfsgetfeature.h
View file
Expand Up @@ -58,17 +58,17 @@ namespace QgsWfs
/**
* Transform Query element to getFeatureQuery
*/
getFeatureQuery parseQueryElement( QDomElement &queryElem );
getFeatureQuery parseQueryElement( QDomElement &queryElem, const QgsProject *project = nullptr );

/**
* Transform RequestBody root element to getFeatureRequest
*/
getFeatureRequest parseGetFeatureRequestBody( QDomElement &docElem );
getFeatureRequest parseGetFeatureRequestBody( QDomElement &docElem, const QgsProject *project = nullptr );

/**
* Transform parameters to getFeatureRequest
*/
getFeatureRequest parseGetFeatureParameters();
getFeatureRequest parseGetFeatureParameters( const QgsProject *project = nullptr );

/**
* Output WFS GetFeature response
Expand Down
5 changes: 1 addition & 4 deletions src/server/services/wfs/qgswfstransaction.cpp
View file
Expand Up @@ -259,10 +259,7 @@ namespace QgsWfs
continue;
}

QString name = layer->name();
if ( !layer->shortName().isEmpty() )
name = layer->shortName();
name = name.replace( ' ', '_' );
QString name = layerTypeName( layer );

if ( !typeNameList.contains( name ) )
{
Expand Down
5 changes: 1 addition & 4 deletions src/server/services/wfs/qgswfstransaction_1_0_0.cpp
View file
Expand Up @@ -242,10 +242,7 @@ namespace QgsWfs
continue;
}

QString name = layer->name();
if ( !layer->shortName().isEmpty() )
name = layer->shortName();
name = name.replace( ' ', '_' );
QString name = layerTypeName( layer );

if ( !typeNameList.contains( name ) )
{
Expand Down

0 comments on commit b07c334

Please sign in to comment.