Skip to content
Permalink
Browse files

Classifications on joined fields should only consider values which

are matched to layer's features (fix #9051)

(cherry-picked from 16eb1e1)
  • Loading branch information
nyalldawson committed Jul 1, 2016
1 parent 44c7372 commit 54e5dfc839ea876a4fa0035b9ff9cfc2680b2cbd
Showing with 165 additions and 147 deletions.
  1. +124 −145 src/core/qgsvectorlayer.cpp
  2. +41 −2 tests/src/python/test_qgsvectorlayer.py
@@ -2938,87 +2938,81 @@ void QgsVectorLayer::uniqueValues( int index, QList<QVariant> &uniqueValues, int
}

QgsFields::FieldOrigin origin = mUpdatedFields.fieldOrigin( index );
if ( origin == QgsFields::OriginUnknown )
switch ( origin )
{
return;
}

if ( origin == QgsFields::OriginProvider ) //a provider field
{
mDataProvider->uniqueValues( index, uniqueValues, limit );
case QgsFields::OriginUnknown:
return;

if ( mEditBuffer )
case QgsFields::OriginProvider: //a provider field
{
QSet<QString> vals;
Q_FOREACH ( const QVariant& v, uniqueValues )
{
vals << v.toString();
}
mDataProvider->uniqueValues( index, uniqueValues, limit );

QMapIterator< QgsFeatureId, QgsAttributeMap > it( mEditBuffer->changedAttributeValues() );
while ( it.hasNext() && ( limit < 0 || uniqueValues.count() < limit ) )
if ( mEditBuffer )
{
it.next();
QVariant v = it.value().value( index );
if ( v.isValid() )
QSet<QString> vals;
Q_FOREACH ( const QVariant& v, uniqueValues )
{
QString vs = v.toString();
if ( !vals.contains( vs ) )
vals << v.toString();
}

QMapIterator< QgsFeatureId, QgsAttributeMap > it( mEditBuffer->changedAttributeValues() );
while ( it.hasNext() && ( limit < 0 || uniqueValues.count() < limit ) )
{
it.next();
QVariant v = it.value().value( index );
if ( v.isValid() )
{
vals << vs;
uniqueValues << v;
QString vs = v.toString();
if ( !vals.contains( vs ) )
{
vals << vs;
uniqueValues << v;
}
}
}
}
}

return;
}
else if ( origin == QgsFields::OriginJoin )
{
int sourceLayerIndex;
const QgsVectorJoinInfo* join = mJoinBuffer->joinForFieldIndex( index, mUpdatedFields, sourceLayerIndex );
Q_ASSERT( join );

QgsVectorLayer *vl = dynamic_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( join->joinLayerId ) );

if ( vl )
vl->dataProvider()->uniqueValues( sourceLayerIndex, uniqueValues, limit );

return;
}
else if ( origin == QgsFields::OriginEdit || origin == QgsFields::OriginExpression )
{
// the layer is editable, but in certain cases it can still be avoided going through all features
if ( origin == QgsFields::OriginEdit && mEditBuffer->mDeletedFeatureIds.isEmpty() && mEditBuffer->mAddedFeatures.isEmpty() && !mEditBuffer->mDeletedAttributeIds.contains( index ) && mEditBuffer->mChangedAttributeValues.isEmpty() )
{
mDataProvider->uniqueValues( index, uniqueValues, limit );
return;
}

// we need to go through each feature
QgsAttributeList attList;
attList << index;
case QgsFields::OriginEdit:
// the layer is editable, but in certain cases it can still be avoided going through all features
if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
mEditBuffer->mAddedFeatures.isEmpty() &&
!mEditBuffer->mDeletedAttributeIds.contains( index ) &&
mEditBuffer->mChangedAttributeValues.isEmpty() )
{
mDataProvider->uniqueValues( index, uniqueValues, limit );
return;
}
FALLTHROUGH
//we need to go through each feature
case QgsFields::OriginJoin:
case QgsFields::OriginExpression:
{
QgsAttributeList attList;
attList << index;

QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
.setFlags( QgsFeatureRequest::NoGeometry )
.setSubsetOfAttributes( attList ) );
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
.setFlags( QgsFeatureRequest::NoGeometry )
.setSubsetOfAttributes( attList ) );

QgsFeature f;
QVariant currentValue;
QHash<QString, QVariant> val;
while ( fit.nextFeature( f ) )
{
currentValue = f.attribute( index );
val.insert( currentValue.toString(), currentValue );
if ( limit >= 0 && val.size() >= limit )
QgsFeature f;
QVariant currentValue;
QHash<QString, QVariant> val;
while ( fit.nextFeature( f ) )
{
break;
currentValue = f.attribute( index );
val.insert( currentValue.toString(), currentValue );
if ( limit >= 0 && val.size() >= limit )
{
break;
}
}
}

