[memory] Correctly store converted field values when adding or

changing attributes If testing for value compatiblity via QgsField::convertCompatibility only resulted in true because an automatic type conversion happened then we need to store the auto converted value, not the original. (cherry picked from commit d5eb6b0)
qgis · Feb 28, 2021 · 4e77d4d · 4e77d4d
1 parent bb6f6de
commit 4e77d4d
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/src/core/providers/memory/qgsmemoryprovider.cpp b/src/core/providers/memory/qgsmemoryprovider.cpp
@@ -430,7 +430,8 @@ bool QgsMemoryProvider::addFeatures( QgsFeatureList &flist, Flags flags )
     QString errorMessage;
     for ( int i = 0; i < mFields.count(); ++i )
     {
-      QVariant attrValue { it->attribute( i ) };
+      const QVariant originalValue = it->attribute( i );
+      QVariant attrValue = originalValue;
       if ( ! attrValue.isNull() && ! mFields.at( i ).convertCompatible( attrValue, &errorMessage ) )
       {
         // Push first conversion error only
@@ -443,6 +444,11 @@ bool QgsMemoryProvider::addFeatures( QgsFeatureList &flist, Flags flags )
         conversionError = true;
         continue;
       }
+      else if ( attrValue.type() != originalValue.type() )
+      {
+        // convertCompatible has resulted in a data type conversion
+        it->setAttribute( i, attrValue );
+      }
     }
 
     // Skip the feature if there is at least one conversion error
@@ -614,7 +620,7 @@ bool QgsMemoryProvider::changeAttributeValues( const QgsChangedAttributesMap &at
     // Break on errors
     for ( QgsAttributeMap::const_iterator it2 = attrs.constBegin(); it2 != attrs.constEnd(); ++it2 )
     {
-      QVariant attrValue { it2.value() };
+      QVariant attrValue = it2.value();
       // Check attribute conversion
       const bool conversionError { ! attrValue.isNull()
                                    && ! mFields.at( it2.key() ).convertCompatible( attrValue, &errorMessage ) };
@@ -631,7 +637,7 @@ bool QgsMemoryProvider::changeAttributeValues( const QgsChangedAttributesMap &at
         break;
       }
       rollBackAttrs.insert( it2.key(), fit->attribute( it2.key() ) );
-      fit->setAttribute( it2.key(), it2.value() );
+      fit->setAttribute( it2.key(), attrValue );
     }
     rollBackMap.insert( it.key(), rollBackAttrs );
   }

diff --git a/tests/src/analysis/testqgsnetworkanalysis.cpp b/tests/src/analysis/testqgsnetworkanalysis.cpp
@@ -520,7 +520,7 @@ void TestQgsNetworkAnalysis::testRouteFail()
 
 void TestQgsNetworkAnalysis::testRouteFail2()
 {
-  std::unique_ptr< QgsVectorLayer > network = qgis::make_unique< QgsVectorLayer >( QStringLiteral( "LineString?crs=epsg:4326&field=cost:int" ), QStringLiteral( "x" ), QStringLiteral( "memory" ) );
+  std::unique_ptr< QgsVectorLayer > network = qgis::make_unique< QgsVectorLayer >( QStringLiteral( "LineString?crs=epsg:4326&field=cost:double" ), QStringLiteral( "x" ), QStringLiteral( "memory" ) );
 
   QStringList lines = QStringList() << QStringLiteral( "LineString (11.25044997999680874 48.42605439713970128, 11.25044693759680925 48.42603339773970106, 11.25044760759680962 48.42591690773969759, 11.25052289759680946 48.42589190773969676)" )
                       << QStringLiteral( "LineString (11.25052289759680946 48.42589190773969676, 11.25050350759680917 48.42586202773969717, 11.25047190759680937 48.42581754773969749, 11.2504146475968092 48.42573849773970096, 11.25038716759680923 48.42569834773969717, 11.2502920175968093 48.42557470773969897, 11.25019984759680902 48.42560406773969817, 11.25020393759680992 48.42571203773970012, 11.2502482875968095 48.42577478773969801, 11.25021922759680848 48.42578442773969982)" )

diff --git a/tests/src/python/test_provider_memory.py b/tests/src/python/test_provider_memory.py
@@ -506,6 +506,27 @@ def testCreateMemoryLayer(self):
             self.assertEqual(layer.fields()[i].length(), fields[i].length())
             self.assertEqual(layer.fields()[i].precision(), fields[i].precision())
 
+    def testAddChangeFeatureConvertAttribute(self):
+        """
+        Test add features with attribute values which require conversion
+        """
+        layer = QgsVectorLayer(
+            'Point?crs=epsg:4326&index=yes&field=pk:integer&field=cnt:int8&field=dt:datetime', 'test', 'memory')
+        provider = layer.dataProvider()
+        f = QgsFeature()
+        # string value specified for datetime field -- must be converted when adding the feature
+        f.setAttributes([5, -200, '2021-02-10 00:00'])
+        self.assertTrue(provider.addFeatures([f]))
+
+        saved_feature = next(provider.getFeatures())
+        # saved feature must have a QDateTime value for field, not string
+        self.assertEqual(saved_feature.attributes(), [5, -200, QDateTime(2021, 2, 10, 0, 0)])
+
+        self.assertTrue(provider.changeAttributeValues({saved_feature.id(): {2: '2021-02-12 00:00'}}))
+        saved_feature = next(provider.getFeatures())
+        # saved feature must have a QDateTime value for field, not string
+        self.assertEqual(saved_feature.attributes(), [5, -200, QDateTime(2021, 2, 12, 0, 0)])
+
     def testThreadSafetyWithIndex(self):
         layer = QgsVectorLayer(
             'Point?crs=epsg:4326&index=yes&field=pk:integer&field=cnt:int8&field=name:string(0)&field=name2:string(0)&field=num_char:string&key=pk',