Skip to content
Permalink
Browse files

Fix fallback orderby and attribute subset

Also properly closes the iterator when a fallback orderby is used
  • Loading branch information
m-kuhn committed Dec 20, 2015
1 parent 9d81938 commit a5f88181f69515f9636ff8db58d43509c533c5fb
@@ -78,6 +78,30 @@ class QgsFeatureRequest

};

class OrderBy
{
public:
/**
* Serialize to XML
*/
void save( QDomElement& elem ) const;

/**
* Deserialize from XML
*/
void load( const QDomElement& elem );

/**
* Returns a set of used attributes
*/
QSet<QString> usedAttributes() const;

/**
* Dumps the content to an SQL equivalent syntax
*/
QString dump() const;
};

/**
* A special attribute that if set matches all attributes
*/
@@ -175,12 +199,12 @@ class QgsFeatureRequest
/**
* Return a list of order by clauses specified for this feature request.
*/
QList<QgsFeatureRequest::OrderByClause> orderBys() const;
QgsFeatureRequest::OrderBy orderBys() const;

/**
* Set a list of order by clauses.
*/
void setOrderBys(const QList<QgsFeatureRequest::OrderByClause>& orderBys );
QgsFeatureRequest& setOrderBys(const QgsFeatureRequest::OrderBy& orderBys );

/** Set the maximum number of features to request.
* @param limit maximum number of features, or -1 to request all features.
@@ -149,6 +149,12 @@ bool QgsAbstractFeatureIterator::nextFeature( QgsFeature& f )
++mFeatureIterator;
dataOk = true;
}
else
{
dataOk = false;
// don't call close, the provider connection has already been closed
mClosed = true;
}
}
else
{
@@ -283,6 +289,7 @@ void QgsAbstractFeatureIterator::setupOrderBy( const QList<QgsFeatureRequest::Or

mFeatureIterator = mCachedFeatures.constBegin();
mUseCachedFeatures = true;
mClosed = false;
}
}

@@ -154,14 +154,15 @@ QgsFeatureRequest& QgsFeatureRequest::addOrderBy( const QString& expression, boo
return *this;
}

QList<QgsFeatureRequest::OrderByClause> QgsFeatureRequest::orderBys() const
QgsFeatureRequest::OrderBy QgsFeatureRequest::orderBys() const
{
return mOrderBys;
}

void QgsFeatureRequest::setOrderBys( const QList<QgsFeatureRequest::OrderByClause>& orderBys )
QgsFeatureRequest& QgsFeatureRequest::setOrderBys( const QgsFeatureRequest::OrderBy& orderBys )
{
mOrderBys = orderBys;
return *this;
}

QgsFeatureRequest& QgsFeatureRequest::setLimit( long limit )
@@ -307,7 +308,77 @@ void QgsFeatureRequest::OrderByClause::setNullsFirst( bool nullsFirst )
mNullsFirst = nullsFirst;
}

QString QgsFeatureRequest::OrderByClause::dump() const
{
return QString( "%1 %2 %3" )
.arg( mExpression.expression() )
.arg( mAscending ? "ASC" : "DESC" )
.arg( mNullsFirst ? "NULLS FIRST" : "NULLS LAST" );
}

QgsExpression QgsFeatureRequest::OrderByClause::expression() const
{
return mExpression;
}

void QgsFeatureRequest::OrderBy::save( QDomElement& elem ) const
{
QDomDocument doc = elem.ownerDocument();
QList<OrderByClause>::ConstIterator it;
for ( it = constBegin(); it != constEnd(); ++it )
{
const OrderByClause& clause = *it;
QDomElement clauseElem = doc.createElement( "orderByClause" );
clauseElem.setAttribute( "asc", clause.ascending() );
clauseElem.setAttribute( "nullsFirst", clause.nullsFirst() );
clauseElem.appendChild( doc.createTextNode( clause.expression().expression() ) );

elem.appendChild( clauseElem );
}
}

void QgsFeatureRequest::OrderBy::load( const QDomElement& elem )
{
clear();

QDomNodeList clauses = elem.childNodes();

for ( int i = 0; i < clauses.size(); ++i )
{
QDomElement clauseElem = clauses.at( i ).toElement();
QString expression = clauseElem.toText().data();
bool asc = clauseElem.attribute( "asc" ).toInt() != 0;
bool nullsFirst = clauseElem.attribute( "nullsFirst" ).toInt() != 0;

append( OrderByClause( expression, asc, nullsFirst ) );
}
}

QSet<QString> QgsFeatureRequest::OrderBy::usedAttributes() const
{
QSet<QString> usedAttributes;

QList<OrderByClause>::ConstIterator it;
for ( it = constBegin(); it != constEnd(); ++it )
{
const OrderByClause& clause = *it;

usedAttributes.unite( clause.expression().referencedColumns().toSet() );
}

return usedAttributes;
}
QString QgsFeatureRequest::OrderBy::dump() const
{
QStringList results;

QList<OrderByClause>::ConstIterator it;
for ( it = constBegin(); it != constEnd(); ++it )
{
const OrderByClause& clause = *it;

results << clause.dump();
}

return results.join( ", " );
}
@@ -155,12 +155,47 @@ class CORE_EXPORT QgsFeatureRequest
*/
void setNullsFirst( bool nullsFirst );