uniqueValues = val.values();
return;
uniqueValues = val.values();
return;
}
}

Q_ASSERT_X( false, "QgsVectorLayer::uniqueValues()", "Unknown source of the field!" );
@@ -3032,58 +3026,52 @@ QVariant QgsVectorLayer::minimumValue( int index )
}

QgsFields::FieldOrigin origin = mUpdatedFields.fieldOrigin( index );
if ( origin == QgsFields::OriginUnknown )
{
return QVariant();
}

if ( origin == QgsFields::OriginProvider ) //a provider field
switch ( origin )
{
return mDataProvider->minimumValue( index );
}
else if ( origin == QgsFields::OriginJoin )
{
int sourceLayerIndex;
const QgsVectorJoinInfo* join = mJoinBuffer->joinForFieldIndex( index, mUpdatedFields, sourceLayerIndex );
Q_ASSERT( join );
case QgsFields::OriginUnknown:
return QVariant();

QgsVectorLayer* vl = dynamic_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( join->joinLayerId ) );
Q_ASSERT( vl );
case QgsFields::OriginProvider: //a provider field
return mDataProvider->minimumValue( index );

return vl->minimumValue( sourceLayerIndex );
}
else if ( origin == QgsFields::OriginEdit || origin == QgsFields::OriginExpression )
{
// the layer is editable, but in certain cases it can still be avoided going through all features
if ( origin == QgsFields::OriginEdit &&
mEditBuffer->mDeletedFeatureIds.isEmpty() &&
mEditBuffer->mAddedFeatures.isEmpty() && !
mEditBuffer->mDeletedAttributeIds.contains( index ) &&
mEditBuffer->mChangedAttributeValues.isEmpty() )
case QgsFields::OriginEdit:
{
return mDataProvider->minimumValue( index );
// the layer is editable, but in certain cases it can still be avoided going through all features
if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
mEditBuffer->mAddedFeatures.isEmpty() && !
mEditBuffer->mDeletedAttributeIds.contains( index ) &&
mEditBuffer->mChangedAttributeValues.isEmpty() )
{
return mDataProvider->minimumValue( index );
}
}
FALLTHROUGH
// no choice but to go through all features
case QgsFields::OriginExpression:
case QgsFields::OriginJoin:
{
// we need to go through each feature
QgsAttributeList attList;
attList << index;

// we need to go through each feature
QgsAttributeList attList;
attList << index;

QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
.setFlags( QgsFeatureRequest::NoGeometry )
.setSubsetOfAttributes( attList ) );
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
.setFlags( QgsFeatureRequest::NoGeometry )
.setSubsetOfAttributes( attList ) );

QgsFeature f;
double minimumValue = std::numeric_limits<double>::max();
double currentValue = 0;
while ( fit.nextFeature( f ) )
{
currentValue = f.attribute( index ).toDouble();
if ( currentValue < minimumValue )
QgsFeature f;
double minimumValue = std::numeric_limits<double>::max();
double currentValue = 0;
while ( fit.nextFeature( f ) )
{
minimumValue = currentValue;
currentValue = f.attribute( index ).toDouble();
if ( currentValue < minimumValue )
{
minimumValue = currentValue;
}
}
return QVariant( minimumValue );
}
return QVariant( minimumValue );
}

Q_ASSERT_X( false, "QgsVectorLayer::minimumValue()", "Unknown source of the field!" );
@@ -3098,58 +3086,49 @@ QVariant QgsVectorLayer::maximumValue( int index )
}

QgsFields::FieldOrigin origin = mUpdatedFields.fieldOrigin( index );
if ( origin == QgsFields::OriginUnknown )
switch ( origin )
{
return QVariant();
}
case QgsFields::OriginUnknown:
return QVariant();

if ( origin == QgsFields::OriginProvider ) //a provider field
{
return mDataProvider->maximumValue( index );
}
else if ( origin == QgsFields::OriginJoin )
{
int sourceLayerIndex;
const QgsVectorJoinInfo* join = mJoinBuffer->joinForFieldIndex( index, mUpdatedFields, sourceLayerIndex );
Q_ASSERT( join );
case QgsFields::OriginProvider: //a provider field
return mDataProvider->maximumValue( index );

QgsVectorLayer* vl = dynamic_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( join->joinLayerId ) );
Q_ASSERT( vl );
case QgsFields::OriginEdit:
// the layer is editable, but in certain cases it can still be avoided going through all features
if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
mEditBuffer->mAddedFeatures.isEmpty() &&
!mEditBuffer->mDeletedAttributeIds.contains( index ) &&
mEditBuffer->mChangedAttributeValues.isEmpty() )
{
return mDataProvider->maximumValue( index );
}

