Skip to content

Commit 16eb1e1

Browse files
nyalldawsonnyalldawson
committedJun 13, 2016
Classifications on joined fields should only consider values which
are matched to layer's features (fix #9051)
·
ltr-3_40archive/master_2
1 parent cb4dbea commit 16eb1e1

File tree

2 files changed

+165
-147
lines changed

2 files changed

+165
-147
lines changed
 

‎src/core/qgsvectorlayer.cpp

Lines changed: 124 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -3136,87 +3136,81 @@ void QgsVectorLayer::uniqueValues( int index, QList<QVariant> &uniqueValues, int
31363136
}
31373137

31383138
QgsFields::FieldOrigin origin = mUpdatedFields.fieldOrigin( index );
3139-
if ( origin == QgsFields::OriginUnknown )
3139+
switch ( origin )
31403140
{
3141-
return;
3142-
}
3143-
3144-
if ( origin == QgsFields::OriginProvider ) //a provider field
3145-
{
3146-
mDataProvider->uniqueValues( index, uniqueValues, limit );
3141+
case QgsFields::OriginUnknown:
3142+
return;
31473143

3148-
if ( mEditBuffer )
3144+
case QgsFields::OriginProvider: //a provider field
31493145
{
3150-
QSet<QString> vals;
3151-
Q_FOREACH ( const QVariant& v, uniqueValues )
3152-
{
3153-
vals << v.toString();
3154-
}
3146+
mDataProvider->uniqueValues( index, uniqueValues, limit );
31553147

3156-
QMapIterator< QgsFeatureId, QgsAttributeMap > it( mEditBuffer->changedAttributeValues() );
3157-
while ( it.hasNext() && ( limit < 0 || uniqueValues.count() < limit ) )
3148+
if ( mEditBuffer )
31583149
{
3159-
it.next();
3160-
QVariant v = it.value().value( index );
3161-
if ( v.isValid() )
3150+
QSet<QString> vals;
3151+
Q_FOREACH ( const QVariant& v, uniqueValues )
31623152
{
3163-
QString vs = v.toString();
3164-
if ( !vals.contains( vs ) )
3153+
vals << v.toString();
3154+
}
3155+
3156+
QMapIterator< QgsFeatureId, QgsAttributeMap > it( mEditBuffer->changedAttributeValues() );
3157+
while ( it.hasNext() && ( limit < 0 || uniqueValues.count() < limit ) )
3158+
{
3159+
it.next();
3160+
QVariant v = it.value().value( index );
3161+
if ( v.isValid() )
31653162
{
3166-
vals << vs;
3167-
uniqueValues << v;
3163+
QString vs = v.toString();
3164+
if ( !vals.contains( vs ) )
3165+
{
3166+
vals << vs;
3167+
uniqueValues << v;
3168+
}
31683169
}
31693170
}
31703171
}
3171-
}
3172-
3173-
return;
3174-
}
3175-
else if ( origin == QgsFields::OriginJoin )
3176-
{
3177-
int sourceLayerIndex;
3178-
const QgsVectorJoinInfo* join = mJoinBuffer->joinForFieldIndex( index, mUpdatedFields, sourceLayerIndex );
3179-
Q_ASSERT( join );
3180-
3181-
QgsVectorLayer *vl = dynamic_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( join->joinLayerId ) );
3182-
3183-
if ( vl )
3184-
vl->dataProvider()->uniqueValues( sourceLayerIndex, uniqueValues, limit );
31853172

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

3197-
// we need to go through each feature
3198-
QgsAttributeList attList;
3199-
attList << index;
3176+
case QgsFields::OriginEdit:
3177+
// the layer is editable, but in certain cases it can still be avoided going through all features
3178+
if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
3179+
mEditBuffer->mAddedFeatures.isEmpty() &&
3180+
!mEditBuffer->mDeletedAttributeIds.contains( index ) &&
3181+
mEditBuffer->mChangedAttributeValues.isEmpty() )
3182+
{
3183+
mDataProvider->uniqueValues( index, uniqueValues, limit );
3184+
return;
3185+
}
3186+
FALLTHROUGH
3187+
//we need to go through each feature
3188+
case QgsFields::OriginJoin:
3189+
case QgsFields::OriginExpression:
3190+
{
3191+
QgsAttributeList attList;
3192+
attList << index;
32003193

3201-
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
3202-
.setFlags( QgsFeatureRequest::NoGeometry )
3203-
.setSubsetOfAttributes( attList ) );
3194+
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
3195+
.setFlags( QgsFeatureRequest::NoGeometry )
3196+
.setSubsetOfAttributes( attList ) );
32043197

