Merge branch 'bugfix_relationquote' of https://github.com/pblottiere/…

…QGIS into pblottiere-bugfix_relationquote
qgis · Jul 6, 2017 · fcf3361 · fcf3361
2 parents 603168e + e306cb4
commit fcf3361
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 44 deletions.
diff --git a/python/core/expression/qgsexpression.sip b/python/core/expression/qgsexpression.sip
@@ -495,6 +495,17 @@ return index of the function in Functions array
  :rtype: str
 %End
 
+    static QString createFieldEqualityExpression( const QString &fieldName, const QVariant &value );
+%Docstring
+ Create an expression allowing to evaluate if a field is equal to a
+  value. The value may be null.
+ \param fieldName the name of the field
+ \param value the value of the field
+ :return: the expression to evaluate field equality
+.. versionadded:: 3.0
+ :rtype: str
+%End
+
 };
 
 

diff --git a/src/core/expression/qgsexpression.cpp b/src/core/expression/qgsexpression.cpp
@@ -836,6 +836,18 @@ QString QgsExpression::formatPreviewString( const QVariant &value )
   }
 }
 
+QString QgsExpression::createFieldEqualityExpression( const QString &fieldName, const QVariant &value )
+{
+  QString expr;
+
+  if ( value.isNull() )
+    expr = QStringLiteral( "%1 IS NULL" ).arg( quotedColumnRef( fieldName ) );
+  else
+    expr = QStringLiteral( "%1 = %2" ).arg( quotedColumnRef( fieldName ), quotedValue( value ) );
+
+  return expr;
+}
+
 const QgsExpressionNode *QgsExpression::rootNode() const
 {
   return d->mRootNode;

diff --git a/src/core/expression/qgsexpression.h b/src/core/expression/qgsexpression.h
@@ -449,6 +449,15 @@ class CORE_EXPORT QgsExpression
      */
     static QString formatPreviewString( const QVariant &value );
 
+    /** Create an expression allowing to evaluate if a field is equal to a
+     *  value. The value may be null.
+     * \param fieldName the name of the field
+     * \param value the value of the field
+     * \returns the expression to evaluate field equality
+     * \since QGIS 3.0
+     */
+    static QString createFieldEqualityExpression( const QString &fieldName, const QVariant &value );
+
   private:
     void initGeomCalculator();
 

diff --git a/src/core/qgsrelation.cpp b/src/core/qgsrelation.cpp
@@ -170,23 +170,8 @@ QString QgsRelation::getRelatedFeaturesFilter( const QgsFeature &feature ) const
   {
     int referencingIdx = referencingLayer()->fields().indexFromName( fieldPair.referencingField() );
     QgsField referencingField = referencingLayer()->fields().at( referencingIdx );
-
     QVariant val( feature.attribute( fieldPair.referencedField() ) );
-
-    if ( val.isNull() )
-    {
-      conditions << QStringLiteral( "\"%1\" IS NULL" ).arg( fieldPair.referencingField() );
-    }
-    else if ( referencingField.type() == QVariant::String )
-    {
-      // Use quotes
-      conditions << QStringLiteral( "\"%1\" = '%2'" ).arg( fieldPair.referencingField(), val.toString() );
-    }
-    else
-    {
-      // No quotes
-      conditions << QStringLiteral( "\"%1\" = %2" ).arg( fieldPair.referencingField(), val.toString() );
-    }
+    conditions << QgsExpression::createFieldEqualityExpression( fieldPair.referencingField(), val );
   }
 
   return conditions.join( QStringLiteral( " AND " ) );
@@ -203,16 +188,7 @@ QgsFeatureRequest QgsRelation::getReferencedFeatureRequest( const QgsAttributes
 
     QgsField referencedField = referencedLayer()->fields().at( referencedIdx );
 
-    if ( referencedField.type() == QVariant::String )
-    {
-      // Use quotes
-      conditions << QStringLiteral( "\"%1\" = '%2'" ).arg( fieldPair.referencedField(), attributes.at( referencingIdx ).toString() );
-    }
-    else
-    {
-      // No quotes
-      conditions << QStringLiteral( "\"%1\" = %2" ).arg( fieldPair.referencedField(), attributes.at( referencingIdx ).toString() );
-    }
+    conditions << QStringLiteral( "%1 = %2" ).arg( QgsExpression::quotedColumnRef( fieldPair.referencedField() ), QgsExpression::quotedValue( attributes.at( referencingIdx ) ) );
   }
 
   QgsFeatureRequest myRequest;

diff --git a/src/gui/editorwidgets/qgsrelationreferencewidget.cpp b/src/gui/editorwidgets/qgsrelationreferencewidget.cpp
@@ -861,22 +861,7 @@ void QgsRelationReferenceWidget::filterChanged()
     if ( cb->currentIndex() != 0 )
     {
       const QString fieldName = cb->property( "Field" ).toString();
-
-      if ( cb->currentText() == nullValue.toString() )
-      {
-        filters << QStringLiteral( "\"%1\" IS NULL" ).arg( fieldName );
-      }
-      else
-      {
-        if ( mReferencedLayer->fields().field( fieldName ).type() == QVariant::String )
-        {
-          filters << QStringLiteral( "\"%1\" = '%2'" ).arg( fieldName, cb->currentText() );
-        }
-        else
-        {
-          filters << QStringLiteral( "\"%1\" = %2" ).arg( fieldName, cb->currentText() );
-        }
-      }
+      filters << QgsExpression::createFieldEqualityExpression( fieldName, cb->currentText() );
       attrs << mReferencedLayer->fields().lookupField( fieldName );
     }
   }