/**
* Dumps the content to an SQL equivalent
*/
QString dump() const;

private:
QgsExpression mExpression;
bool mAscending;
bool mNullsFirst;
};

/**
* Represents a list of OrderByClauses, with the most important first and the least
* important last.
*
* @note added in QGIS 2.14
*/
class OrderBy : public QList<OrderByClause>
{
public:
/**
* Serialize to XML
*/
void save( QDomElement& elem ) const;

/**
* Deserialize from XML
*/
void load( const QDomElement& elem );

/**
* Returns a set of used attributes
*/
QSet<QString> usedAttributes() const;

/**
* Dumps the content to an SQL equivalent syntax
*/
QString dump() const;
};

/**
* A special attribute that if set matches all attributes
*/
@@ -277,13 +312,17 @@ class CORE_EXPORT QgsFeatureRequest

/**
* Return a list of order by clauses specified for this feature request.
*
* @note added in 2.14
*/
QList<OrderByClause> orderBys() const;
OrderBy orderBys() const;

/**
* Set a list of order by clauses.
*
* @note added in 2.14
*/
void setOrderBys( const QList<OrderByClause>& orderBys );
QgsFeatureRequest& setOrderBys( const OrderBy& orderBys );

/** Set the maximum number of features to request.
* @param limit maximum number of features, or -1 to request all features.
@@ -346,7 +385,7 @@ class CORE_EXPORT QgsFeatureRequest
QgsAttributeList mAttrs;
QgsSimplifyMethod mSimplifyMethod;
long mLimit;
QList<OrderByClause> mOrderBys;
OrderBy mOrderBys;
};

Q_DECLARE_OPERATORS_FOR_FLAGS( QgsFeatureRequest::Flags )
@@ -105,17 +105,30 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
if ( mProviderRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
{
// prepare list of attributes to match provider fields
QgsAttributeList providerSubset;
QSet<int> providerSubset;
QgsAttributeList subset = mProviderRequest.subsetOfAttributes();
int nPendingFields = mSource->mFields.count();
for ( int i = 0; i < subset.count(); ++i )
Q_FOREACH ( int attrIndex, subset )
{
int attrIndex = subset[i];
if ( attrIndex < 0 || attrIndex >= nPendingFields ) continue;
if ( mSource->mFields.fieldOrigin( attrIndex ) == QgsFields::OriginProvider )
providerSubset << mSource->mFields.fieldOriginIndex( attrIndex );
}
mProviderRequest.setSubsetOfAttributes( providerSubset );

// This is done in order to be prepared to do fallback order bys
// and be sure we have the required columns.
// TODO:
// It would be nicer to first check if we can compile the order by
// and only modify the subset if we cannot.
if ( !mProviderRequest.orderBys().isEmpty() )
{
Q_FOREACH ( const QString& attr, mProviderRequest.orderBys().usedAttributes() )
{
providerSubset << mSource->mFields.fieldNameIndex( attr );
}
}

mProviderRequest.setSubsetOfAttributes( providerSubset.toList() );
}

if ( mProviderRequest.filterType() == QgsFeatureRequest::FilterExpression )

3 comments on commit a5f8818

@SebDieBln

This comment has been minimized.

Copy link
Contributor

@SebDieBln SebDieBln replied Dec 24, 2015

I am sorry to say it, especially on the 24th of December, but I think you broke the build on Windows with this commit 😢

  • The latest nightly build in OSgeo4W is from 06553db which is by now more than 2 nights old.
  • On my machine the build fails starting with this commit.

@jef-n Can you confirm the nightly builds for OSgeo4W failed the last few days?

@jef-n

This comment has been minimized.

Copy link
Member

@jef-n jef-n replied Dec 24, 2015

fixed in 8f29f28.

@SebDieBln

This comment has been minimized.

Copy link
Contributor

@SebDieBln SebDieBln replied Dec 24, 2015

👍
Now 🎅 may come!

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