Skip to content

Commit

Permalink
Merge pull request #42295 from suricactus/fix_attr_table_shifts2
Browse files Browse the repository at this point in the history 
Fix wrong attr values in joined fields
  • Loading branch information
@m-kuhn
m-kuhn committed Mar 18, 2021
2 parents c22f5a9 + 7370da4 commit 073e3f8
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 20 deletions.
42 changes: 24 additions & 18 deletions src/core/vector/qgsvectorlayerfeatureiterator.cpp
View file
Expand Up @@ -719,6 +719,7 @@ void QgsVectorLayerFeatureIterator::prepareJoin( int fieldIdx )


// store field source index - we'll need it when fetching from provider // store field source index - we'll need it when fetching from provider
mFetchJoinInfo[ joinInfo ].attributes.push_back( sourceLayerIndex ); mFetchJoinInfo[ joinInfo ].attributes.push_back( sourceLayerIndex );
mFetchJoinInfo[ joinInfo ].attributesSourceToDestLayerMap[sourceLayerIndex] = fieldIdx;
} }




Expand Down Expand Up @@ -786,7 +787,9 @@ void QgsVectorLayerFeatureIterator::prepareFields()


mExpressionContext.reset(); mExpressionContext.reset();


mFieldsToPrepare = ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes ) ? mRequest.subsetOfAttributes() : mSource->mFields.allAttributesList(); mFieldsToPrepare = ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
? mRequest.subsetOfAttributes()
: mSource->mFields.allAttributesList();