diff --git a/tests/src/python/test_qgsexpression.py b/tests/src/python/test_qgsexpression.py
@@ -14,6 +14,7 @@
 
 import qgis  # NOQA
 
+from qgis.PyQt.QtCore import QVariant
 from qgis.testing import unittest
 from qgis.utils import qgsfunction
 from qgis.core import QgsExpression, QgsFeatureRequest
@@ -196,6 +197,39 @@ def testValid(self):
         e.setExpression('1')
         self.assertTrue(e.isValid())
 
+    def testCreateFieldEqualityExpression(self):
+        e = QgsExpression()
+
+        # test when value is null
+        field = "myfield"
+        value = QVariant()
+        res = '"myfield" IS NULL'
+        self.assertEqual(e.createFieldEqualityExpression(field, value), res)
+
+        # test when value is null and field name has a quote
+        field = "my'field"
+        value = QVariant()
+        res = '"my\'field" IS NULL'
+        self.assertEqual(e.createFieldEqualityExpression(field, value), res)
+
+        # test when field name has a quote and value is an int
+        field = "my'field"
+        value = 5
+        res = '"my\'field" = 5'
+        self.assertEqual(e.createFieldEqualityExpression(field, value), res)
+
+        # test when field name has a quote and value is a string
+        field = "my'field"
+        value = '5'
+        res = '"my\'field" = \'5\''
+        self.assertEqual(e.createFieldEqualityExpression(field, value), res)
+
+        # test when field name has a quote and value is a boolean
+        field = "my'field"
+        value = True
+        res = '"my\'field" = TRUE'
+        self.assertEqual(e.createFieldEqualityExpression(field, value), res)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tests/src/python/test_qgsrelation.py b/tests/src/python/test_qgsrelation.py
@@ -38,7 +38,11 @@ def createReferencingLayer():
     f2.setFields(layer.pendingFields())
     f2.setAttributes(["test2", 123])
     f2.setGeometry(QgsGeometry.fromPoint(QgsPointXY(101, 201)))
-    assert pr.addFeatures([f1, f2])
+    f3 = QgsFeature()
+    f3.setFields(layer.pendingFields())
+    f3.setAttributes(["foobar'bar", 124])
+    f3.setGeometry(QgsGeometry.fromPoint(QgsPointXY(101, 201)))
+    assert pr.addFeatures([f1, f2, f3])
     return layer
 
 
@@ -57,7 +61,7 @@ def createReferencedLayer():
     f2.setGeometry(QgsGeometry.fromPoint(QgsPointXY(2, 2)))
     f3 = QgsFeature()
     f3.setFields(layer.pendingFields())
-    f3.setAttributes(["foobar", 789, 554])
+    f3.setAttributes(["foobar'bar", 789, 554])
     f3.setGeometry(QgsGeometry.fromPoint(QgsPointXY(2, 3)))
     assert pr.addFeatures([f1, f2, f3])
     return layer
@@ -112,6 +116,20 @@ def test_getRelatedFeatures(self):
         it = rel.getRelatedFeatures(feat)
         assert [a.attributes() for a in it] == [['test1', 123], ['test2', 123]]
 
+    def test_getRelatedFeaturesWithQuote(self):
+        rel = QgsRelation()
+
+        rel.setId('rel1')
+        rel.setName('Relation Number One')
+        rel.setReferencingLayer(self.referencingLayer.id())
+        rel.setReferencedLayer(self.referencedLayer.id())
+        rel.addFieldPair('fldtxt', 'x')
+
+        feat = self.referencedLayer.getFeature(3)
+
+        it = rel.getRelatedFeatures(feat)
+        assert next(it).attributes() == ["foobar'bar", 124]
+
     def test_getReferencedFeature(self):
         rel = QgsRelation()
         rel.setId('rel1')