diff --git a/python/PyQt6/core/auto_additions/qgstransaction.py b/python/PyQt6/core/auto_additions/qgstransaction.py index 180403b80ca6..284cc9fa4347 100644 --- a/python/PyQt6/core/auto_additions/qgstransaction.py +++ b/python/PyQt6/core/auto_additions/qgstransaction.py @@ -1,8 +1,8 @@ # The following has been generated automatically from src/core/qgstransaction.h try: - QgsTransaction.__attribute_docs__ = {'afterRollback': 'Emitted after a rollback\n', 'dirtied': 'Emitted if a sql query is executed and the underlying data is modified\n'} + QgsTransaction.__attribute_docs__ = {'afterRollback': 'Emitted after a rollback\n', 'afterRollbackToSavepoint': 'Emitted after a rollback to savepoint\n\n.. versionadded:: 3.42\n', 'dirtied': 'Emitted if a sql query is executed and the underlying data is modified\n'} QgsTransaction.create = staticmethod(QgsTransaction.create) QgsTransaction.supportsTransaction = staticmethod(QgsTransaction.supportsTransaction) - QgsTransaction.__signal_arguments__ = {'dirtied': ['sql: str', 'name: str']} + QgsTransaction.__signal_arguments__ = {'afterRollbackToSavepoint': ['savepointName: str'], 'dirtied': ['sql: str', 'name: str']} except (NameError, AttributeError): pass diff --git a/python/PyQt6/core/auto_generated/qgstransaction.sip.in b/python/PyQt6/core/auto_generated/qgstransaction.sip.in index 412929f07859..b5f528812390 100644 --- a/python/PyQt6/core/auto_generated/qgstransaction.sip.in +++ b/python/PyQt6/core/auto_generated/qgstransaction.sip.in @@ -152,6 +152,13 @@ returns the last created savepoint void afterRollback(); %Docstring Emitted after a rollback +%End + + void afterRollbackToSavepoint( const QString &savepointName ); +%Docstring +Emitted after a rollback to savepoint + +.. versionadded:: 3.42 %End void dirtied( const QString &sql, const QString &name ); diff --git a/python/core/auto_additions/qgstransaction.py b/python/core/auto_additions/qgstransaction.py index 180403b80ca6..284cc9fa4347 100644 --- a/python/core/auto_additions/qgstransaction.py +++ b/python/core/auto_additions/qgstransaction.py @@ -1,8 +1,8 @@ # The following has been generated automatically from src/core/qgstransaction.h try: - QgsTransaction.__attribute_docs__ = {'afterRollback': 'Emitted after a rollback\n', 'dirtied': 'Emitted if a sql query is executed and the underlying data is modified\n'} + QgsTransaction.__attribute_docs__ = {'afterRollback': 'Emitted after a rollback\n', 'afterRollbackToSavepoint': 'Emitted after a rollback to savepoint\n\n.. versionadded:: 3.42\n', 'dirtied': 'Emitted if a sql query is executed and the underlying data is modified\n'} QgsTransaction.create = staticmethod(QgsTransaction.create) QgsTransaction.supportsTransaction = staticmethod(QgsTransaction.supportsTransaction) - QgsTransaction.__signal_arguments__ = {'dirtied': ['sql: str', 'name: str']} + QgsTransaction.__signal_arguments__ = {'afterRollbackToSavepoint': ['savepointName: str'], 'dirtied': ['sql: str', 'name: str']} except (NameError, AttributeError): pass diff --git a/python/core/auto_generated/qgstransaction.sip.in b/python/core/auto_generated/qgstransaction.sip.in index 412929f07859..b5f528812390 100644 --- a/python/core/auto_generated/qgstransaction.sip.in +++ b/python/core/auto_generated/qgstransaction.sip.in @@ -152,6 +152,13 @@ returns the last created savepoint void afterRollback(); %Docstring Emitted after a rollback +%End + + void afterRollbackToSavepoint( const QString &savepointName ); +%Docstring +Emitted after a rollback to savepoint + +.. versionadded:: 3.42 %End void dirtied( const QString &sql, const QString &name ); diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp index 0c72d6337609..5cbbfe351fed 100644 --- a/src/core/providers/ogr/qgsogrprovider.cpp +++ b/src/core/providers/ogr/qgsogrprovider.cpp @@ -544,6 +544,11 @@ void QgsOgrProvider::setTransaction( QgsTransaction *transaction ) QgsDebugMsgLevel( QStringLiteral( "set transaction %1" ).arg( transaction != nullptr ), 2 ); // static_cast since layers cannot be added to a transaction of a non-matching provider mTransaction = static_cast( transaction ); + if ( transaction ) + { + connect( mTransaction, &QgsTransaction::afterRollback, this, &QgsOgrProvider::afterRollback ); + connect( mTransaction, &QgsTransaction::afterRollbackToSavepoint, this, &QgsOgrProvider::afterRollbackToSavepoint ); + } } QgsAbstractFeatureSource *QgsOgrProvider::featureSource() const @@ -726,6 +731,17 @@ QList QgsOgrProvider::_subLayers( Qgis::SublayerQuer return mSubLayerList; } +void QgsOgrProvider::afterRollbackToSavepoint( const QString &savepointName ) +{ + Q_UNUSED( savepointName ); + mFieldsRequireReload = true; +} + +void QgsOgrProvider::afterRollback() +{ + mFieldsRequireReload = true; +} + void QgsOgrProvider::setEncoding( const QString &e ) { QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg ); @@ -1073,6 +1089,7 @@ void QgsOgrProvider::loadFields() mAttributeFields.append( newField ); createdFields++; } + mFieldsRequireReload = false; } void QgsOgrProvider::loadMetadata() @@ -1205,7 +1222,7 @@ void QgsOgrProvider::setRelevantFields( bool fetchGeometry, const QgsAttributeLi QRecursiveMutex *mutex = nullptr; OGRLayerH ogrLayer = mOgrLayer->getHandleAndMutex( mutex ); QMutexLocker locker( mutex ); - QgsOgrProviderUtils::setRelevantFields( ogrLayer, mAttributeFields.count(), fetchGeometry, fetchAttributes, mFirstFieldIsFid, QgsOgrProviderUtils::cleanSubsetString( mSubsetString ) ); + QgsOgrProviderUtils::setRelevantFields( ogrLayer, fields().count(), fetchGeometry, fetchAttributes, mFirstFieldIsFid, QgsOgrProviderUtils::cleanSubsetString( mSubsetString ) ); } QgsFeatureIterator QgsOgrProvider::getFeatures( const QgsFeatureRequest &request ) const @@ -1452,7 +1469,7 @@ QVariant QgsOgrProvider::defaultValue( int fieldId ) const QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg ); QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrProvider" ) ); - if ( fieldId < 0 || fieldId >= mAttributeFields.count() ) + if ( fieldId < 0 || fieldId >= fields().count() ) return QVariant(); QString defaultVal = mDefaultValues.value( fieldId, QString() ); @@ -1516,7 +1533,7 @@ QVariant QgsOgrProvider::defaultValue( int fieldId ) const } } - const bool compatible = mAttributeFields.at( fieldId ).convertCompatible( resultVar ); + const bool compatible = fields().at( fieldId ).convertCompatible( resultVar ); return compatible && !QgsVariantUtils::isNull( resultVar ) ? resultVar : QVariant(); } @@ -1604,6 +1621,10 @@ long long QgsOgrProvider::featureCount() const QgsFields QgsOgrProvider::fields() const { + if ( mFieldsRequireReload ) + { + const_cast( this )->loadFields(); + } return mAttributeFields; } @@ -1803,6 +1824,8 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags, QgsFeatureId { bool errorEmitted = false; bool ok = false; + // Get an updated copy + const QgsFields fieldsCopy { f.fields() }; switch ( type ) { case OFTInteger: @@ -1815,7 +1838,7 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags, QgsFeatureId if ( !ok ) { pushError( tr( "wrong value for attribute %1 of feature %2: %3" ) - .arg( mAttributeFields.at( qgisAttributeId ).name() ) + .arg( fieldsCopy.at( qgisAttributeId ).name() ) .arg( f.id() ) .arg( strVal ) ); errorEmitted = true; @@ -2033,9 +2056,9 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags, QgsFeatureId if ( !errorEmitted ) { pushError( tr( "wrong data type for attribute %1 of feature %2: Got %3, expected %4" ) - .arg( mAttributeFields.at( qgisAttributeId ).name() ) + .arg( fieldsCopy.at( qgisAttributeId ).name() ) .arg( f.id() ) - .arg( attrVal.typeName(), QVariant::typeToName( mAttributeFields.at( qgisAttributeId ).type() ) ) ); + .arg( attrVal.typeName(), QVariant::typeToName( fieldsCopy.at( qgisAttributeId ).type() ) ) ); } returnValue = false; } @@ -2304,8 +2327,9 @@ bool QgsOgrProvider::addAttributes( const QList &attributes ) } // Backup existing fields. We need them to 'restore' field type, length, precision - QgsFields oldFields = mAttributeFields; + QgsFields oldFields = fields(); + // Updates mAttributeFields loadFields(); // The check in QgsVectorLayerEditBuffer::commitChanges() is questionable with @@ -2413,13 +2437,13 @@ bool QgsOgrProvider::renameAttributes( const QgsFieldNameMap &renamedAttributes for ( ; renameIt != renamedAttributes.constEnd(); ++renameIt ) { int fieldIndex = renameIt.key(); - if ( fieldIndex < 0 || fieldIndex >= mAttributeFields.count() ) + if ( fieldIndex < 0 || fieldIndex >= fields().count() ) { pushError( tr( "Invalid attribute index" ) ); result = false; continue; } - if ( mAttributeFields.indexFromName( renameIt.value() ) >= 0 ) + if ( fields().indexFromName( renameIt.value() ) >= 0 ) { //field name already in use pushError( tr( "Error renaming field %1: name '%2' already exists" ).arg( fieldIndex ).arg( renameIt.value() ) ); @@ -3360,7 +3384,7 @@ bool QgsOgrProvider::createAttributeIndex( int field ) QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg ); QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrProvider" ) ); - if ( field < 0 || field >= mAttributeFields.count() ) + if ( field < 0 || field >= fields().count() ) return false; if ( !doInitialActionsForEdition() ) @@ -3751,10 +3775,10 @@ QSet QgsOgrProvider::uniqueValues( int index, int limit ) const { QSet uniqueValues; - if ( !mValid || index < 0 || index >= mAttributeFields.count() ) + if ( !mValid || index < 0 || index >= fields().count() ) return uniqueValues; - const QgsField fld = mAttributeFields.at( index ); + const QgsField fld = fields().at( index ); if ( fld.name().isNull() ) { return uniqueValues; //not a provider field @@ -3816,10 +3840,10 @@ QStringList QgsOgrProvider::uniqueStringsMatching( int index, const QString &sub { QStringList results; - if ( !mValid || index < 0 || index >= mAttributeFields.count() ) + if ( !mValid || index < 0 || index >= fields().count() ) return results; - QgsField fld = mAttributeFields.at( index ); + QgsField fld = fields().at( index ); if ( fld.name().isNull() ) { return results; //not a provider field @@ -4097,7 +4121,7 @@ QList QgsOgrProvider::discoverRelations( const QgsVectorLayer *targ QVariant QgsOgrProvider::minimumValue( int index ) const { - if ( !mValid || index < 0 || index >= mAttributeFields.count() ) + if ( !mValid || index < 0 || index >= fields().count() ) { return QVariant(); } @@ -4105,7 +4129,7 @@ QVariant QgsOgrProvider::minimumValue( int index ) const QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg ); QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrProvider" ) ); - const QgsField originalField = mAttributeFields.at( index ); + const QgsField originalField = fields().at( index ); QgsField fld = originalField; // can't use native date/datetime types -- OGR converts these to string in the MAX return value @@ -4157,7 +4181,7 @@ QVariant QgsOgrProvider::minimumValue( int index ) const QVariant QgsOgrProvider::maximumValue( int index ) const { - if ( !mValid || index < 0 || index >= mAttributeFields.count() ) + if ( !mValid || index < 0 || index >= fields().count() ) { return QVariant(); } @@ -4165,7 +4189,7 @@ QVariant QgsOgrProvider::maximumValue( int index ) const QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg ); QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrProvider" ) ); - const QgsField originalField = mAttributeFields.at( index ); + const QgsField originalField = fields().at( index ); QgsField fld = originalField; // can't use native date/datetime types -- OGR converts these to string in the MAX return value @@ -4708,7 +4732,10 @@ bool QgsOgrProvider::leaveUpdateMode() // Backup fields since if we created new fields, but didn't populate it // with any feature yet, it will disappear. const QgsFields oldFields = mAttributeFields; + + // This will also update mAttributeFields reloadData(); + if ( mValid ) { // Make sure that new fields we added, but didn't populate yet, are diff --git a/src/core/providers/ogr/qgsogrprovider.h b/src/core/providers/ogr/qgsogrprovider.h index e8d7a45854fe..30fd17baf71e 100644 --- a/src/core/providers/ogr/qgsogrprovider.h +++ b/src/core/providers/ogr/qgsogrprovider.h @@ -222,6 +222,13 @@ class QgsOgrProvider final: public QgsVectorDataProvider mutable std::unique_ptr< OGREnvelope3D > mExtent3D; bool mForceRecomputeExtent = false; + //! Flag set after a rollback to indicate that fields require reloading + bool mFieldsRequireReload = false; + + //! Called after a transaction rollback + void afterRollback(); + void afterRollbackToSavepoint( const QString &savePointName ); + QList mPrimaryKeyAttrs; /** diff --git a/src/core/qgstransaction.cpp b/src/core/qgstransaction.cpp index 714bf24d0c43..5b2d4de9ddbf 100644 --- a/src/core/qgstransaction.cpp +++ b/src/core/qgstransaction.cpp @@ -271,7 +271,12 @@ bool QgsTransaction::rollbackToSavepoint( const QString &name, QString &error SI // the status of the DB has changed between the previous savepoint and the // one we are rolling back to. mLastSavePointIsDirty = true; - return executeSql( QStringLiteral( "ROLLBACK TO SAVEPOINT %1" ).arg( QgsExpression::quotedColumnRef( name ) ), error ); + if ( ! executeSql( QStringLiteral( "ROLLBACK TO SAVEPOINT %1" ).arg( QgsExpression::quotedColumnRef( name ) ), error ) ) + { + return false; + } + emit afterRollbackToSavepoint( name ); + return true; } void QgsTransaction::dirtyLastSavePoint() diff --git a/src/core/qgstransaction.h b/src/core/qgstransaction.h index e14ee719392b..1ed5ed08369b 100644 --- a/src/core/qgstransaction.h +++ b/src/core/qgstransaction.h @@ -173,6 +173,12 @@ class CORE_EXPORT QgsTransaction : public QObject SIP_ABSTRACT */ void afterRollback(); + /** + * Emitted after a rollback to savepoint + * \since QGIS 3.42 + */ + void afterRollbackToSavepoint( const QString &savepointName ); + /** * Emitted if a sql query is executed and the underlying data is modified */ diff --git a/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp b/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp index 3b824bf0a5e9..36d984a4da3f 100644 --- a/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp +++ b/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp @@ -393,7 +393,13 @@ void QgsVectorLayerUndoPassthroughCommandAddAttribute::undo() const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() ); if ( rollBackToSavePoint() ) { - mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ); + // GDAL SQLite-based drivers (since version 3.11) keep the fields in sync with + // the backend after a rollback, to stay on the safe side check if the field + // isn't already gone + if ( mBuffer->L->dataProvider()->fieldNameIndex( mField.name() ) != -1 ) + { + mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ); + } mBuffer->mAddedAttributes.removeAll( mField ); mBuffer->updateLayerFields(); emit mBuffer->attributeDeleted( attr ); @@ -432,11 +438,26 @@ void QgsVectorLayerUndoPassthroughCommandDeleteAttribute::undo() // note that the addAttributes here is only necessary to inform the provider that // an attribute is added back after the rollBackToSavePoint mBuffer->L->dataProvider()->clearErrors(); - if ( mBuffer->L->dataProvider()->addAttributes( QList() << mField ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() ) + if ( rollBackToSavePoint() ) { - mBuffer->mDeletedAttributeIds.removeOne( mOriginalFieldIndex ); - mBuffer->updateLayerFields(); - emit mBuffer->attributeAdded( mOriginalFieldIndex ); + // GDA SQLite-based drivers (since version 3.11) keep the fields in sync with + // the backend after a rollback, to stay on the safe side check if the field + // isn't already there + bool ok = true; + if ( mBuffer->L->dataProvider()->fields().indexFromName( mField.name() ) == -1 ) + { + ok = mBuffer->L->dataProvider()->addAttributes( QList() << mField ); + } + if ( ok && ! mBuffer->L->dataProvider()->hasErrors() ) + { + mBuffer->mDeletedAttributeIds.removeOne( mOriginalFieldIndex ); + mBuffer->updateLayerFields(); + emit mBuffer->attributeAdded( mOriginalFieldIndex ); + } + else + { + setError(); + } } else { @@ -474,10 +495,25 @@ void QgsVectorLayerUndoPassthroughCommandRenameAttribute::undo() QgsFieldNameMap map; map[ mAttr ] = mOldName; mBuffer->L->dataProvider()->clearErrors(); - if ( mBuffer->L->dataProvider()->renameAttributes( map ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() ) + if ( rollBackToSavePoint() ) { - mBuffer->updateLayerFields(); - emit mBuffer->attributeRenamed( mAttr, mOldName ); + // GDAL SQLite-based drivers (since version 3.11) keep the fields in sync with + // the backend after a rollback, to stay on the safe side check if the field + // isn't already renamed + bool ok = true; + if ( mBuffer->L->dataProvider()->fields().indexFromName( mOldName ) == -1 ) + { + ok = mBuffer->L->dataProvider()->renameAttributes( map ); + } + if ( ok && ! mBuffer->L->dataProvider()->hasErrors() ) + { + mBuffer->updateLayerFields(); + emit mBuffer->attributeRenamed( mAttr, mOldName ); + } + else + { + setError(); + } } else { diff --git a/tests/src/python/test_qgsvectorlayereditbuffer.py b/tests/src/python/test_qgsvectorlayereditbuffer.py index 196f20052d94..7358ac9f121f 100644 --- a/tests/src/python/test_qgsvectorlayereditbuffer.py +++ b/tests/src/python/test_qgsvectorlayereditbuffer.py @@ -851,7 +851,10 @@ def _check_feature(wkt): # THIS FUNCTIONALITY IS BROKEN ON NEWER GDAL VERSIONS, DUE TO INCORRECT # assumptions at time of development. See https://github.com/qgis/QGIS/pull/59797#issuecomment-2544133498 - if int(gdal.VersionInfo("VERSION_NUM")) < GDAL_COMPUTE_VERSION(3, 5, 0): + # see also: https://github.com/OSGeo/gdal/pull/11695 for a GDAL 3.11 fix + if int(gdal.VersionInfo("VERSION_NUM")) < GDAL_COMPUTE_VERSION(3, 5, 0) or int( + gdal.VersionInfo("VERSION_NUM") + ) >= GDAL_COMPUTE_VERSION(3, 11, 0): _test(Qgis.TransactionMode.AutomaticGroups) _test(Qgis.TransactionMode.BufferedGroups)