while ( !mFieldsToPrepare.isEmpty() ) while ( !mFieldsToPrepare.isEmpty() )
{ {
Expand Down Expand Up @@ -1078,44 +1081,47 @@ void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesDirect( Qg
subsetString += '=' + v; subsetString += '=' + v;
} }


QList<int> joinedAttributeIndices;

// maybe user requested just a subset of layer's attributes // maybe user requested just a subset of layer's attributes
// so we do not have to cache everything // so we do not have to cache everything
QVector<int> subsetIndices;
if ( joinInfo->hasSubset() ) if ( joinInfo->hasSubset() )
{ {
const QStringList subsetNames = QgsVectorLayerJoinInfo::joinFieldNamesSubset( *joinInfo ); const QStringList subsetNames = QgsVectorLayerJoinInfo::joinFieldNamesSubset( *joinInfo );
subsetIndices = QgsVectorLayerJoinBuffer::joinSubsetIndices( joinLayer, subsetNames ); QVector<int> subsetIndices = QgsVectorLayerJoinBuffer::joinSubsetIndices( joinLayer, subsetNames );
joinedAttributeIndices = qgis::setToList( qgis::listToSet( attributes ).intersect( qgis::listToSet( subsetIndices.toList() ) ) );
}
else
{
joinedAttributeIndices = attributes;
} }


// we don't need the join field, it is already present in the other table
joinedAttributeIndices.removeAll( joinField );

// select (no geometry) // select (no geometry)
QgsFeatureRequest request; QgsFeatureRequest request;
request.setFlags( QgsFeatureRequest::NoGeometry ); request.setFlags( QgsFeatureRequest::NoGeometry );
request.setSubsetOfAttributes( attributes ); request.setSubsetOfAttributes( joinedAttributeIndices );
request.setFilterExpression( subsetString ); request.setFilterExpression( subsetString );
request.setLimit( 1 ); request.setLimit( 1 );
QgsFeatureIterator fi = joinLayer->getFeatures( request ); QgsFeatureIterator fi = joinLayer->getFeatures( request );


// get first feature // get first feature
const QList<int> sourceAttrIndexes = attributesSourceToDestLayerMap.keys();
QgsFeature fet; QgsFeature fet;
if ( fi.nextFeature( fet ) ) if ( fi.nextFeature( fet ) )
{ {
int index = indexOffset;
QgsAttributes attr = fet.attributes(); QgsAttributes attr = fet.attributes();
if ( joinInfo->hasSubset() )
{ for ( const int sourceAttrIndex : sourceAttrIndexes )
for ( int i = 0; i < subsetIndices.count(); ++i )
f.setAttribute( index++, attr.at( subsetIndices.at( i ) ) );
}
else
{ {
// use all fields except for the one used for join (has same value as exiting field in target layer) if ( sourceAttrIndex == joinField )
for ( int i = 0; i < attr.count(); ++i ) continue;
{
if ( i == joinField )
continue;


f.setAttribute( index++, attr.at( i ) ); int destAttrIndex = attributesSourceToDestLayerMap.value( sourceAttrIndex );
}
f.setAttribute( destAttrIndex, attr.at( sourceAttrIndex ) );
} }
} }
else else
Expand Down
1 change: 1 addition & 0 deletions src/core/vector/qgsvectorlayerfeatureiterator.h
View file
Expand Up @@ -148,6 +148,7 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
{ {
const QgsVectorLayerJoinInfo *joinInfo;//!< Canonical source of information about the join const QgsVectorLayerJoinInfo *joinInfo;//!< Canonical source of information about the join
QgsAttributeList attributes; //!< Attributes to fetch QgsAttributeList attributes; //!< Attributes to fetch
QMap<int, int> attributesSourceToDestLayerMap SIP_SKIP; //!< Mapping from original attribute index to the joined layer index
int indexOffset; //!< At what position the joined fields start int indexOffset; //!< At what position the joined fields start
QgsVectorLayer *joinLayer; //!< Resolved pointer to the joined layer QgsVectorLayer *joinLayer; //!< Resolved pointer to the joined layer
int targetField; //!< Index of field (of this layer) that drives the join int targetField; //!< Index of field (of this layer) that drives the join
Expand Down
138 changes: 136 additions & 2 deletions tests/src/core/testqgsvectorlayerjoinbuffer.cpp
View file
Expand Up @@ -68,6 +68,7 @@ class TestVectorLayerJoinBuffer : public QObject
void testResolveReferences(); void testResolveReferences();
void testSignals(); void testSignals();
void testChangeAttributeValues(); void testChangeAttributeValues();
void testCollidingNameColumn();


private: private:
QgsProject mProject; QgsProject mProject;
Expand Down Expand Up @@ -841,8 +842,141 @@ void TestVectorLayerJoinBuffer::testChangeAttributeValues()


} }


// Check https://github.com/qgis/QGIS/issues/26652
void TestVectorLayerJoinBuffer::testCollidingNameColumn()
{
mProject.clear();
QgsVectorLayer *vlA = new QgsVectorLayer( QStringLiteral( "Point?field=id_a:integer&field=name" ), QStringLiteral( "cacheA" ), QStringLiteral( "memory" ) );
QVERIFY( vlA->isValid() );
QgsVectorLayer *vlB = new QgsVectorLayer( QStringLiteral( "Point?field=id_b:integer&field=name&field=value_b&field=value_c" ), QStringLiteral( "cacheB" ), QStringLiteral( "memory" ) );
QVERIFY( vlB->isValid() );
mProject.addMapLayer( vlA );
mProject.addMapLayer( vlB );


QGSTEST_MAIN( TestVectorLayerJoinBuffer ) QgsFeature fA1( vlA->dataProvider()->fields(), 1 );
#include "testqgsvectorlayerjoinbuffer.moc" fA1.setAttribute( QStringLiteral( "id_a" ), 1 );
fA1.setAttribute( QStringLiteral( "name" ), QStringLiteral( "name_a" ) );

vlA->dataProvider()->addFeatures( QgsFeatureList() << fA1 );

QgsVectorLayerJoinInfo joinInfo;
joinInfo.setTargetFieldName( QStringLiteral( "id_a" ) );
joinInfo.setJoinLayer( vlB );
joinInfo.setJoinFieldName( QStringLiteral( "id_b" ) );
joinInfo.setPrefix( QStringLiteral( "" ) );
joinInfo.setEditable( true );
joinInfo.setUpsertOnEdit( true );
vlA->addJoin( joinInfo );


QgsFeatureIterator fi1 = vlA->getFeatures();
fi1.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QVERIFY( !fA1.attribute( "value_b" ).isValid() );
QVERIFY( !fA1.attribute( "value_c" ).isValid() );

QgsFeature fB1( vlB->dataProvider()->fields(), 1 );
fB1.setAttribute( QStringLiteral( "id_b" ), 1 );
fB1.setAttribute( QStringLiteral( "name" ), QStringLiteral( "name_b" ) );
fB1.setAttribute( QStringLiteral( "value_b" ), QStringLiteral( "value_b" ) );
fB1.setAttribute( QStringLiteral( "value_c" ), QStringLiteral( "value_c" ) );

vlB->dataProvider()->addFeatures( QgsFeatureList() << fB1 );

QgsFeatureIterator fi2 = vlA->getFeatures();
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QCOMPARE( fA1.attribute( "value_b" ).toString(), QStringLiteral( "value_b" ) );
QCOMPARE( fA1.attribute( "value_c" ).toString(), QStringLiteral( "value_c" ) );

fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 2} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QCOMPARE( fA1.attribute( "value_b" ).toString(), QStringLiteral( "value_b" ) );
QVERIFY( !fA1.attribute( "value_c" ).isValid() );

fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 3} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QVERIFY( !fA1.attribute( "value_b" ).isValid() );
QCOMPARE( fA1.attribute( "value_c" ).toString(), QStringLiteral( "value_c" ) );