3205-
QgsFeature f;
3206-
QVariant currentValue;
3207-
QHash<QString, QVariant> val;
3208-
while ( fit.nextFeature( f ) )
3209-
{
3210-
currentValue = f.attribute( index );
3211-
val.insert( currentValue.toString(), currentValue );
3212-
if ( limit >= 0 && val.size() >= limit )
3198+
QgsFeature f;
3199+
QVariant currentValue;
3200+
QHash<QString, QVariant> val;
3201+
while ( fit.nextFeature( f ) )
32133202
{
3214-
break;
3203+
currentValue = f.attribute( index );
3204+
val.insert( currentValue.toString(), currentValue );
3205+
if ( limit >= 0 && val.size() >= limit )
3206+
{
3207+
break;
3208+
}
32153209
}
3216-
}
32173210

3218-
uniqueValues = val.values();
3219-
return;
3211+
uniqueValues = val.values();
3212+
return;
3213+
}
32203214
}
32213215

32223216
Q_ASSERT_X( false, "QgsVectorLayer::uniqueValues()", "Unknown source of the field!" );
@@ -3230,58 +3224,52 @@ QVariant QgsVectorLayer::minimumValue( int index )
32303224
}
32313225

32323226
QgsFields::FieldOrigin origin = mUpdatedFields.fieldOrigin( index );
3233-
if ( origin == QgsFields::OriginUnknown )
3234-
{
3235-
return QVariant();
3236-
}
32373227

3238-
if ( origin == QgsFields::OriginProvider ) //a provider field
3228+
switch ( origin )
32393229
{
3240-
return mDataProvider->minimumValue( index );
3241-
}
3242-
else if ( origin == QgsFields::OriginJoin )
3243-
{
3244-
int sourceLayerIndex;
3245-
const QgsVectorJoinInfo* join = mJoinBuffer->joinForFieldIndex( index, mUpdatedFields, sourceLayerIndex );
3246-
Q_ASSERT( join );
3230+
case QgsFields::OriginUnknown:
3231+
return QVariant();
32473232

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

3251-
return vl->minimumValue( sourceLayerIndex );
3252-
}
3253-
else if ( origin == QgsFields::OriginEdit || origin == QgsFields::OriginExpression )
3254-
{
3255-
// the layer is editable, but in certain cases it can still be avoided going through all features
3256-
if ( origin == QgsFields::OriginEdit &&
3257-
mEditBuffer->mDeletedFeatureIds.isEmpty() &&
3258-
mEditBuffer->mAddedFeatures.isEmpty() && !
3259-
mEditBuffer->mDeletedAttributeIds.contains( index ) &&
3260-
mEditBuffer->mChangedAttributeValues.isEmpty() )
3236+
case QgsFields::OriginEdit:
32613237
{
3262-
return mDataProvider->minimumValue( index );
3238+
// the layer is editable, but in certain cases it can still be avoided going through all features
3239+
if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
3240+
mEditBuffer->mAddedFeatures.isEmpty() && !
3241+
mEditBuffer->mDeletedAttributeIds.contains( index ) &&
3242+
mEditBuffer->mChangedAttributeValues.isEmpty() )
3243+
{
3244+
return mDataProvider->minimumValue( index );
3245+
}
32633246
}
3247+
FALLTHROUGH
3248+
// no choice but to go through all features
3249+
case QgsFields::OriginExpression:
3250+
case QgsFields::OriginJoin:
3251+
{
3252+
// we need to go through each feature
3253+
QgsAttributeList attList;
3254+
attList << index;
32643255

3265-
// we need to go through each feature
3266-
QgsAttributeList attList;
3267-
attList << index;
3268-
3269-
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
3270-
.setFlags( QgsFeatureRequest::NoGeometry )
3271-
.setSubsetOfAttributes( attList ) );
3256+
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
3257+
.setFlags( QgsFeatureRequest::NoGeometry )
3258+
.setSubsetOfAttributes( attList ) );
32723259

