Skip to content
Permalink
Browse files

Fix wrong attr values in joined fields

Improved version of #41215 Fix #26652

If the joined layer has field names that collide with already existing
fields, these fields from the joined layer are dropped. However, this
situation was not handled in the code, as the real joined field attr
index is not being used, but indices starting from 0, 1, 2 for each
of the joined fields.

For example:
"joined" layer has fields "id", "name" and "descr";
"source" layer has fields "id", "name", "joined_id"

Then "id" and "name" columns from "joined" will be discarded, but the
the value for "descr" would not be taken from attribute index 2, but
index 1.
  • Loading branch information
suricactus committed Mar 17, 2021
1 parent dee2346 commit ae875f50baf1bf8bc57d12d0991b6603a2e8b811
@@ -719,6 +719,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;
}


@@ -786,7 +787,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() )
{
@@ -1099,20 +1102,27 @@ void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesDirect( Qg
// 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();

for ( int i = 0; i < joinedAttributeIndices.count(); ++i )
f.setAttribute( index++, attr.at( joinedAttributeIndices.at( i ) ) );
for ( const int sourceAttrIndex : sourceAttrIndexes )
{
if ( sourceAttrIndex == joinField )
continue;

int destAttrIndex = attributesSourceToDestLayerMap.value( sourceAttrIndex );

f.setAttribute( destAttrIndex, attr.at( sourceAttrIndex ) );
}
}
else
{
@@ -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
@@ -848,7 +848,7 @@ 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" ), QStringLiteral( "cacheB" ), QStringLiteral( "memory" ) );
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 );
@@ -870,28 +870,44 @@ void TestVectorLayerJoinBuffer::testCollidingNameColumn()

QgsFeatureIterator fi1 = vlA->getFeatures();
fi1.nextFeature( fA1 );
QCOMPARE( fA1.fields().names(), QStringList( {"id_a", "name", "value_b"} ) );
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"} ) );
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" ) );
}

QGSTEST_MAIN( TestVectorLayerJoinBuffer )
#include "testqgsvectorlayerjoinbuffer.moc"


0 comments on commit ae875f5

Please sign in to comment.