Merge pull request #41272 from qgis/backport-41215-to-release-3_16

[Backport release-3_16] Fix wrong attr values in joined fields
qgis · Mar 15, 2021 · dee2346 · dee2346
1 parent f80a754
commit dee2346
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 17 deletions.
diff --git a/src/core/vector/qgsvectorlayerfeatureiterator.cpp b/src/core/vector/qgsvectorlayerfeatureiterator.cpp
@@ -1078,15 +1078,24 @@ 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 );
@@ -1101,22 +1110,9 @@ void QgsVectorLayerFeatureIterator::FetchJoinInfo::addJoinedAttributesDirect( Qg
   {
     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
-    {
-      // 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;
 
-        f.setAttribute( index++, attr.at( i ) );
-      }
-    }
+    for ( int i = 0; i < joinedAttributeIndices.count(); ++i )
+      f.setAttribute( index++, attr.at( joinedAttributeIndices.at( i ) ) );
   }
   else
   {

diff --git a/tests/src/core/testqgsvectorlayerjoinbuffer.cpp b/tests/src/core/testqgsvectorlayerjoinbuffer.cpp
@@ -68,6 +68,7 @@ class TestVectorLayerJoinBuffer : public QObject
     void testResolveReferences();
     void testSignals();
     void testChangeAttributeValues();
+    void testCollidingNameColumn();
 
   private:
     QgsProject mProject;
@@ -841,6 +842,54 @@ 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" ), QStringLiteral( "cacheB" ), QStringLiteral( "memory" ) );
+  QVERIFY( vlB->isValid() );
+  mProject.addMapLayer( vlA );
+  mProject.addMapLayer( vlB );
+
+  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"} ) );
+  QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
+  QCOMPARE( fA1.attribute( "name" ).toString(), QStringLiteral( "name_a" ) );
+  QVERIFY( !fA1.attribute( "value_b" ).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" ) );
+
+  vlB->dataProvider()->addFeatures( QgsFeatureList() << fB1 );
+
+  QgsFeatureIterator fi2 = vlA->getFeatures();
+  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" ) );
+
+}
 
 QGSTEST_MAIN( TestVectorLayerJoinBuffer )
 #include "testqgsvectorlayerjoinbuffer.moc"