Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nyalldawson committed Nov 17, 2015
1 parent d19adfe commit 26f7683
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 28 deletions.
18 changes: 17 additions & 1 deletion src/providers/ogr/qgsogrexpressioncompiler.cpp
Expand Up @@ -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;

Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Expand Up @@ -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
{
Expand Down
4 changes: 4 additions & 0 deletions tests/src/python/providertestbase.py
Expand Up @@ -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):
Expand Down
12 changes: 6 additions & 6 deletions tests/src/python/test_provider_memory.py
Expand Up @@ -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])
Expand Down
12 changes: 6 additions & 6 deletions 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)
12 changes: 6 additions & 6 deletions 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
Binary file modified tests/testdata/provider/shapefile.dbf
Binary file not shown.
Binary file modified tests/testdata/provider/spatialite.db
Binary file not shown.
13 changes: 7 additions & 6 deletions tests/testdata/provider/testdata.sql
Expand Up @@ -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)
);

Expand All @@ -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')
;


Expand Down

0 comments on commit 26f7683

Please sign in to comment.