3273-
QgsFeature f;
3274-
double minimumValue = std::numeric_limits<double>::max();
3275-
double currentValue = 0;
3276-
while ( fit.nextFeature( f ) )
3277-
{
3278-
currentValue = f.attribute( index ).toDouble();
3279-
if ( currentValue < minimumValue )
3260+
QgsFeature f;
3261+
double minimumValue = std::numeric_limits<double>::max();
3262+
double currentValue = 0;
3263+
while ( fit.nextFeature( f ) )
32803264
{
3281-
minimumValue = currentValue;
3265+
currentValue = f.attribute( index ).toDouble();
3266+
if ( currentValue < minimumValue )
3267+
{
3268+
minimumValue = currentValue;
3269+
}
32823270
}
3271+
return QVariant( minimumValue );
32833272
}
3284-
return QVariant( minimumValue );
32853273
}
32863274

32873275
Q_ASSERT_X( false, "QgsVectorLayer::minimumValue()", "Unknown source of the field!" );
@@ -3296,58 +3284,49 @@ QVariant QgsVectorLayer::maximumValue( int index )
32963284
}
32973285

32983286
QgsFields::FieldOrigin origin = mUpdatedFields.fieldOrigin( index );
3299-
if ( origin == QgsFields::OriginUnknown )
3287+
switch ( origin )
33003288
{
3301-
return QVariant();
3302-
}
3289+
case QgsFields::OriginUnknown:
3290+
return QVariant();
33033291

3304-
if ( origin == QgsFields::OriginProvider ) //a provider field
3305-
{
3306-
return mDataProvider->maximumValue( index );
3307-
}
3308-
else if ( origin == QgsFields::OriginJoin )
3309-
{
3310-
int sourceLayerIndex;
3311-
const QgsVectorJoinInfo* join = mJoinBuffer->joinForFieldIndex( index, mUpdatedFields, sourceLayerIndex );
3312-
Q_ASSERT( join );
3292+
case QgsFields::OriginProvider: //a provider field
3293+
return mDataProvider->maximumValue( index );
33133294

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

3317-
return vl->maximumValue( sourceLayerIndex );
3318-
}
3319-
else if ( origin == QgsFields::OriginEdit || origin == QgsFields::OriginExpression )
3320-
{
3321-
// the layer is editable, but in certain cases it can still be avoided going through all features
3322-
if ( origin == QgsFields::OriginEdit &&
3323-
mEditBuffer->mDeletedFeatureIds.isEmpty() &&
3324-
mEditBuffer->mAddedFeatures.isEmpty() &&
3325-
!mEditBuffer->mDeletedAttributeIds.contains( index ) &&
3326-
mEditBuffer->mChangedAttributeValues.isEmpty() )
3305+
FALLTHROUGH
3306+
//no choice but to go through each feature
3307+
case QgsFields::OriginJoin:
3308+
case QgsFields::OriginExpression:
33273309
{
3328-
return mDataProvider->maximumValue( index );
3329-
}
3330-
3331-
// we need to go through each feature
3332-
QgsAttributeList attList;
3333-
attList << index;
3310+
QgsAttributeList attList;
3311+
attList << index;
33343312

3335-
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
3336-
.setFlags( QgsFeatureRequest::NoGeometry )
3337-
.setSubsetOfAttributes( attList ) );
3313+
QgsFeatureIterator fit = getFeatures( QgsFeatureRequest()
3314+
.setFlags( QgsFeatureRequest::NoGeometry )
3315+
.setSubsetOfAttributes( attList ) );
33383316

