From f2f9d3a9313da6a25fb25717ecd268d38dad15cb Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 21 Jan 2025 12:40:53 +0100 Subject: [PATCH 1/4] [OGR] Fix transactional editing for GPKG/SQLite Tell the provider to reload the fields after a rollback and add some checks to verify if after the rollback the provider still needs to update the field. Followup https://github.com/OSGeo/gdal/pull/11695 Followup https://github.com/qgis/QGIS/pull/59797 --- .../core/auto_additions/qgstransaction.py | 4 +- .../core/auto_generated/qgstransaction.sip.in | 7 +++ python/core/auto_additions/qgstransaction.py | 4 +- .../core/auto_generated/qgstransaction.sip.in | 7 +++ src/core/providers/ogr/qgsogrprovider.cpp | 13 +++++ src/core/providers/ogr/qgsogrprovider.h | 1 + src/core/qgstransaction.cpp | 7 ++- src/core/qgstransaction.h | 6 +++ .../qgsvectorlayerundopassthroughcommand.cpp | 52 ++++++++++++++++--- .../python/test_qgsvectorlayereditbuffer.py | 3 +- 10 files changed, 90 insertions(+), 14 deletions(-) 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..0f08b6bec592 100644 --- a/src/core/providers/ogr/qgsogrprovider.cpp +++ b/src/core/providers/ogr/qgsogrprovider.cpp @@ -544,6 +544,14 @@ 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 ); + connect( mTransaction, &QgsTransaction::afterRollback, this, [ = ]( ) + { + mFieldsRequireReload = true; + } ); + connect( mTransaction, &QgsTransaction::afterRollbackToSavepoint, this, [ = ]( const QString & ) + { + mFieldsRequireReload = true; + } ); } QgsAbstractFeatureSource *QgsOgrProvider::featureSource() const @@ -1073,6 +1081,7 @@ void QgsOgrProvider::loadFields() mAttributeFields.append( newField ); createdFields++; } + mFieldsRequireReload = false; } void QgsOgrProvider::loadMetadata() @@ -1604,6 +1613,10 @@ long long QgsOgrProvider::featureCount() const QgsFields QgsOgrProvider::fields() const { + if ( mFieldsRequireReload ) + { + const_cast( this )->loadFields(); + } return mAttributeFields; } diff --git a/src/core/providers/ogr/qgsogrprovider.h b/src/core/providers/ogr/qgsogrprovider.h index e8d7a45854fe..924d529f855a 100644 --- a/src/core/providers/ogr/qgsogrprovider.h +++ b/src/core/providers/ogr/qgsogrprovider.h @@ -221,6 +221,7 @@ class QgsOgrProvider final: public QgsVectorDataProvider mutable std::unique_ptr< OGREnvelope > mExtent2D; mutable std::unique_ptr< OGREnvelope3D > mExtent3D; bool mForceRecomputeExtent = false; + bool mFieldsRequireReload = false; 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..26afe1aa6096 100644 --- a/tests/src/python/test_qgsvectorlayereditbuffer.py +++ b/tests/src/python/test_qgsvectorlayereditbuffer.py @@ -851,7 +851,8 @@ 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) From eaf5e00dd65c88687825a0072f3c6ea7b472b6a9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 21 Jan 2025 11:48:04 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/src/python/test_qgsvectorlayereditbuffer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/src/python/test_qgsvectorlayereditbuffer.py b/tests/src/python/test_qgsvectorlayereditbuffer.py index 26afe1aa6096..7358ac9f121f 100644 --- a/tests/src/python/test_qgsvectorlayereditbuffer.py +++ b/tests/src/python/test_qgsvectorlayereditbuffer.py @@ -852,7 +852,9 @@ 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 # 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): + 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) From 2ac0a60f61b810bb5ed7ca885a26a87f4b648fc9 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 21 Jan 2025 17:07:23 +0100 Subject: [PATCH 3/4] Fix deprecated this capture --- src/core/providers/ogr/qgsogrprovider.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp index 0f08b6bec592..0868115059aa 100644 --- a/src/core/providers/ogr/qgsogrprovider.cpp +++ b/src/core/providers/ogr/qgsogrprovider.cpp @@ -544,11 +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 ); - connect( mTransaction, &QgsTransaction::afterRollback, this, [ = ]( ) + connect( mTransaction, &QgsTransaction::afterRollback, this, [ this ]( ) { mFieldsRequireReload = true; } ); - connect( mTransaction, &QgsTransaction::afterRollbackToSavepoint, this, [ = ]( const QString & ) + connect( mTransaction, &QgsTransaction::afterRollbackToSavepoint, this, [ this ]( const QString & ) { mFieldsRequireReload = true; } ); From 61c30dae583817befef0f396f049ee1155f282a8 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 22 Jan 2025 15:16:18 +0100 Subject: [PATCH 4/4] Call fields() when it might be invalidated --- src/core/providers/ogr/qgsogrprovider.cpp | 64 ++++++++++++++--------- src/core/providers/ogr/qgsogrprovider.h | 6 +++ 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp index 0868115059aa..5cbbfe351fed 100644 --- a/src/core/providers/ogr/qgsogrprovider.cpp +++ b/src/core/providers/ogr/qgsogrprovider.cpp @@ -544,14 +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 ); - connect( mTransaction, &QgsTransaction::afterRollback, this, [ this ]( ) + if ( transaction ) { - mFieldsRequireReload = true; - } ); - connect( mTransaction, &QgsTransaction::afterRollbackToSavepoint, this, [ this ]( const QString & ) - { - mFieldsRequireReload = true; - } ); + connect( mTransaction, &QgsTransaction::afterRollback, this, &QgsOgrProvider::afterRollback ); + connect( mTransaction, &QgsTransaction::afterRollbackToSavepoint, this, &QgsOgrProvider::afterRollbackToSavepoint ); + } } QgsAbstractFeatureSource *QgsOgrProvider::featureSource() const @@ -734,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 ); @@ -1214,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 @@ -1461,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() ); @@ -1525,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(); } @@ -1816,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: @@ -1828,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; @@ -2046,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; } @@ -2317,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 @@ -2426,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() ) ); @@ -3373,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() ) @@ -3764,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 @@ -3829,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 @@ -4110,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(); } @@ -4118,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 @@ -4170,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(); } @@ -4178,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 @@ -4721,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 924d529f855a..30fd17baf71e 100644 --- a/src/core/providers/ogr/qgsogrprovider.h +++ b/src/core/providers/ogr/qgsogrprovider.h @@ -221,8 +221,14 @@ class QgsOgrProvider final: public QgsVectorDataProvider mutable std::unique_ptr< OGREnvelope > mExtent2D; 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; /**