return vl->maximumValue( sourceLayerIndex );
}
else if ( origin == QgsFields::OriginEdit || origin == QgsFields::OriginExpression )
{
// the layer is editable, but in certain cases it can still be avoided going through all features
if ( origin == QgsFields::OriginEdit &&
mEditBuffer->mDeletedFeatureIds.isEmpty() &&
mEditBuffer->mAddedFeatures.isEmpty() &&
!mEditBuffer->mDeletedAttributeIds.contains( index ) &&
mEditBuffer->mChangedAttributeValues.isEmpty() )
FALLTHROUGH
//no choice but to go through each feature
case QgsFields::OriginJoin:
case QgsFields::OriginExpression:
{
return mDataProvider->maximumValue( index );
}

// we need to go through each feature
QgsAttributeList attList;
attList << index;
QgsAttributeList attList;
attList << index;

QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
.setFlags( QgsFeatureRequest::NoGeometry )
.setSubsetOfAttributes( attList ) );
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
.setFlags( QgsFeatureRequest::NoGeometry )
.setSubsetOfAttributes( attList ) );

QgsFeature f;
double maximumValue = -std::numeric_limits<double>::max();
double currentValue = 0;
while ( fit.nextFeature( f ) )
{
currentValue = f.attribute( index ).toDouble();
if ( currentValue > maximumValue )
QgsFeature f;
double maximumValue = -std::numeric_limits<double>::max();
double currentValue = 0;
while ( fit.nextFeature( f ) )
{
maximumValue = currentValue;
currentValue = f.attribute( index ).toDouble();
if ( currentValue > maximumValue )
{
maximumValue = currentValue;
}
}
return QVariant( maximumValue );
}
return QVariant( maximumValue );
}

Q_ASSERT_X( false, "QgsVectorLayer::maximumValue()", "Unknown source of the field!" );
@@ -58,6 +58,21 @@ def createLayerWithOnePoint():
return layer


def createLayerWithTwoPoints():
layer = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer",
"addfeat", "memory")
pr = layer.dataProvider()
f = QgsFeature()
f.setAttributes(["test", 123])
f.setGeometry(QgsGeometry.fromPoint(QgsPoint(100, 200)))
f2 = QgsFeature()
f2.setAttributes(["test2", 457])
f2.setGeometry(QgsGeometry.fromPoint(QgsPoint(100, 200)))
assert pr.addFeatures([f, f2])
assert layer.pendingFeatureCount() == 2
return layer


def createJoinLayer():
joinLayer = QgsVectorLayer(
"Point?field=x:string&field=y:integer&field=z:integer",
@@ -69,8 +84,14 @@ def createJoinLayer():
f2 = QgsFeature()
f2.setAttributes(["bar", 456, 654])
f2.setGeometry(QgsGeometry.fromPoint(QgsPoint(2, 2)))
assert pr.addFeatures([f1, f2])
assert joinLayer.pendingFeatureCount() == 2
f3 = QgsFeature()
f3.setAttributes(["qar", 457, 111])
f3.setGeometry(QgsGeometry.fromPoint(QgsPoint(2, 2)))
f4 = QgsFeature()
f4.setAttributes(["a", 458, 19])
f4.setGeometry(QgsGeometry.fromPoint(QgsPoint(2, 2)))
assert pr.addFeatures([f1, f2, f3, f4])
assert joinLayer.pendingFeatureCount() == 4
return joinLayer


@@ -900,6 +921,24 @@ def test_join(self):
assert f2[2] == "foo"
assert f2[3] == 321

def test_JoinStats(self):
""" test calculating min/max/uniqueValues on joined field """
joinLayer = createJoinLayer()
layer = createLayerWithTwoPoints()
QgsMapLayerRegistry.instance().addMapLayers([joinLayer, layer])

join = QgsVectorJoinInfo()
join.targetFieldName = "fldint"
join.joinLayerId = joinLayer.id()
join.joinFieldName = "y"
join.memoryCache = True
layer.addJoin(join)

# stats on joined fields should only include values present by join
self.assertEqual(layer.minimumValue(3), 111)
self.assertEqual(layer.maximumValue(3), 321)
self.assertEqual(set(layer.uniqueValues(3)), set([111, 321]))

def test_InvalidOperations(self):
layer = createLayerWithOnePoint()

0 comments on commit 54e5dfc

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