Skip to content

Commit

Permalink
[OGR provider] Recognize "false"/"true" string as valid value for a b…
Browse files Browse the repository at this point in the history 
…oolean field in addFeature()/changeAttributeValues()"

Fixes qgis#55517
  • Loading branch information
@rouault
rouault committed Mar 31, 2024
1 parent 5f1465f commit a25fef1
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 4 deletions.
66 changes: 62 additions & 4 deletions src/core/providers/ogr/qgsogrprovider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1593,6 +1593,24 @@ static int strictToInt( const QVariant &v, bool *ok )
return 0;
}

// Converts a string with values "0", "false", "1", "true" to 0 / 1
static int stringToBool( const QString &strVal, bool *ok )
{
if ( strVal.compare( QLatin1String( "0" ) ) == 0 || strVal.compare( QLatin1String( "false" ), Qt::CaseInsensitive ) == 0 )
{
*ok = true;
return 0;
}
else if ( strVal.compare( QLatin1String( "1" ) ) == 0 || strVal.compare( QLatin1String( "true" ), Qt::CaseInsensitive ) == 0 )
{
*ok = true;
return 1;
}
*ok = false;
return 0;
}


bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags, QgsFeatureId incrementalFeatureId )
{
bool returnValue = true;
Expand Down Expand Up @@ -1688,12 +1706,29 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags, QgsFeatureId
}
else
{
bool errorEmitted = false;
bool ok = false;
switch ( type )
{
case OFTInteger:
OGR_F_SetFieldInteger( feature.get(), ogrAttributeId, strictToInt( attrVal, &ok ) );
{
if ( OGR_Fld_GetSubType( fldDef ) == OFSTBoolean && qType == QVariant::String )
{
// compatibility with use case of https://github.com/qgis/QGIS/issues/55517
const QString strVal = attrVal.toString();
OGR_F_SetFieldInteger( feature.get(), ogrAttributeId, stringToBool( strVal, &ok ) );
if ( !ok )
{
pushError( tr( "wrong value for attribute %1 of feature %2: %3" ).arg( qgisAttributeId ) .arg( f.id() ).arg( strVal ) );
errorEmitted = true;
}
}
else
{
OGR_F_SetFieldInteger( feature.get(), ogrAttributeId, strictToInt( attrVal, &ok ) );
}
break;
}

case OFTInteger64:
OGR_F_SetFieldInteger64( feature.get(), ogrAttributeId, attrVal.toLongLong( &ok ) );
Expand Down Expand Up @@ -1895,7 +1930,10 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags, QgsFeatureId

if ( !ok )
{
pushError( tr( "wrong data type for attribute %1 of feature %2: %3" ).arg( qgisAttributeId ) .arg( f.id() ).arg( qType ) );
if ( !errorEmitted )
{
pushError( tr( "wrong data type for attribute %1 of feature %2: %3" ).arg( qgisAttributeId ) .arg( f.id() ).arg( attrVal.typeName() ) );
}
returnValue = false;
}
}
Expand Down Expand Up @@ -2636,12 +2674,29 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
}
else
{
bool errorEmitted = false;
bool ok = false;
switch ( type )
{
case OFTInteger:
OGR_F_SetFieldInteger( of.get(), f, strictToInt( *it2, &ok ) );
{
if ( OGR_Fld_GetSubType( fd ) == OFSTBoolean && qType == QVariant::String )
{
// compatibility with use case of https://github.com/qgis/QGIS/issues/55517
const QString strVal = it2->toString();
OGR_F_SetFieldInteger( of.get(), f, stringToBool( strVal, &ok ) );
if ( !ok )
{
pushError( tr( "wrong value for attribute %1 of feature %2: %3" ).arg( it2.key() ) . arg( fid ) .arg( strVal ) );
errorEmitted = true;
}
}
else
{
OGR_F_SetFieldInteger( of.get(), f, strictToInt( *it2, &ok ) );
}
break;
}
case OFTInteger64:
OGR_F_SetFieldInteger64( of.get(), f, it2->toLongLong( &ok ) );
break;
Expand Down Expand Up @@ -2837,7 +2892,10 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_

if ( !ok )
{
pushError( tr( "wrong data type for attribute %1 of feature %2: %3" ).arg( it2.key() ) . arg( fid ) .arg( qType ) );
if ( !errorEmitted )
{
pushError( tr( "wrong data type for attribute %1 of feature %2: %3" ).arg( it2.key() ) . arg( fid ) .arg( it2->typeName() ) );
}
returnValue = false;
}
}
Expand Down
89 changes: 89 additions & 0 deletions tests/src/python/test_provider_ogr_gpkg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2860,6 +2860,9 @@ def testChangeAttributeValuesErrors(self):
lyr.CreateField(ogr.FieldDefn('datetime_field', ogr.OFTDateTime))
lyr.CreateField(ogr.FieldDefn('date_field', ogr.OFTDateTime))
lyr.CreateField(ogr.FieldDefn('string_field', ogr.OFTString))
fld_defn = ogr.FieldDefn('bool_field', ogr.OFTInteger)
fld_defn.SetSubType(ogr.OFSTBoolean)
lyr.CreateField(fld_defn)
f = ogr.Feature(lyr.GetLayerDefn())
lyr.CreateFeature(f)
ds = None
Expand All @@ -2886,6 +2889,9 @@ def testChangeAttributeValuesErrors(self):
self.assertFalse(vl.dataProvider().changeAttributeValues({1: {4: "not a datetime"}}))
self.assertFalse(vl.dataProvider().changeAttributeValues({1: {5: "not a date"}}))

# wrong value for attribute 7 of feature 1: wrong
self.assertFalse(vl.dataProvider().changeAttributeValues({1: {7: "wrong"}}))

# OK
# int_field
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {1: 1}}))
Expand All @@ -2903,6 +2909,89 @@ def testChangeAttributeValuesErrors(self):
# string_field
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {6: "foo"}}))
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {6: 12345}}))
# bool field
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: True}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], True)
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: False}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], False)
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: 1}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], True)
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: 0}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], False)
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: "true"}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], True)
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: "false"}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], False)
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: "1"}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], True)
self.assertTrue(vl.dataProvider().changeAttributeValues({1: {7: "0"}}))
self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()][0], False)

