Skip to content
Permalink
Browse files

Merge pull request #7118 from m-kuhn/stringCompOperators

Cast left node to text on string comparison
  • Loading branch information
m-kuhn committed Jun 4, 2018
2 parents adf5eb7 + 48bbd24 commit 36712b9ed2921e0591e2a910fe43651d10b51809
@@ -65,6 +65,14 @@ after a timeout to give the system some time to stay responsive).
If you want to check if the iterator successfully completed, better use :py:func:`QgsFeatureIterator.isClosed()`

.. versionadded:: 3.0
%End

bool compileFailed() const;
%Docstring
Indicator if there was an error when sending the compiled query to the server.
This indicates that there is something wrong with the expression compiler.

.. versionadded:: 3.2
%End

protected:
@@ -144,6 +152,7 @@ Remove reference, delete if refs == 0




virtual bool prepareSimplification( const QgsSimplifyMethod &simplifyMethod );
%Docstring
Setup the simplification of geometries to fetch using the specified simplify method
@@ -251,6 +260,14 @@ Returns the status of expression compilation for filter expression requests.
.. versionadded:: 2.16
%End

bool compileFailed() const;
%Docstring
Indicator if there was an error when sending the compiled query to the server.
This indicates that there is something wrong with the expression compiler.

.. versionadded:: 3.2
%End


protected:

@@ -147,6 +147,11 @@ void QgsAbstractFeatureIterator::deref()
delete this;
}

bool QgsAbstractFeatureIterator::compileFailed() const
{
return mCompileFailed;
}

bool QgsAbstractFeatureIterator::prepareSimplification( const QgsSimplifyMethod &simplifyMethod )
{
Q_UNUSED( simplifyMethod );
@@ -83,6 +83,14 @@ class CORE_EXPORT QgsAbstractFeatureIterator
return mValid;
}

/**
* Indicator if there was an error when sending the compiled query to the server.
* This indicates that there is something wrong with the expression compiler.
*
* \since QGIS 3.2
*/
bool compileFailed() const;

protected:

/**
@@ -173,6 +181,8 @@ class CORE_EXPORT QgsAbstractFeatureIterator
//! Status of compilation of filter expression
CompileStatus mCompileStatus = NoCompilation;

bool mCompileFailed = false;

//! Setup the simplification of geometries to fetch using the specified simplify method
virtual bool prepareSimplification( const QgsSimplifyMethod &simplifyMethod );

@@ -323,6 +333,14 @@ class CORE_EXPORT QgsFeatureIterator
*/
QgsAbstractFeatureIterator::CompileStatus compileStatus() const { return mIter->compileStatus(); }

/**
* Indicator if there was an error when sending the compiled query to the server.
* This indicates that there is something wrong with the expression compiler.
*
* \since QGIS 3.2
*/
bool compileFailed() const { return mIter->compileFailed(); }

friend bool operator== ( const QgsFeatureIterator &fi1, const QgsFeatureIterator &fi2 ) SIP_SKIP;
friend bool operator!= ( const QgsFeatureIterator &fi1, const QgsFeatureIterator &fi2 ) SIP_SKIP;

@@ -38,6 +38,18 @@ QString QgsSqlExpressionCompiler::result()
return mResult;
}

bool QgsSqlExpressionCompiler::opIsStringComparison( QgsExpressionNodeBinaryOperator::BinaryOperator op )
{
if ( op == QgsExpressionNodeBinaryOperator::BinaryOperator::boILike ||
op == QgsExpressionNodeBinaryOperator::BinaryOperator::boLike ||
op == QgsExpressionNodeBinaryOperator::BinaryOperator::boNotILike ||
op == QgsExpressionNodeBinaryOperator::BinaryOperator::boNotLike ||
op == QgsExpressionNodeBinaryOperator::BinaryOperator::boRegexp )
return true;
else
return false;
}

