[server] Proper convertion of literals in Filters

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.
qgis · Jun 5, 2018 · b07c334 · b07c334
1 parent facf7a2
commit b07c334
Show file tree

Hide file tree

Showing 14 changed files with 126 additions and 62 deletions.
diff --git a/python/core/auto_generated/qgsogcutils.sip.in b/python/core/auto_generated/qgsogcutils.sip.in
@@ -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

diff --git a/src/core/qgsogcutils.cpp b/src/core/qgsogcutils.cpp
@@ -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;
@@ -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
@@ -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
@@ -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" ) )
   {
@@ -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;
@@ -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() )
@@ -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() )
@@ -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" ) )
   {
@@ -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;
@@ -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,
+      bool converted = false;
-      // otherwise let's use it as string
+
-      bool ok;
+      // try to convert the node content to corresponding field type if possible
-      double d = value.toDouble( &ok );
+      if ( layer != nullptr )
-      if ( ok )
+      {
-        value = d;
+        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 )

diff --git a/src/core/qgsogcutils.h b/src/core/qgsogcutils.h
@@ -37,6 +37,7 @@ class QgsRectangle;
 #include "qgsexpressionnode.h"
 #include "qgsexpressionnodeimpl.h"
 #include "qgssqlstatement.h"
+#include "qgsvectorlayer.h"
 
 /**
  * \ingroup core
@@ -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
@@ -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

diff --git a/src/server/qgsserverprojectutils.h b/src/server/qgsserverprojectutils.h
@@ -20,6 +20,7 @@
 
 #include "qgis_server.h"
 #include "qgsproject.h"
+#include "qgsvectorlayer.h"
 
 #ifdef SIP_RUN
 % ModuleHeaderCode

diff --git a/src/server/services/wfs/qgswfsdescribefeaturetype.cpp b/src/server/services/wfs/qgswfsdescribefeaturetype.cpp
@@ -139,10 +139,7 @@ namespace QgsWfs
         continue;
       }
 
-      QString name = layer->name();
+      QString name = layerTypeName( layer );
-      if ( !layer->shortName().isEmpty() )
-        name = layer->shortName();
-      name = name.replace( ' ', '_' );
 
       if ( !typeNameList.isEmpty() && !typeNameList.contains( name ) )
       {
@@ -180,10 +177,7 @@ namespace QgsWfs
       return;
     }
 
-    QString typeName = layer->name();
+    QString typeName = layerTypeName( layer );
-    if ( !layer->shortName().isEmpty() )
-      typeName = layer->shortName();
-    typeName = typeName.replace( ' ', '_' );
 
     //xsd:element
     QDomElement elementElem = doc.createElement( QStringLiteral( "element" )/*xsd:element*/ );

diff --git a/src/server/services/wfs/qgswfsgetcapabilities.cpp b/src/server/services/wfs/qgswfsgetcapabilities.cpp
@@ -438,11 +438,7 @@ namespace QgsWfs
 
       //create Name
       QDomElement nameElem = doc.createElement( QStringLiteral( "Name" ) );
-      QString typeName = layer->name();
+      QDomText nameText = doc.createTextNode( layerTypeName( layer ) );
-      if ( !layer->shortName().isEmpty() )
-        typeName = layer->shortName();
-      typeName = typeName.replace( QLatin1String( " " ), QLatin1String( "_" ) );
-      QDomText nameText = doc.createTextNode( typeName );
       nameElem.appendChild( nameText );
       layerElem.appendChild( nameElem );
 

diff --git a/src/server/services/wfs/qgswfsgetfeature.cpp b/src/server/services/wfs/qgswfsgetfeature.cpp
@@ -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
@@ -141,10 +141,7 @@ namespace QgsWfs
         continue;
       }
 
-      QString name = layer->name();
+      QString name = layerTypeName( layer );
-      if ( !layer->shortName().isEmpty() )
-        name = layer->shortName();
-      name = name.replace( ' ', '_' );
 
       if ( typeNameList.contains( name ) )
       {
@@ -433,7 +430,7 @@ namespace QgsWfs
 
   }
 
-  getFeatureRequest parseGetFeatureParameters()
+  getFeatureRequest parseGetFeatureParameters( const QgsProject *project )
   {
     getFeatureRequest request;
     request.maxFeatures = mWfsParameters.maxFeaturesAsInt();;
@@ -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() )
         {
@@ -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();;
@@ -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;
@@ -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( ':' ) )
@@ -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" ) )
         {

diff --git a/src/server/services/wfs/qgswfsgetfeature.h b/src/server/services/wfs/qgswfsgetfeature.h
@@ -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

diff --git a/src/server/services/wfs/qgswfstransaction.cpp b/src/server/services/wfs/qgswfstransaction.cpp
@@ -259,10 +259,7 @@ namespace QgsWfs
         continue;
       }
 
-      QString name = layer->name();
+      QString name = layerTypeName( layer );
-      if ( !layer->shortName().isEmpty() )
-        name = layer->shortName();
-      name = name.replace( ' ', '_' );
 
       if ( !typeNameList.contains( name ) )
       {

diff --git a/src/server/services/wfs/qgswfstransaction_1_0_0.cpp b/src/server/services/wfs/qgswfstransaction_1_0_0.cpp
@@ -242,10 +242,7 @@ namespace QgsWfs
           continue;
         }
 
-        QString name = layer->name();
+        QString name = layerTypeName( layer );
-        if ( !layer->shortName().isEmpty() )
-          name = layer->shortName();
-        name = name.replace( ' ', '_' );
 
         if ( !typeNameList.contains( name ) )
         {