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.
qgis · Mar 17, 2021 · ae875f5 · ae875f5
1 parent dee2346
commit ae875f5
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 10 deletions.
diff --git a/src/core/vector/qgsvectorlayerfeatureiterator.cpp b/src/core/vector/qgsvectorlayerfeatureiterator.cpp
@@ -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
   {

diff --git a/src/core/vector/qgsvectorlayerfeatureiterator.h b/src/core/vector/qgsvectorlayerfeatureiterator.h
@@ -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

diff --git a/tests/src/core/testqgsvectorlayerjoinbuffer.cpp b/tests/src/core/testqgsvectorlayerjoinbuffer.cpp
@@ -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"
-
-