Skip to content
Permalink
Browse files

Fix virtual layer FilterRect handling when no uid is defined (fixes #…

…15709)

When no uid if defined, features returned have an id defined by an
autoincremented integer. So we cannot use a SQL filter here because it
would return a subset of features and then an autoincremented id that
does not correspond to ids without filters.

So in this case, all the features are requested and the rectangle
intersection is done by the provider, not by SQLite.
  • Loading branch information
mhugo committed Oct 25, 2018
1 parent 225c922 commit efe4a79687eef7866de0f02df5bf1950431356aa
@@ -115,6 +115,14 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF
else // never return a feature if the id is negative
offset = QStringLiteral( " LIMIT 0" );
}
else if ( !mFilterRect.isNull() &&
mRequest.flags() & QgsFeatureRequest::ExactIntersect )
{
// if an exact intersection is requested, prepare the geometry to intersect
QgsGeometry rectGeom = QgsGeometry::fromRect( mFilterRect );
mRectEngine.reset( QgsGeometry::createGeometryEngine( rectGeom.constGet() ) );
mRectEngine->prepareGeometry();
}
}

if ( request.flags() & QgsFeatureRequest::SubsetOfAttributes )
@@ -246,61 +254,85 @@ bool QgsVirtualLayerFeatureIterator::fetchFeature( QgsFeature &feature )
{
return false;
}
if ( mQuery->step() != SQLITE_ROW )
{
return false;
}

feature.setFields( mSource->mFields, /* init */ true );

if ( mSource->mDefinition.uid().isNull() &&
mRequest.filterType() != QgsFeatureRequest::FilterFid )
{
// no id column => autoincrement
feature.setId( mFid++ );
}
else
{
// first column: uid
feature.setId( mQuery->columnInt64( 0 ) );
}

int n = mQuery->columnCount();
int i = 0;
Q_FOREACH ( int idx, mAttributes )
bool skipFeature = false;
do
{
int type = mQuery->columnType( i + 1 );
switch ( type )
if ( mQuery->step() != SQLITE_ROW )
{
case SQLITE_INTEGER:
feature.setAttribute( idx, mQuery->columnInt64( i + 1 ) );
break;
case SQLITE_FLOAT:
feature.setAttribute( idx, mQuery->columnDouble( i + 1 ) );
break;
case SQLITE_TEXT:
default:
feature.setAttribute( idx, mQuery->columnText( i + 1 ) );
break;
};
i++;
}
if ( n > mAttributes.size() + 1 )
{
// geometry field
QByteArray blob( mQuery->columnBlob( n - 1 ) );
if ( blob.size() > 0 )
return false;
}

feature.setFields( mSource->mFields, /* init */ true );

if ( mSource->mDefinition.uid().isNull() &&
mRequest.filterType() != QgsFeatureRequest::FilterFid )
{
feature.setGeometry( spatialiteBlobToQgsGeometry( blob.constData(), blob.size() ) );
// no id column => autoincrement
feature.setId( mFid++ );
}
else
{
feature.clearGeometry();
// first column: uid
feature.setId( mQuery->columnInt64( 0 ) );
}

int n = mQuery->columnCount();
int i = 0;
Q_FOREACH ( int idx, mAttributes )
{
int type = mQuery->columnType( i + 1 );
switch ( type )
{
case SQLITE_INTEGER:
feature.setAttribute( idx, mQuery->columnInt64( i + 1 ) );
break;
case SQLITE_FLOAT:
feature.setAttribute( idx, mQuery->columnDouble( i + 1 ) );
break;
case SQLITE_TEXT:
default:
feature.setAttribute( idx, mQuery->columnText( i + 1 ) );
break;
};
i++;
}
if ( n > mAttributes.size() + 1 )
{
// geometry field
QByteArray blob( mQuery->columnBlob( n - 1 ) );
if ( blob.size() > 0 )
{
feature.setGeometry( spatialiteBlobToQgsGeometry( blob.constData(), blob.size() ) );
}
else
{
feature.clearGeometry();
}
}

feature.setValid( true );
geometryToDestinationCrs( feature, mTransform );