def testAttributeBoolean(self):

tmpfile = os.path.join(self.basetestpath, 'testAttributeBoolean.gpkg')
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPoint)
fld_defn = ogr.FieldDefn('bool_field', ogr.OFTInteger)
fld_defn.SetSubType(ogr.OFSTBoolean)
lyr.CreateField(fld_defn)
ds = None

vl = QgsVectorLayer(f'{tmpfile}' + "|layername=" + "test", 'test', 'ogr')

f = QgsFeature(vl.fields())
f.setAttribute(1, False)
ret, _ = vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, True)
vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, 0)
ret, _ = vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, 1)
vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

# Test compatibility with use case of https://github.com/qgis/QGIS/issues/55517
f = QgsFeature(vl.fields())
f.setAttribute(1, "false")
ret, _ = vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, "true")
vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, "0")
ret, _ = vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, "1")
vl.dataProvider().addFeatures([f])
self.assertTrue(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, "invalid")
ret, _ = vl.dataProvider().addFeatures([f])
self.assertFalse(ret)

f = QgsFeature(vl.fields())
f.setAttribute(1, [1])
ret, _ = vl.dataProvider().addFeatures([f])
self.assertFalse(ret)

self.assertEqual([feat["bool_field"] for feat in vl.getFeatures()],
[False, True, False, True, False, True, False, True])

def testExtent(self):
# 2D points
Expand Down

0 comments on commit a25fef1

Please sign in to comment.