QString QgsSqlExpressionCompiler::quotedIdentifier( const QString &identifier )
{
QString quoted = identifier;
@@ -253,6 +265,9 @@ QgsSqlExpressionCompiler::Result QgsSqlExpressionCompiler::compileNode( const Qg
QString left;
Result lr( compileNode( n->opLeft(), left ) );

if ( opIsStringComparison( n ->op() ) )
left = castToText( left );

QString right;
Result rr( compileNode( n->opRight(), right ) );

@@ -409,6 +424,11 @@ QString QgsSqlExpressionCompiler::castToReal( const QString &value ) const
return QString();
}

QString QgsSqlExpressionCompiler::castToText( const QString &value ) const
{
return value;
}

QString QgsSqlExpressionCompiler::castToInt( const QString &value ) const
{
Q_UNUSED( value );
@@ -20,6 +20,7 @@

#include "qgis_core.h"
#include "qgsfields.h"
#include "qgsexpressionnodeimpl.h"

class QgsExpression;
class QgsExpressionNode;
@@ -80,6 +81,22 @@ class CORE_EXPORT QgsSqlExpressionCompiler
*/
virtual QString result();

/**
* Returns true if \a op is one of
*
* - LIKE
* - ILIKE
* - NOT LIKE
* - NOT ILIKE
* - ~ (regexp)
*
* In such cases the left operator will be cast to string to behave equal to
* QGIS own expression engine.
*
* \since QGIS 3.2
*/
bool opIsStringComparison( QgsExpressionNodeBinaryOperator::BinaryOperator op );

protected:

/**
@@ -132,6 +149,23 @@ class CORE_EXPORT QgsSqlExpressionCompiler
*/
virtual QString castToReal( const QString &value ) const;

/**
* Casts a value to a text result. Subclasses that support casting to text may implement this function
* to get equal behavior to the QGIS expression engine when string comparison operators are applied
* on non-string data.
*
* Example:
*
* 579 LIKE '5%'
*
* which on a postgres database needs to be
*
* 579::text LIKE '5%'
*
* \since QGIS 3.2
*/
virtual QString castToText( const QString &value ) const;

/**
* Casts a value to a integer result. Subclasses must reimplement this to cast a numeric value to a integer
* type value so that integer division results in a integer value result instead of real.
@@ -111,4 +111,9 @@ QString QgsSQLiteExpressionCompiler::castToInt( const QString &value ) const
return QStringLiteral( "CAST((%1) AS INTEGER)" ).arg( value );
}

QString QgsSQLiteExpressionCompiler::castToText( const QString &value ) const
{
return QStringLiteral( "CAST((%1) AS TEXT)" ).arg( value );
}

///@endcond
@@ -52,6 +52,7 @@ class CORE_EXPORT QgsSQLiteExpressionCompiler : public QgsSqlExpressionCompiler
QString sqlFunctionFromFunctionName( const QString &fnName ) const override;
QString castToReal( const QString &value ) const override;
QString castToInt( const QString &value ) const override;
QString castToText( const QString &value ) const override;

};

@@ -144,6 +144,11 @@ QString QgsPostgresExpressionCompiler::castToInt( const QString &value ) const
return QStringLiteral( "((%1)::int)" ).arg( value );
}

QString QgsPostgresExpressionCompiler::castToText( const QString &value ) const
{
return QStringLiteral( "((%1)::text)" ).arg( value );
}

QgsSqlExpressionCompiler::Result QgsPostgresExpressionCompiler::compileNode( const QgsExpressionNode *node, QString &result )
{
switch ( node->nodeType() )
@@ -36,6 +36,7 @@ class QgsPostgresExpressionCompiler : public QgsSqlExpressionCompiler
QStringList sqlArgumentsFromFunctionName( const QString &fnName, const QStringList &fnArgs ) const override;
QString castToReal( const QString &value ) const override;
QString castToInt( const QString &value ) const override;
QString castToText( const QString &value ) const override;

QString mGeometryColumn;
QgsPostgresGeometryColumnType mSpatialColType;
@@ -210,7 +210,10 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
//try with the fallback where clause, e.g., for cases when using compiled expression failed to prepare
success = declareCursor( fallbackWhereClause, -1, false, orderByParts.join( QStringLiteral( "," ) ) );
if ( success )
{
mExpressionCompiled = false;
mCompileFailed = true;
}
}

if ( !success && !orderByParts.isEmpty() )
@@ -213,6 +213,7 @@ QgsSpatiaLiteFeatureIterator::QgsSpatiaLiteFeatureIterator( QgsSpatiaLiteFeature
//try with the fallback where clause, e.g., for cases when using compiled expression failed to prepare
mExpressionCompiled = false;
success = prepareStatement( fallbackWhereClause, -1, orderByParts.join( QStringLiteral( "," ) ) );
mCompileFailed = true;
}

if ( !success )
@@ -52,6 +52,11 @@ def uncompiledFilters(self):
cannot be compiled """
return set()

def enableCompiler(self):
"""By default there is no expression compiling available, needs to be overridden in subclass"""
print('Provider does not support compiling')
return False

def partiallyCompiledFilters(self):
""" Individual derived provider tests should override this to return a list of expressions which
should be partially compiled """
@@ -141,14 +146,11 @@ def testGetFeaturesUncompiled(self):
self.runPolyGetFeatureTests(self.poly_provider)

def testGetFeaturesExp(self):
try:
self.enableCompiler()
if self.enableCompiler():
self.compiled = True
self.runGetFeatureTests(self.source)
if hasattr(self, 'poly_provider'):
self.runPolyGetFeatureTests(self.poly_provider)
except AttributeError:
print('Provider does not support compiling')

def testSubsetString(self):
if not self.source.supportsSubsetString():
@@ -232,11 +234,8 @@ def testOrderBy(self):
self.runOrderByTests()

def testOrderByCompiled(self):
try:
self.enableCompiler()
if self.enableCompiler():
self.runOrderByTests()
except AttributeError:
print('Provider does not support compiling')

def runOrderByTests(self):
FeatureSourceTestCase.runOrderByTests(self)
@@ -817,3 +816,24 @@ def testMinMaxAfterChanges(self):
self.assertTrue(vl.dataProvider().deleteAttributes([0]))
self.assertEqual(vl.dataProvider().minimumValue(0), -200)
self.assertEqual(vl.dataProvider().maximumValue(0), 400)

def testStringComparison(self):
"""
Test if string comparisons with numbers are cast by the expression
compiler (or work fine without doing anything :P)
"""
for expression in (
'5 LIKE \'5\'',
'5 ILIKE \'5\'',
'15 NOT LIKE \'5\'',
'15 NOT ILIKE \'5\'',
'5 ~ \'5\''):
iterator = self.source.getFeatures(QgsFeatureRequest().setFilterExpression('5 LIKE \'5\''))
count = len([f for f in iterator])
self.assertEqual(count, 5)
self.assertFalse(iterator.compileFailed())
if self.enableCompiler():
iterator = self.source.getFeatures(QgsFeatureRequest().setFilterExpression('5 LIKE \'5\''))
self.assertEqual(count, 5)
self.assertFalse(iterator.compileFailed())
self.disableCompiler()
@@ -58,6 +58,7 @@ def getSubsetString(self):

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

def disableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', False)
@@ -52,6 +52,7 @@ def tearDownClass(cls):

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

def disableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', False)
@@ -85,6 +85,7 @@ def getEditableLayer(self):

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

def disableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', False)
@@ -104,6 +104,7 @@ def getEditableLayer(self):

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

def disableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', False)
@@ -1125,6 +1126,7 @@ def tearDownClass(cls):

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

def disableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', False)
@@ -93,6 +93,7 @@ def getEditableLayer(self):

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

def disableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', False)
@@ -260,6 +260,7 @@ def tearDown(self):

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

def disableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', False)

0 comments on commit 36712b9

Please sign in to comment.
You can’t perform that action at this time.