// if the FilterRect has not been applied on the query
// apply it here by skipping features until they intersect
if ( mSource->mDefinition.uid().isNull() && feature.hasGeometry() && mSource->mDefinition.hasDefinedGeometry() && !mFilterRect.isNull() )
{
if ( mRequest.flags() & QgsFeatureRequest::ExactIntersect )
{
// using exact test when checking for intersection
skipFeature = !mRectEngine->intersects( feature.geometry().constGet() );
}
else
{
// check just bounding box against rect when not using intersection
skipFeature = !feature.geometry().boundingBox().intersects( mFilterRect );
}
}
}
while ( skipFeature );

feature.setValid( true );
geometryToDestinationCrs( feature, mTransform );
return true;
}

@@ -20,6 +20,7 @@ email : hugo dot mercier at oslandia dot com

#include "qgsvirtuallayerprovider.h"
#include "qgsfeatureiterator.h"
#include "qgsgeometryengine.h"

#include <memory>
#include <QPointer>
@@ -74,6 +75,7 @@ class QgsVirtualLayerFeatureIterator : public QgsAbstractFeatureIteratorFromSour
QgsCoordinateTransform mTransform;
QgsRectangle mFilterRect;

std::unique_ptr< QgsGeometryEngine > mRectEngine;
};

#endif
@@ -866,9 +866,9 @@ def test_joined_layers_conversion(self):
v1.addJoin(joinInfo2)
self.assertEqual(len(v1.fields()), 7)
df = QgsVirtualLayerDefinitionUtils.fromJoinedLayer(v1)
self.assertEqual(df.query(), ('SELECT t.geometry, t.rowid AS uid, t."id", t."b_id", t."c_id", t."name", j1."bname" AS "B_bname", j1."bfield" AS "B_bfield", j2."cname" AS "C_cname" FROM "{}" AS t ' +
'LEFT JOIN "{}" AS j1 ON t."b_id"=j1."id" ' +
'LEFT JOIN "{}" AS j2 ON t."c_id"=j2."id"').format(v1.id(), v2.id(), v3.id()))
self.assertEqual(df.query(), ('SELECT t.geometry, t.rowid AS uid, t."id", t."b_id", t."c_id", t."name", j1."bname" AS "B_bname", j1."bfield" AS "B_bfield", j2."cname" AS "C_cname" FROM "{}" AS t '
+ 'LEFT JOIN "{}" AS j1 ON t."b_id"=j1."id" '
+ 'LEFT JOIN "{}" AS j2 ON t."c_id"=j2."id"').format(v1.id(), v2.id(), v3.id()))

# test NoGeometry joined layers with field names starting with a digit or containing white spaces
joinInfo3 = QgsVectorLayerJoinInfo()
@@ -938,6 +938,45 @@ def testFieldsWithSpecialCharacters(self):

QgsProject.instance().removeMapLayer(ml)

def testFiltersWithoutUid(self):
ml = QgsVectorLayer("Point?srid=EPSG:4326&field=a:int", "mem_no_uid", "memory")
self.assertEqual(ml.isValid(), True)
QgsProject.instance().addMapLayer(ml)

# a memory layer with 10 features
ml.startEditing()
for i in range(10):
f = QgsFeature(ml.fields())
f.setGeometry(QgsGeometry.fromWkt('POINT({} 0)'.format(i)))
f.setAttributes([i])
ml.addFeatures([f])
ml.commitChanges()

df = QgsVirtualLayerDefinition()
df.setQuery('select * from mem_no_uid')
vl = QgsVectorLayer(df.toString(), "vl", "virtual")
self.assertEqual(vl.isValid(), True)

# make sure the returned id with a filter is the same as
# if there is no filter
req = QgsFeatureRequest().setFilterRect(QgsRectangle(4.5, -1, 5.5, 1))
fids = [f.id() for f in vl.getFeatures(req)]
self.assertEqual(fids, [5])

req = QgsFeatureRequest().setFilterExpression("a = 5")
fids = [f.id() for f in vl.getFeatures(req)]
self.assertEqual(fids, [5])

req = QgsFeatureRequest().setFilterFid(5)
a = [(f.id(), f['a']) for f in vl.getFeatures(req)]
self.assertEqual(a, [(5, 5)])

req = QgsFeatureRequest().setFilterFids([5, 6, 8])
a = [(f.id(), f['a']) for f in vl.getFeatures(req)]
self.assertEqual(a, [(5, 5), (6, 6), (8, 8)])

QgsProject.instance().removeMapLayer(ml)


if __name__ == '__main__':
unittest.main()

0 comments on commit efe4a79

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