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 @github-actions
m-kuhn authored and github-actions[bot] committed Mar 18, 2021
1 parent b7b73a0 commit 45e79c4
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 20 deletions.
42 changes: 24 additions & 18 deletions src/core/qgsvectorlayerfeatureiterator.cpp
View file
Expand Up @@ -712,6 +712,7 @@ void QgsVectorLayerFeatureIterator::prepareJoin( int fieldIdx )

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


Expand Down Expand Up @@ -779,7 +780,9 @@ void QgsVectorLayerFeatureIterator::prepareFields()

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() )
{
Expand Down Expand Up @@ -1071,44 +1074,47 @@ void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesDirect( Qg
subsetString += '=' + v;
}

QList<int> joinedAttributeIndices;

// maybe user requested just a subset of layer's attributes
// so we do not have to cache everything
QVector<int> subsetIndices;
if ( joinInfo->hasSubset() )
{
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)
QgsFeatureRequest request;
request.setFlags( QgsFeatureRequest::NoGeometry );
request.setSubsetOfAttributes( attributes );
request.setSubsetOfAttributes( joinedAttributeIndices );
request.setFilterExpression( subsetString );
request.setLimit( 1 );
QgsFeatureIterator fi = joinLayer->getFeatures( request );

// get first feature
const QList<int> sourceAttrIndexes = attributesSourceToDestLayerMap.keys();
QgsFeature fet;
if ( fi.nextFeature( fet ) )
{
int index = indexOffset;
QgsAttributes attr = fet.attributes();
if ( joinInfo->hasSubset() )
{
for ( int i = 0; i < subsetIndices.count(); ++i )
f.setAttribute( index++, attr.at( subsetIndices.at( i ) ) );
}
else

for ( const int sourceAttrIndex : sourceAttrIndexes )
{
// use all fields except for the one used for join (has same value as exiting field in target layer)
for ( int i = 0; i < attr.count(); ++i )
{
if ( i == joinField )
continue;
if ( sourceAttrIndex == joinField )
continue;

f.setAttribute( index++, attr.at( i ) );
}
int destAttrIndex = attributesSourceToDestLayerMap.value( sourceAttrIndex );

f.setAttribute( destAttrIndex, attr.at( sourceAttrIndex ) );
}
}
else
Expand Down
1 change: 1 addition & 0 deletions src/core/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
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
QgsVectorLayer *joinLayer; //!< Resolved pointer to the joined layer
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 @@ -67,6 +67,7 @@ class TestVectorLayerJoinBuffer : public QObject
void testResolveReferences();
void testSignals();
void testChangeAttributeValues();
void testCollidingNameColumn();

private:
QgsProject mProject;
Expand Down Expand Up @@ -840,8 +841,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 )
#include "testqgsvectorlayerjoinbuffer.moc"
QgsFeature fA1( vlA->dataProvider()->fields(), 1 );
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 45e79c4

Please sign in to comment.