From 7d67b02ae461adf5bbf8cc753ea61a64ec125bbc Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sat, 8 Jul 2017 14:01:10 +0200 Subject: [PATCH] [OGR] Use OGR_F_SetFieldNull() with GDAL >= 2.2 to avoid GeoJSON fields to be unset (fixes #16812) --- src/core/qgsvectorfilewriter.cpp | 12 +++++++++++ src/providers/ogr/qgsogrprovider.cpp | 22 ++++++++++++++++++++ tests/src/python/test_qgsvectorfilewriter.py | 11 ++++++++-- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/core/qgsvectorfilewriter.cpp b/src/core/qgsvectorfilewriter.cpp index 2b7b3657a56..3f97bdb9319 100644 --- a/src/core/qgsvectorfilewriter.cpp +++ b/src/core/qgsvectorfilewriter.cpp @@ -1949,7 +1949,19 @@ OGRFeatureH QgsVectorFileWriter::createFeature( const 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 index 6af84901351..3cd4afde408 100644 --- a/src/providers/ogr/qgsogrprovider.cpp +++ b/src/providers/ogr/qgsogrprovider.cpp @@ -1289,7 +1289,18 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) 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 { @@ -1684,7 +1695,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 index 43cc15a9868..7f081f1a7df 100755 --- a/tests/src/python/test_qgsvectorfilewriter.py +++ b/tests/src/python/test_qgsvectorfilewriter.py @@ -703,10 +703,17 @@ class TestQgsVectorFileWriter(unittest.TestCase): 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)