Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
QgsExpression::referencedAttributeIndexes(): only report valid indices
If the expression was referencing a non-existing field, -1 was returned in the
result set, which caused later crashed in various providers, including the
Spatialite, Postgres, etc..., due to tried to dereference mFields.at(-1)

Discarding invalid indices is what is also done in
QgsFeatureRequest::OrderBy::usedAttributeIndices()

Fixes #33878
  • Loading branch information
rouault authored and nyalldawson committed Jan 21, 2020
1 parent f84d90d commit bf7dffe
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 2 deletions.
6 changes: 5 additions & 1 deletion src/core/expression/qgsexpression.cpp
Expand Up @@ -250,7 +250,11 @@ QSet<int> QgsExpression::referencedAttributeIndexes( const QgsFields &fields ) c
referencedIndexes = fields.allAttributesList().toSet();
break;
}
referencedIndexes << fields.lookupField( fieldName );
const int idx = fields.lookupField( fieldName );
if ( idx >= 0 )
{
referencedIndexes << idx;
}
}

return referencedIndexes;
Expand Down
7 changes: 7 additions & 0 deletions tests/src/python/featuresourcetestbase.py
Expand Up @@ -715,3 +715,10 @@ def testMaximumValue(self):
def testAllFeatureIds(self):
ids = set([f.id() for f in self.source.getFeatures()])
self.assertEqual(set(self.source.allFeatureIds()), ids)

def testSubsetOfAttributesWithFilterExprWithNonExistingColumn(self):
""" Test fix for https://github.com/qgis/QGIS/issues/33878 """
request = QgsFeatureRequest().setSubsetOfAttributes([0])
request.setFilterExpression("non_existing = 1")
features = [f for f in self.source.getFeatures(request)]
self.assertEqual(len(features), 0)
8 changes: 7 additions & 1 deletion tests/src/python/test_qgsexpression.py
Expand Up @@ -15,7 +15,7 @@
from qgis.PyQt.QtCore import QVariant
from qgis.testing import unittest
from qgis.utils import qgsfunction
from qgis.core import QgsExpression, QgsFeatureRequest, QgsExpressionContext, NULL
from qgis.core import QgsExpression, QgsFeatureRequest, QgsFields, QgsExpressionContext, NULL


class TestQgsExpressionCustomFunctions(unittest.TestCase):
Expand Down Expand Up @@ -264,6 +264,12 @@ def testCreateFieldEqualityExpression(self):
res = '"my\'field" = TRUE'
self.assertEqual(e.createFieldEqualityExpression(field, value), res)

def testReferencedAttributeIndexesNonExistingField(self):
e = QgsExpression()
e.setExpression("foo = 1")
self.assertTrue(e.isValid())
self.assertEqual(len(e.referencedAttributeIndexes(QgsFields())), 0)


if __name__ == "__main__":
unittest.main()
Binary file modified tests/testdata/provider/spatialite.db
Binary file not shown.

0 comments on commit bf7dffe

Please sign in to comment.