vlA->removeJoin( vlB->id() );
joinInfo.setJoinFieldNamesSubset( new QStringList( {"name"} ) );
vlA->addJoin( joinInfo );
fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 2} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );

vlA->removeJoin( vlB->id() );
joinInfo.setJoinFieldNamesSubset( new QStringList( {"value_b"} ) );
vlA->addJoin( joinInfo );
fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 2} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QCOMPARE( fA1.attribute( "value_b" ).toString(), QStringLiteral( "value_b" ) );

vlA->removeJoin( vlB->id() );
joinInfo.setJoinFieldNamesSubset( new QStringList( {"value_c"} ) );
vlA->addJoin( joinInfo );
fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 2} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QCOMPARE( fA1.attribute( "value_c" ).toString(), QStringLiteral( "value_c" ) );

vlA->removeJoin( vlB->id() );
joinInfo.setJoinFieldNamesSubset( new QStringList( {"name", "value_c"} ) );
vlA->addJoin( joinInfo );
fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 2, 3} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QCOMPARE( fA1.attribute( "value_c" ).toString(), QStringLiteral( "value_c" ) );

vlA->removeJoin( vlB->id() );
joinInfo.setJoinFieldNamesSubset( new QStringList( {"value_b", "value_c"} ) );
vlA->addJoin( joinInfo );
fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 2, 3} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QCOMPARE( fA1.attribute( "value_b" ).toString(), QStringLiteral( "value_b" ) );
QCOMPARE( fA1.attribute( "value_c" ).toString(), QStringLiteral( "value_c" ) );

vlA->removeJoin( vlB->id() );
joinInfo.setJoinFieldNamesSubset( nullptr );
vlA->addJoin( joinInfo );
fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 2} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QCOMPARE( fA1.attribute( "value_b" ).toString(), QStringLiteral( "value_b" ) );
QVERIFY( !fA1.attribute( "value_c" ).isValid() );

fi2 = vlA->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList( {0, 1, 3} ) ) );
fi2.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b", "value_c"} ) );
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
QVERIFY( !fA1.attribute( "value_b" ).isValid() );
QCOMPARE( fA1.attribute( "value_c" ).toString(), QStringLiteral( "value_c" ) );
}

QGSTEST_MAIN( TestVectorLayerJoinBuffer )
#include "testqgsvectorlayerjoinbuffer.moc"

0 comments on commit 073e3f8

Please sign in to comment.