Skip to content

Commit

Permalink
Classifications on joined fields should only consider values which
Browse files Browse the repository at this point in the history 
are matched to layer's features (fix #9051)

(cherry-picked from 16eb1e1)
  • Loading branch information
@nyalldawson
nyalldawson committed Jul 1, 2016
1 parent 44c7372 commit 54e5dfc
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 147 deletions.
269 changes: 124 additions & 145 deletions src/core/qgsvectorlayer.cpp
View file
Expand Up @@ -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!" );
Expand All @@ -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!" );
Expand All @@ -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!" );
Expand Down
43 changes: 41 additions & 2 deletions tests/src/python/test_qgsvectorlayer.py
View file
Expand Up @@ -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",
Expand All @@ -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


Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 54e5dfc

Please sign in to comment.