OGR expression compiler fixes

- Allow simple operators as supported by OGR (+, -, * )
- Only return a partial compilation success if expression contains
"column"="column2". Since OGR SQL performs case insensitive
comparisons, we need to double check the result using QGIS'
expressions to ensure that case matches. Add unit tests for this
to provider tests.

Author: nyalldawson

src/providers/ogr/qgsogrexpressioncompiler.cpp

@@ -59,9 +59,16 @@ QgsOgrExpressionCompiler::Result QgsOgrExpressionCompiler::compile( const QgsExp
       const QgsExpression::NodeBinaryOperator* n = static_cast<const QgsExpression::NodeBinaryOperator*>( node );
 
       QString op;
+      bool partialCompilation = false;
       switch ( n->op() )
       {
         case QgsExpression::boEQ:
+          if ( n->opLeft()->nodeType() == QgsExpression::ntColumnRef && n->opRight()->nodeType() == QgsExpression::ntColumnRef )
+          {
+            // equality between column refs results in a partial compilation, since OGR will case-insensitive match strings
+            // in columns
+            partialCompilation = true;
+          }
           op = "=";
           break;
 
@@ -118,8 +125,17 @@ QgsOgrExpressionCompiler::Result QgsOgrExpressionCompiler::compile( const QgsExp
           break;
 
         case QgsExpression::boMul:
+          op = "*";
+          break;
+
         case QgsExpression::boPlus:
+          op = "+";
+          break;
+
         case QgsExpression::boMinus:
+          op = "-";
+          break;
+
         case QgsExpression::boDiv:
         case QgsExpression::boMod:
         case QgsExpression::boConcat:
@@ -140,7 +156,7 @@ QgsOgrExpressionCompiler::Result QgsOgrExpressionCompiler::compile( const QgsExp
 
       result = left + ' ' + op + ' ' + right;
       if ( lr == Complete && rr == Complete )
-        return Complete;
+        return ( partialCompilation ? Partial : Complete );
       else if (( lr == Partial && rr == Complete ) || ( lr == Complete && rr == Partial ) || ( lr == Partial && rr == Partial ) )
         return Partial;
       else

src/providers/ogr/qgsogrfeatureiterator.cpp

@@ -93,9 +93,11 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool
     if ( result == QgsOgrExpressionCompiler::Complete || result == QgsOgrExpressionCompiler::Partial )
     {
       QString whereClause = compiler.result();
-      OGR_L_SetAttributeFilter( ogrLayer, whereClause.toLocal8Bit().data() );
-      //if only partial success when compiling expression, we need to double-check results using QGIS' expressions
-      mExpressionCompiled = ( result == QgsOgrExpressionCompiler::Complete );
+      if ( OGR_L_SetAttributeFilter( ogrLayer, whereClause.toLocal8Bit().data() ) == OGRERR_NONE )
+      {
+        //if only partial success when compiling expression, we need to double-check results using QGIS' expressions
+        mExpressionCompiled = ( result == QgsOgrExpressionCompiler::Complete );
+      }
     }
     else
     {

tests/src/python/providertestbase.py

@@ -53,10 +53,14 @@ def runGetFeatureTests(self, provider):
         self.assert_query(provider, 'pk IN (1, 2, 4, 8)', [1, 2, 4])
         self.assert_query(provider, 'cnt = 50 * 2', [1])
         self.assert_query(provider, 'cnt = 99 + 1', [1])
+        self.assert_query(provider, 'cnt = 101 - 1', [1])
+        self.assert_query(provider, 'cnt - 1 = 99', [1])
+        self.assert_query(provider, 'cnt + 1 = 101', [1])
         self.assert_query(provider, 'cnt = 1100 % 1000', [1])
         self.assert_query(provider, '"name" || \' \' || "cnt" = \'Orange 100\'', [1])
         self.assert_query(provider, 'cnt = 10 ^ 2', [1])
         self.assert_query(provider, '"name" ~ \'[OP]ra[gne]+\'', [1])
+        self.assert_query(provider, '"name"="name2"', [2, 4])  # mix of matched and non-matched case sensitive names
         self.assert_query(provider, 'true', [1, 2, 3, 4, 5])
 
     def testGetFeaturesUncompiled(self):

tests/src/python/test_provider_memory.py

@@ -39,28 +39,28 @@ class TestPyQgsMemoryProvider(TestCase, ProviderTestCase):
     def setUpClass(cls):
         """Run before all tests"""
         # Create test layer
-        cls.vl = QgsVectorLayer(u'Point?crs=epsg:4326&field=pk:integer&field=cnt:integer&field=name:string(0)&key=pk',
+        cls.vl = QgsVectorLayer(u'Point?crs=epsg:4326&field=pk:integer&field=cnt:integer&field=name:string(0)&field=name2:string(0)&key=pk',
                                 u'test', u'memory')
         assert (cls.vl.isValid())
         cls.provider = cls.vl.dataProvider()
 
         f1 = QgsFeature()
-        f1.setAttributes([5, -200, NULL])
+        f1.setAttributes([5, -200, NULL, 'NuLl'])
         f1.setGeometry(QgsGeometry.fromWkt('Point (-71.123 78.23)'))
 
         f2 = QgsFeature()
-        f2.setAttributes([3, 300, 'Pear'])
+        f2.setAttributes([3, 300, 'Pear', 'PEaR'])
 
         f3 = QgsFeature()
-        f3.setAttributes([1, 100, 'Orange'])
+        f3.setAttributes([1, 100, 'Orange', 'oranGe'])
         f3.setGeometry(QgsGeometry.fromWkt('Point (-70.332 66.33)'))
 
         f4 = QgsFeature()
-        f4.setAttributes([2, 200, 'Apple'])
+        f4.setAttributes([2, 200, 'Apple', 'Apple'])
         f4.setGeometry(QgsGeometry.fromWkt('Point (-68.2 70.8)'))
 
         f5 = QgsFeature()
-        f5.setAttributes([4, 400, 'Honey'])
+        f5.setAttributes([4, 400, 'Honey', 'Honey'])
         f5.setGeometry(QgsGeometry.fromWkt('Point (-65.32 78.3)'))
 
         cls.provider.addFeatures([f1, f2, f3, f4, f5])

tests/testdata/provider/delimited_wkt.csv

@@ -1,6 +1,6 @@
-pk,cnt,name,wkt
-5,-200,,"Point(-71.123 78.23)"
-3,300,Pear,
-1,100,Orange,"Point(-70.332 66.33)"
-2,200,Apple,"Point(-68.2 70.8)"
-4,400,Honey,"Point(-65.32 78.3)"
+pk,cnt,name,name2,wkt
+5,-200,,NuLl,Point(-71.123 78.23)
+3,300,Pear,PEaR,
+1,100,Orange,oranGe,Point(-70.332 66.33)
+2,200,Apple,Apple,Point(-68.2 70.8)
+4,400,Honey,Honey,Point(-65.32 78.3)

tests/testdata/provider/delimited_xy.csv

@@ -1,6 +1,6 @@
-pk,cnt,name,X,Y
-5,-200,,-71.123,78.23
-3,300,Pear,,
-1,100,Orange,-70.332,66.33
-2,200,Apple,-68.2,70.8
-4,400,Honey,-65.32,78.3
+pk,cnt,name,name2,X,Y
+5,-200,,NuLl,-71.123,78.23
+3,300,Pear,PEaR,,
+1,100,Orange,oranGe,-70.332,66.33
+2,200,Apple,Apple,-68.2,70.8
+4,400,Honey,Honey,-65.32,78.3

tests/testdata/provider/shapefile.dbf

@@ -37,6 +37,7 @@ CREATE TABLE qgis_test."someData" (
     pk SERIAL NOT NULL,
     cnt integer,
     name text DEFAULT 'qgis',
+    name2 text DEFAULT 'qgis',
     geom public.geometry(Point,4326)
 );
 
@@ -47,12 +48,12 @@ CREATE TABLE qgis_test."someData" (
 -- Data for Name: someData; Type: TABLE DATA; Schema: qgis_test; Owner: postgres
 --
 
-INSERT INTO qgis_test."someData" (pk, cnt, name, geom) VALUES
-(5, -200, NULL,     '0101000020E61000001D5A643BDFC751C01F85EB51B88E5340'),
-(3,  300, 'Pear',   NULL),
-(1,  100, 'Orange', '0101000020E61000006891ED7C3F9551C085EB51B81E955040'),
-(2,  200, 'Apple',  '0101000020E6100000CDCCCCCCCC0C51C03333333333B35140'),
-(4,  400, 'Honey',  '0101000020E610000014AE47E17A5450C03333333333935340')
+INSERT INTO qgis_test."someData" (pk, cnt, name, name2, geom) VALUES
+(5, -200, NULL, 'NuLl', '0101000020E61000001D5A643BDFC751C01F85EB51B88E5340'),
+(3,  300, 'Pear', 'PEaR', NULL),
+(1,  100, 'Orange', 'oranGe', '0101000020E61000006891ED7C3F9551C085EB51B81E955040'),
+(2,  200, 'Apple', 'Apple', '0101000020E6100000CDCCCCCCCC0C51C03333333333B35140'),
+(4,  400, 'Honey', 'Honey', '0101000020E610000014AE47E17A5450C03333333333935340')
 ;