diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp index bd3cd35e8f3b..90fd15a9cc8a 100644 --- a/src/core/providers/ogr/qgsogrprovider.cpp +++ b/src/core/providers/ogr/qgsogrprovider.cpp @@ -1529,8 +1529,8 @@ QString QgsOgrProvider::jsonStringValue( const QVariant &value ) const bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) { bool returnValue = true; - QgsOgrFeatureDefn &fdef = mOgrLayer->GetLayerDefn(); - gdal::ogr_feature_unique_ptr feature( fdef.CreateFeature() ); + QgsOgrFeatureDefn &featureDefinition = mOgrLayer->GetLayerDefn(); + gdal::ogr_feature_unique_ptr feature( featureDefinition.CreateFeature() ); if ( f.hasGeometry() ) { @@ -1551,15 +1551,16 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) } } - QgsAttributes attrs = f.attributes(); + QgsAttributes attributes = f.attributes(); + const QgsFields qgisFields { f.fields() }; QgsLocaleNumC l; - int qgisAttId = ( mFirstFieldIsFid ) ? 1 : 0; + int qgisAttributeId = ( mFirstFieldIsFid ) ? 1 : 0; // If the first attribute is the FID and the user has set it, then use it - if ( mFirstFieldIsFid && attrs.count() > 0 ) + if ( mFirstFieldIsFid && attributes.count() > 0 ) { - QVariant attrFid = attrs.at( 0 ); + QVariant attrFid = attributes.at( 0 ); if ( !attrFid.isNull() ) { bool ok = false; @@ -1572,26 +1573,33 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) } //add possible attribute information - for ( int ogrAttId = 0; qgisAttId < attrs.count(); ++qgisAttId, ++ogrAttId ) + for ( int ogrAttributeId = 0; qgisAttributeId < attributes.count(); ++qgisAttributeId, ++ogrAttributeId ) { + // Skip fields that have no provider origin + if ( qgisFields.exists( qgisAttributeId ) && qgisFields.fieldOrigin( qgisAttributeId ) != QgsFields::FieldOrigin::OriginProvider ) + { + qgisAttributeId++; + continue; + } + // don't try to set field from attribute map if it's not present in layer - if ( ogrAttId >= fdef.GetFieldCount() ) + if ( ogrAttributeId >= featureDefinition.GetFieldCount() ) { - pushError( tr( "Feature has too many attributes (expecting %1, received %2)" ).arg( fdef.GetFieldCount() ).arg( f.attributes().count() ) ); + pushError( tr( "Feature has too many attributes (expecting %1, received %2)" ).arg( featureDefinition.GetFieldCount() ).arg( f.attributes().count() ) ); continue; } //if(!s.isEmpty()) // continue; // - OGRFieldDefnH fldDef = fdef.GetFieldDefn( ogrAttId ); + OGRFieldDefnH fldDef = featureDefinition.GetFieldDefn( ogrAttributeId ); OGRFieldType type = OGR_Fld_GetType( fldDef ); - QVariant attrVal = attrs.at( qgisAttId ); + QVariant attrVal = attributes.at( qgisAttributeId ); // The field value is equal to the default (that might be a provider-side expression) - if ( mDefaultValues.contains( qgisAttId ) && attrVal.toString() == mDefaultValues.value( qgisAttId ) ) + if ( mDefaultValues.contains( qgisAttributeId ) && attrVal.toString() == mDefaultValues.value( qgisAttributeId ) ) { - OGR_F_UnsetField( feature.get(), ogrAttId ); + OGR_F_UnsetField( feature.get(), ogrAttributeId ); } else if ( attrVal.isNull() || ( type != OFTString && attrVal.toString().isEmpty() ) ) { @@ -1603,7 +1611,7 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) // field to not be present at all in the output, and thus on reading to // have disappeared. #16812 #ifdef OGRNullMarker - OGR_F_SetFieldNull( feature.get(), ogrAttId ); + OGR_F_SetFieldNull( feature.get(), ogrAttributeId ); #else OGR_F_UnsetField( feature.get(), ogrAttId ); #endif @@ -1613,20 +1621,20 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) switch ( type ) { case OFTInteger: - OGR_F_SetFieldInteger( feature.get(), ogrAttId, attrVal.toInt() ); + OGR_F_SetFieldInteger( feature.get(), ogrAttributeId, attrVal.toInt() ); break; case OFTInteger64: - OGR_F_SetFieldInteger64( feature.get(), ogrAttId, attrVal.toLongLong() ); + OGR_F_SetFieldInteger64( feature.get(), ogrAttributeId, attrVal.toLongLong() ); break; case OFTReal: - OGR_F_SetFieldDouble( feature.get(), ogrAttId, attrVal.toDouble() ); + OGR_F_SetFieldDouble( feature.get(), ogrAttributeId, attrVal.toDouble() ); break; case OFTDate: - OGR_F_SetFieldDateTime( feature.get(), ogrAttId, + OGR_F_SetFieldDateTime( feature.get(), ogrAttributeId, attrVal.toDate().year(), attrVal.toDate().month(), attrVal.toDate().day(), @@ -1635,7 +1643,7 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) break; case OFTTime: - OGR_F_SetFieldDateTime( feature.get(), ogrAttId, + OGR_F_SetFieldDateTime( feature.get(), ogrAttributeId, 0, 0, 0, attrVal.toTime().hour(), attrVal.toTime().minute(), @@ -1644,7 +1652,7 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) break; case OFTDateTime: - OGR_F_SetFieldDateTime( feature.get(), ogrAttId, + OGR_F_SetFieldDateTime( feature.get(), ogrAttributeId, attrVal.toDateTime().date().year(), attrVal.toDateTime().date().month(), attrVal.toDateTime().date().day(), @@ -1669,16 +1677,16 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) stringValue = attrVal.toString(); #endif QgsDebugMsgLevel( QStringLiteral( "Writing string attribute %1 with %2, encoding %3" ) - .arg( qgisAttId ) + .arg( qgisAttributeId ) .arg( attrVal.toString(), textEncoding()->name().data() ), 3 ); - OGR_F_SetFieldString( feature.get(), ogrAttId, textEncoding()->fromUnicode( stringValue ).constData() ); + OGR_F_SetFieldString( feature.get(), ogrAttributeId, textEncoding()->fromUnicode( stringValue ).constData() ); break; } case OFTBinary: { const QByteArray ba = attrVal.toByteArray(); - OGR_F_SetFieldBinary( feature.get(), ogrAttId, ba.size(), const_cast< GByte * >( reinterpret_cast< const GByte * >( ba.data() ) ) ); + OGR_F_SetFieldBinary( feature.get(), ogrAttributeId, ba.size(), const_cast< GByte * >( reinterpret_cast< const GByte * >( ba.data() ) ) ); break; } @@ -1698,13 +1706,13 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) } } lst[count] = nullptr; - OGR_F_SetFieldStringList( feature.get(), ogrAttId, lst ); + OGR_F_SetFieldStringList( feature.get(), ogrAttributeId, lst ); break; } #endif default: - QgsMessageLog::logMessage( tr( "type %1 for attribute %2 not found" ).arg( type ).arg( qgisAttId ), tr( "OGR" ) ); + QgsMessageLog::logMessage( tr( "type %1 for attribute %2 not found" ).arg( type ).arg( qgisAttributeId ), tr( "OGR" ) ); break; } } @@ -1722,7 +1730,7 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags ) { f.setId( id ); - if ( mFirstFieldIsFid && attrs.count() > 0 ) + if ( mFirstFieldIsFid && attributes.count() > 0 ) { f.setAttribute( 0, id ); } diff --git a/tests/src/python/test_provider_ogr.py b/tests/src/python/test_provider_ogr.py index 3bef0bc057cd..40255374d352 100644 --- a/tests/src/python/test_provider_ogr.py +++ b/tests/src/python/test_provider_ogr.py @@ -19,12 +19,23 @@ from osgeo import gdal, ogr # NOQA from qgis.PyQt.QtCore import QVariant, QByteArray -from qgis.core import (NULL, - QgsApplication, - QgsRectangle, - QgsProviderRegistry, - QgsFeature, QgsFeatureRequest, QgsField, QgsSettings, QgsDataProvider, - QgsVectorDataProvider, QgsVectorLayer, QgsWkbTypes, QgsNetworkAccessManager) +from qgis.core import ( + NULL, + QgsApplication, + QgsProject, + QgsField, + QgsFields, + QgsRectangle, + QgsProviderRegistry, + QgsFeature, + QgsFeatureRequest, + QgsSettings, + QgsDataProvider, + QgsVectorDataProvider, + QgsVectorLayer, + QgsWkbTypes, + QgsNetworkAccessManager +) from qgis.testing import start_app, unittest from utilities import unitTestDataPath @@ -712,6 +723,39 @@ def testMixOfFilterExpressionAndSubsetStringWhenFilterExpressionCompilationFails self.assertCountEqual([f.attributes() for f in vl.getFeatures(request)], [['rectangle', '1']]) + def testTransactionGroupExpressionFields(self): + """Test issue GH #39230, this is not really specific to GPKG""" + + project = QgsProject() + project.setAutoTransaction(True) + tmpfile = os.path.join( + self.basetestpath, 'tempGeoPackageTransactionExpressionFields.gpkg') + ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile) + lyr = ds.CreateLayer('test', geom_type=ogr.wkbPoint) + lyr.CreateField(ogr.FieldDefn('str_field', ogr.OFTString)) + + f = ogr.Feature(lyr.GetLayerDefn()) + f.SetGeometry(ogr.CreateGeometryFromWkt('POINT (1 1)')) + f.SetField('str_field', 'one') + lyr.CreateFeature(f) + + del lyr + del ds + + vl = QgsVectorLayer(tmpfile + '|layername=test', 'test', 'ogr') + f = QgsField('expression_field', QVariant.Int) + idx = vl.addExpressionField('123', f) + self.assertEqual(vl.fields().fieldOrigin(idx), QgsFields.OriginExpression) + + project.addMapLayers([vl]) + + feature = next(vl.getFeatures()) + feature.setAttributes([None, 'two', 123]) + + self.assertTrue(vl.startEditing()) + self.assertTrue(vl.addFeature(feature)) + self.assertFalse(vl.dataProvider().hasErrors()) + if __name__ == '__main__': unittest.main()