Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OGR] Fix transactional editing for GPKG/SQLite #60198

Merged
merged 4 commits into from
Jan 22, 2025
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions python/PyQt6/core/auto_additions/qgstransaction.py
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions python/PyQt6/core/auto_generated/qgstransaction.sip.in
Original file line number Diff line number Diff line change
@@ -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 );
4 changes: 2 additions & 2 deletions python/core/auto_additions/qgstransaction.py
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions python/core/auto_generated/qgstransaction.sip.in
Original file line number Diff line number Diff line change
@@ -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 );
13 changes: 13 additions & 0 deletions src/core/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
@@ -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<QgsOgrTransaction *>( transaction );
connect( mTransaction, &QgsTransaction::afterRollback, this, [ this ]( )
{
mFieldsRequireReload = true;
} );
connect( mTransaction, &QgsTransaction::afterRollbackToSavepoint, this, [ 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<QgsOgrProvider *>( this )->loadFields();
}
return mAttributeFields;
}

1 change: 1 addition & 0 deletions src/core/providers/ogr/qgsogrprovider.h
Original file line number Diff line number Diff line change
@@ -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<int> mPrimaryKeyAttrs;

7 changes: 6 additions & 1 deletion src/core/qgstransaction.cpp
Original file line number Diff line number Diff line change
@@ -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()
6 changes: 6 additions & 0 deletions src/core/qgstransaction.h
Original file line number Diff line number Diff line change
@@ -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
*/
52 changes: 44 additions & 8 deletions src/core/vector/qgsvectorlayerundopassthroughcommand.cpp
Original file line number Diff line number Diff line change
@@ -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<QgsField>() << 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<QgsField>() << 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
{
5 changes: 4 additions & 1 deletion tests/src/python/test_qgsvectorlayereditbuffer.py
Original file line number Diff line number Diff line change
@@ -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)