[OGR] Use OGR_F_SetFieldNull() with GDAL >= 2.2 to avoid GeoJSON fields to be unset (fixes #16812)

rouault · rouault · commit fda97b22bdc5 · 2017-07-08T14:31:52.000+02:00
diff --git a/src/core/qgsvectorfilewriter.cpp b/src/core/qgsvectorfilewriter.cpp
@@ -1976,7 +1976,19 @@ OGRFeatureH QgsVectorFileWriter::createFeature( QgsFeature& feature )
     QVariant attrValue = feature.attribute( fldIdx );
 
     if ( !attrValue.isValid() || attrValue.isNull() )
+    {
+// Starting with GDAL 2.2, there are 2 concepts: unset fields and null fields
+// whereas previously there was only unset fields. For a GeoJSON output,
+// leaving a field unset will cause it to not appear at all in the output
+// feature.
+// When all features of a layer have a field unset, this would cause the
+// field to not be present at all in the output, and thus on reading to
+// have disappeared. #16812
+#ifdef OGRNullMarker
+      OGR_F_SetFieldNull( poFeature, ogrField );
+#endif
       continue;
+    }
 
     if ( mFieldValueConverter )
     {
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -1272,7 +1272,18 @@ bool QgsOgrProvider::addFeature( QgsFeature& f )
     QVariant attrVal = attrs.at( qgisAttId );
     if ( attrVal.isNull() || ( type != OFTString && attrVal.toString().isEmpty() ) )
     {
+// Starting with GDAL 2.2, there are 2 concepts: unset fields and null fields
+// whereas previously there was only unset fields. For a GeoJSON output,
+// leaving a field unset will cause it to not appear at all in the output
+// feature.
+// When all features of a layer have a field unset, this would cause the
+// field to not be present at all in the output, and thus on reading to
+// have disappeared. #16812
+#ifdef OGRNullMarker
+      OGR_F_SetFieldNull( feature, ogrAttId );
+#else
       OGR_F_UnsetField( feature, ogrAttId );
+#endif
     }
     else
     {
@@ -1683,7 +1694,18 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
 
       if ( it2->isNull() || ( type != OFTString && it2->toString().isEmpty() ) )
       {
+// Starting with GDAL 2.2, there are 2 concepts: unset fields and null fields
+// whereas previously there was only unset fields. For a GeoJSON output,
+// leaving a field unset will cause it to not appear at all in the output
+// feature.
+// When all features of a layer have a field unset, this would cause the
+// field to not be present at all in the output, and thus on reading to
+// have disappeared. #16812
+#ifdef OGRNullMarker
+        OGR_F_SetFieldNull( of, f );
+#else
         OGR_F_UnsetField( of, f );
+#endif
       }
       else
       {
diff --git a/tests/src/python/test_qgsvectorfilewriter.py b/tests/src/python/test_qgsvectorfilewriter.py
@@ -675,10 +675,17 @@ def testOverwriteLayer(self):
         self.assertIsNotNone(lyr)
         f = lyr.GetNextFeature()
         self.assertEqual(f['firstfield'], 3)
-        self.assertFalse(f.IsFieldSet('secondfield'))
+        if hasattr(f, "IsFieldSetAndNotNull"):
+            # GDAL >= 2.2
+            self.assertFalse(f.IsFieldSetAndNotNull('secondfield'))
+        else:
+            self.assertFalse(f.IsFieldSet('secondfield'))
         f = lyr.GetNextFeature()
         self.assertEqual(f['firstfield'], 4)
-        self.assertFalse(f.IsFieldSet('secondfield'))
+        if hasattr(f, "IsFieldSetAndNotNull"):
+            self.assertFalse(f.IsFieldSetAndNotNull('secondfield'))
+        else:
+            self.assertFalse(f.IsFieldSet('secondfield'))
         f = lyr.GetNextFeature()
         self.assertEqual(f['firstfield'], 5)
         self.assertEqual(f['secondfield'], -1)
