Skip to content
Permalink
Browse files
Partial fix for virtual layer iterator depending on provider
The proper fix is more involved and requires reworking of the
sqlite handle to utilise a shared pointer. Without this
the iterator will return no features if the provider is removed
while a source is active (i.e. removing virtual layer while
map is rendering).

But at least it avoids a crash in this circumstance...
  • Loading branch information
nyalldawson committed Apr 23, 2017
1 parent 0dfe687 commit c85a437855049947e7c95edcd618471a5d0174b2
@@ -29,43 +29,52 @@ static QString quotedColumn( QString name )
QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerFeatureSource *source, bool ownSource, const QgsFeatureRequest &request )
: QgsAbstractFeatureIteratorFromSource<QgsVirtualLayerFeatureSource>( source, ownSource, request )
{
try

// NOTE: this is really bad and should be removed.
// it's only here to guard mSource->mSqlite - because if the provider is removed
// then mSqlite will be meaningless.
// this needs to be totally reworked so that mSqlite no longer depends on the provider
// and can be fully encapsulated in the source
if ( !mSource->mProvider )
{
mSqlite = mSource->provider()->mSqlite.get();
mDefinition = mSource->provider()->mDefinition;
close();
return;
}

QString tableName = mSource->provider()->mTableName;
try
{
QString tableName = mSource->mTableName;

QStringList wheres;
QString subset = mSource->provider()->mSubset;
QString subset = mSource->mSubset;
if ( !subset.isNull() )
{
wheres << subset;
}

if ( !mDefinition.uid().isNull() )
if ( !mSource->mDefinition.uid().isNull() )
{
// filters are only available when a column with unique id exists
if ( mDefinition.hasDefinedGeometry() && !request.filterRect().isNull() )
if ( mSource->mDefinition.hasDefinedGeometry() && !request.filterRect().isNull() )
{
bool do_exact = request.flags() & QgsFeatureRequest::ExactIntersect;
QgsRectangle rect( request.filterRect() );
QString mbr = QStringLiteral( "%1,%2,%3,%4" ).arg( rect.xMinimum() ).arg( rect.yMinimum() ).arg( rect.xMaximum() ).arg( rect.yMaximum() );
wheres << quotedColumn( mDefinition.geometryField() ) + " is not null";
wheres << quotedColumn( mSource->mDefinition.geometryField() ) + " is not null";
wheres << QStringLiteral( "%1Intersects(%2,BuildMbr(%3))" )
.arg( do_exact ? "" : "Mbr",
quotedColumn( mDefinition.geometryField() ),
quotedColumn( mSource->mDefinition.geometryField() ),
mbr );
}
else if ( request.filterType() == QgsFeatureRequest::FilterFid )
{
wheres << QStringLiteral( "%1=%2" )
.arg( quotedColumn( mDefinition.uid() ) )
.arg( quotedColumn( mSource->mDefinition.uid() ) )
.arg( request.filterFid() );
}
else if ( request.filterType() == QgsFeatureRequest::FilterFids )
{
QString values = quotedColumn( mDefinition.uid() ) + " IN (";
QString values = quotedColumn( mSource->mDefinition.uid() ) + " IN (";
bool first = true;
Q_FOREACH ( QgsFeatureId v, request.filterFids() )
{
@@ -81,7 +90,6 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF
}
}

mFields = mSource->provider()->fields();
if ( request.flags() & QgsFeatureRequest::SubsetOfAttributes )
{
// copy only selected fields
@@ -95,23 +103,23 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF
{
Q_FOREACH ( const QString &field, request.filterExpression()->referencedColumns() )
{
int attrIdx = mFields.lookupField( field );
int attrIdx = mSource->mFields.lookupField( field );
if ( !mAttributes.contains( attrIdx ) )
mAttributes << attrIdx;
}
}
}
else
{
mAttributes = mFields.allAttributesList();
mAttributes = mSource->mFields.allAttributesList();
}

QString columns;
{
// the first column is always the uid (or 0)
if ( !mDefinition.uid().isNull() )
if ( !mSource->mDefinition.uid().isNull() )
{
columns = quotedColumn( mDefinition.uid() );
columns = quotedColumn( mSource->mDefinition.uid() );
}
else
{
@@ -120,16 +128,16 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF
Q_FOREACH ( int i, mAttributes )
{
columns += QLatin1String( "," );
QString cname = mFields.at( i ).name().toLower();
QString cname = mSource->mFields.at( i ).name().toLower();
columns += quotedColumn( cname );
}
}
// the last column is the geometry, if any
if ( ( !( request.flags() & QgsFeatureRequest::NoGeometry )
|| ( request.filterType() == QgsFeatureRequest::FilterExpression && request.filterExpression()->needsGeometry() ) )
&& !mDefinition.geometryField().isNull() && mDefinition.geometryField() != QLatin1String( "*no*" ) )
&& !mSource->mDefinition.geometryField().isNull() && mSource->mDefinition.geometryField() != QLatin1String( "*no*" ) )
{
columns += "," + quotedColumn( mDefinition.geometryField() );
columns += "," + quotedColumn( mSource->mDefinition.geometryField() );
}

mSqlQuery = "SELECT " + columns + " FROM " + tableName;
@@ -138,7 +146,7 @@ QgsVirtualLayerFeatureIterator::QgsVirtualLayerFeatureIterator( QgsVirtualLayerF
mSqlQuery += " WHERE " + wheres.join( QStringLiteral( " AND " ) );
}

mQuery.reset( new Sqlite::Query( mSqlite, mSqlQuery ) );
mQuery.reset( new Sqlite::Query( mSource->mSqlite, mSqlQuery ) );

mFid = 0;
}
@@ -193,9 +201,9 @@ bool QgsVirtualLayerFeatureIterator::fetchFeature( QgsFeature &feature )
return false;
}

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

if ( mDefinition.uid().isNull() )
if ( mSource->mDefinition.uid().isNull() )
{
// no id column => autoincrement
feature.setId( mFid++ );
@@ -246,6 +254,11 @@ bool QgsVirtualLayerFeatureIterator::fetchFeature( QgsFeature &feature )

QgsVirtualLayerFeatureSource::QgsVirtualLayerFeatureSource( const QgsVirtualLayerProvider *p )
: mProvider( p )
, mDefinition( p->mDefinition )
, mFields( p->fields() )
, mSqlite( p->mSqlite.get() )
, mTableName( p->mTableName )
, mSubset( p->mSubset )
{
}

@@ -21,6 +21,7 @@ email : hugo dot mercier at oslandia dot com
#include <qgsvirtuallayerprovider.h>
#include <qgsfeatureiterator.h>
#include <memory>
#include <QPointer>

class QgsVirtualLayerFeatureSource : public QgsAbstractFeatureSource
{
@@ -30,9 +31,23 @@ class QgsVirtualLayerFeatureSource : public QgsAbstractFeatureSource

virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request ) override;

const QgsVirtualLayerProvider *provider() const { return mProvider; }
private:
const QgsVirtualLayerProvider *mProvider = nullptr;

// NOTE: this is really bad and should be removed.
// it's only here to guard mSqlite - because if the provider is removed
// then mSqlite will be meaningless.
// this needs to be totally reworked so that mSqlite no longer depends on the provider
// and can be fully encapsulated here
QPointer< const QgsVirtualLayerProvider > mProvider;

QString mPath;
QgsVirtualLayerDefinition mDefinition;
QgsFields mFields;
sqlite3 *mSqlite = nullptr;
QString mTableName;
QString mSubset;

friend class QgsVirtualLayerFeatureIterator;
};

class QgsVirtualLayerFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsVirtualLayerFeatureSource>
@@ -50,16 +65,10 @@ class QgsVirtualLayerFeatureIterator : public QgsAbstractFeatureIteratorFromSour

std::unique_ptr<Sqlite::Query> mQuery;

QgsFeatureId mFid;

QString mPath;
sqlite3 *mSqlite = nullptr;
QgsVirtualLayerDefinition mDefinition;
QgsFields mFields;

QgsAttributeList mAttributes;
QString mSqlQuery;
QgsFeatureId mFid;

QgsAttributeList mAttributes;
};

#endif
@@ -113,7 +113,7 @@ class QgsVirtualLayerProvider: public QgsVectorDataProvider
bool createIt();
bool loadSourceLayers();

friend class QgsVirtualLayerFeatureIterator;
friend class QgsVirtualLayerFeatureSource;

private slots:
void invalidateStatistics();

0 comments on commit c85a437

Please sign in to comment.