Fix OGR expression compiler with GDAL >= 2.0

nyalldawson · nyalldawson · commit 81be55937538 · 2016-03-03T14:02:43.000+11:00
Some extreme corner cases (eg "NULL OR TRUE") tested by the provider
tests were failing with GDAL 2.0. In GDAL&lt;2.0 these expressions would
be rejected by OGR, but it seems GDAL&gt;=2.0 has partial support for
these so the expression compiler needs to handle them before handing
off to GDAL.
diff --git a/src/core/qgssqlexpressioncompiler.cpp b/src/core/qgssqlexpressioncompiler.cpp
@@ -162,10 +162,22 @@ QgsSqlExpressionCompiler::Result QgsSqlExpressionCompiler::compileNode( const Qg
           break;
 
         case QgsExpression::boOr:
+          if ( mFlags.testFlag( NoNullInBooleanLogic ) )
+          {
+            if ( nodeIsNullLiteral( n->opLeft() ) || nodeIsNullLiteral( n->opRight() ) )
+              return Fail;
+          }
+
           op = "OR";
           break;
 
         case QgsExpression::boAnd:
+          if ( mFlags.testFlag( NoNullInBooleanLogic ) )
+          {
+            if ( nodeIsNullLiteral( n->opLeft() ) || nodeIsNullLiteral( n->opRight() ) )
+              return Fail;
+          }
+
           op = "AND";
           break;
 
@@ -297,3 +309,12 @@ QgsSqlExpressionCompiler::Result QgsSqlExpressionCompiler::compileNode( const Qg
 
   return Fail;
 }
+
+bool QgsSqlExpressionCompiler::nodeIsNullLiteral( const QgsExpression::Node* node ) const
+{
+  if ( node->nodeType() != QgsExpression::ntLiteral )
+    return false;
+
+  const QgsExpression::NodeLiteral* nLit = static_cast<const QgsExpression::NodeLiteral*>( node );
+  return nLit->value().isNull();
+}
diff --git a/src/core/qgssqlexpressioncompiler.h b/src/core/qgssqlexpressioncompiler.h
@@ -49,6 +49,7 @@ class CORE_EXPORT QgsSqlExpressionCompiler
     {
       CaseInsensitiveStringMatch = 0x01,  //!< Provider performs case-insensitive string matching for all strings
       LikeIsCaseInsensitive = 0x02, //!< Provider treats LIKE as case-insensitive
+      NoNullInBooleanLogic = 0x04, //!< Provider does not support using NULL with boolean logic, eg "(...) OR NULL"
     };
     Q_DECLARE_FLAGS( Flags, Flag )
 
@@ -98,6 +99,8 @@ class CORE_EXPORT QgsSqlExpressionCompiler
 
     Flags mFlags;
 
+    bool nodeIsNullLiteral( const QgsExpression::Node* node ) const;
+
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsSqlExpressionCompiler::Flags )
diff --git a/src/providers/ogr/qgsogrexpressioncompiler.cpp b/src/providers/ogr/qgsogrexpressioncompiler.cpp
@@ -17,7 +17,7 @@
 #include "qgsogrprovider.h"
 
 QgsOgrExpressionCompiler::QgsOgrExpressionCompiler( QgsOgrFeatureSource* source )
-    : QgsSqlExpressionCompiler( source->mFields, QgsSqlExpressionCompiler::CaseInsensitiveStringMatch )
+    : QgsSqlExpressionCompiler( source->mFields, QgsSqlExpressionCompiler::CaseInsensitiveStringMatch | QgsSqlExpressionCompiler::NoNullInBooleanLogic )
     , mSource( source )
 {
 }
@@ -95,5 +95,12 @@ QString QgsOgrExpressionCompiler::quotedIdentifier( const QString& identifier )
 QString QgsOgrExpressionCompiler::quotedValue( const QVariant& value, bool& ok )
 {
   ok = true;
+
+  if ( value.type() == QVariant::Bool )
+  {
+    // No support for boolean literals, so fake them
+    return value.toBool() ? "(1=1)" : "(1=0)";
+  }
+
   return QgsOgrProviderUtils::quotedValue( value );
 }

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@`
`17`	`17`	`#include "qgsogrprovider.h"`
`18`	`18`
`19`	`19`	`QgsOgrExpressionCompiler::QgsOgrExpressionCompiler( QgsOgrFeatureSource* source )`
`20`		`- : QgsSqlExpressionCompiler( source->mFields, QgsSqlExpressionCompiler::CaseInsensitiveStringMatch )`
	`20`	`+ : QgsSqlExpressionCompiler( source->mFields, QgsSqlExpressionCompiler::CaseInsensitiveStringMatch \| QgsSqlExpressionCompiler::NoNullInBooleanLogic )`
`21`	`21`	`, mSource( source )`
`22`	`22`	`{`
`23`	`23`	`}`
`@@ -95,5 +95,12 @@ QString QgsOgrExpressionCompiler::quotedIdentifier( const QString& identifier )`
`95`	`95`	`QString QgsOgrExpressionCompiler::quotedValue( const QVariant& value, bool& ok )`
`96`	`96`	`{`
`97`	`97`	`ok = true;`
	`98`	`+`
	`99`	`+ if ( value.type() == QVariant::Bool )`
	`100`	`+ {`
	`101`	`+ // No support for boolean literals, so fake them`
	`102`	`+ return value.toBool() ? "(1=1)" : "(1=0)";`
	`103`	`+ }`
	`104`	`+`
`98`	`105`	`return QgsOgrProviderUtils::quotedValue( value );`
`99`	`106`	`}`