3339-
QgsFeature f;
3340-
double maximumValue = -std::numeric_limits<double>::max();
3341-
double currentValue = 0;
3342-
while ( fit.nextFeature( f ) )
3343-
{
3344-
currentValue = f.attribute( index ).toDouble();
3345-
if ( currentValue > maximumValue )
3317+
QgsFeature f;
3318+
double maximumValue = -std::numeric_limits<double>::max();
3319+
double currentValue = 0;
3320+
while ( fit.nextFeature( f ) )
33463321
{
3347-
maximumValue = currentValue;
3322+
currentValue = f.attribute( index ).toDouble();
3323+
if ( currentValue > maximumValue )
3324+
{
3325+
maximumValue = currentValue;
3326+
}
33483327
}
3328+
return QVariant( maximumValue );
33493329
}
3350-
return QVariant( maximumValue );
33513330
}
33523331

33533332
Q_ASSERT_X( false, "QgsVectorLayer::maximumValue()", "Unknown source of the field!" );

‎tests/src/python/test_qgsvectorlayer.py

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,21 @@ def createLayerWithOnePoint():
5959
return layer
6060

6161

62+
def createLayerWithTwoPoints():
63+
layer = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer",
64+
"addfeat", "memory")
65+
pr = layer.dataProvider()
66+
f = QgsFeature()
67+
f.setAttributes(["test", 123])
68+
f.setGeometry(QgsGeometry.fromPoint(QgsPoint(100, 200)))
69+
f2 = QgsFeature()
70+
f2.setAttributes(["test2", 457])
71+
f2.setGeometry(QgsGeometry.fromPoint(QgsPoint(100, 200)))
72+
assert pr.addFeatures([f, f2])
73+
assert layer.pendingFeatureCount() == 2
74+
return layer
75+
76+
6277
def createJoinLayer():
6378
joinLayer = QgsVectorLayer(
6479
"Point?field=x:string&field=y:integer&field=z:integer",
@@ -70,8 +85,14 @@ def createJoinLayer():
7085
f2 = QgsFeature()
7186
f2.setAttributes(["bar", 456, 654])
7287
f2.setGeometry(QgsGeometry.fromPoint(QgsPoint(2, 2)))
73-
assert pr.addFeatures([f1, f2])
74-
assert joinLayer.pendingFeatureCount() == 2
88+
f3 = QgsFeature()
89+
f3.setAttributes(["qar", 457, 111])
90+
f3.setGeometry(QgsGeometry.fromPoint(QgsPoint(2, 2)))
91+
f4 = QgsFeature()
92+
f4.setAttributes(["a", 458, 19])
93+
f4.setGeometry(QgsGeometry.fromPoint(QgsPoint(2, 2)))
94+
assert pr.addFeatures([f1, f2, f3, f4])
95+
assert joinLayer.pendingFeatureCount() == 4
7596
return joinLayer
7697

7798

@@ -1083,6 +1104,24 @@ def test_join(self):
10831104
self.assertEqual(f2[2], "foo")
10841105
self.assertEqual(f2[3], 321)
10851106

1107+
def test_JoinStats(self):
1108+
""" test calculating min/max/uniqueValues on joined field """
1109+
joinLayer = createJoinLayer()
1110+
layer = createLayerWithTwoPoints()
1111+
QgsMapLayerRegistry.instance().addMapLayers([joinLayer, layer])
1112+
1113+
join = QgsVectorJoinInfo()
1114+
join.targetFieldName = "fldint"
1115+
join.joinLayerId = joinLayer.id()
1116+
join.joinFieldName = "y"
1117+
join.memoryCache = True
1118+
layer.addJoin(join)
1119+
1120+
# stats on joined fields should only include values present by join
1121+
self.assertEqual(layer.minimumValue(3), 111)
1122+
self.assertEqual(layer.maximumValue(3), 321)
1123+
self.assertEqual(set(layer.uniqueValues(3)), set([111, 321]))
1124+
10861125
def test_InvalidOperations(self):
10871126
layer = createLayerWithOnePoint()
10881127

0 commit comments

Comments
 (0)
Please sign in to comment.