Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #8323 from mhugo/fix_15709
Fix virtual layer FilterRect handling when no uid is defined
  • Loading branch information
Hugo Mercier committed Oct 26, 2018
2 parents 40e9406 + efe4a79 commit 443afcd
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 47 deletions.
120 changes: 76 additions & 44 deletions src/providers/virtual/qgsvirtuallayerfeatureiterator.cpp
Expand Up @@ -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 )
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions src/providers/virtual/qgsvirtuallayerfeatureiterator.h
Expand Up @@ -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>
Expand Down Expand Up @@ -74,6 +75,7 @@ class QgsVirtualLayerFeatureIterator : public QgsAbstractFeatureIteratorFromSour
QgsCoordinateTransform mTransform;
QgsRectangle mFilterRect;

std::unique_ptr< QgsGeometryEngine > mRectEngine;
};

#endif
45 changes: 42 additions & 3 deletions tests/src/python/test_provider_virtual.py
Expand Up @@ -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()
Expand Down Expand Up @@ -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()

2 comments on commit 443afcd

@jef-n
Copy link
Member

@jef-n jef-n commented on 443afcd Oct 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhugo guess who's doing the release since 12:00 UTC.

@3nids
Copy link
Member

@3nids 3nids commented on 443afcd Oct 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n can we branch 3_4 then?
so we can backport

Please sign in to comment.