Skip to content
Permalink
Browse files

[OGR provider] [FEATURE] Add support for transactions on GPKG databases

For complete support, it requires two GDAL fixes:
- One to avoid feature count to be invalid when using ROLLBACK TO SAVEPOINT
  OSGeo/gdal@f73ec8c

- Another one to avoid nasty issues, at least on Linux, with the POSIX
  advisory locks used by libsqlite that could be invalidated due to how GDAL
  could open files behind the back of libsqlite. The consequence of this
  could be the deletion of -wal and -shm files, which caused issues in QGIS
  (non working iterators when the edit is finished, and later edits in the
  same session not working). Those issues could appear for example if doing
  ogrinfo on the .gpkg opened by QGIS, or if opening two QGIS session on the
  .gpkg

Both fixes are queued for GDAL 2.3.1
  • Loading branch information
rouault committed Jun 14, 2018
1 parent 370bac9 commit 6a987f913bbbc4b7a53ed4016a1ea26a0b9a2565
@@ -146,6 +146,7 @@ returns the last created savepoint
.. versionadded:: 3.0
%End


signals:

void afterRollback();
@@ -51,7 +51,7 @@ QgsTransaction *QgsTransaction::create( const QSet<QgsVectorLayer *> &layers )

QgsVectorLayer *firstLayer = *layers.constBegin();

QString connStr = QgsDataSourceUri( firstLayer->source() ).connectionInfo( false );
QString connStr = connectionString( firstLayer->source() );
QString providerKey = firstLayer->dataProvider()->name();
std::unique_ptr<QgsTransaction> transaction( QgsTransaction::create( connStr, providerKey ) );
if ( transaction )
@@ -81,6 +81,46 @@ QgsTransaction::~QgsTransaction()
setLayerTransactionIds( nullptr );
}

// For the needs of the OGR provider with GeoPackage datasources, remove
// any reference to layers in the connection string
QString QgsTransaction::removeLayerIdOrName( const QString &str )
{
QString res( str );

for ( int i = 0; i < 2; i++ )
{
int pos = res.indexOf( i == 0 ? QLatin1String( "|layername=" ) : QLatin1String( "|layerid=" ) );
if ( pos >= 0 )
{
int end = res.indexOf( '|', pos + 1 );
if ( end >= 0 )
{
res = res.mid( 0, pos ) + res.mid( end );
}
else
{
res = res.mid( 0, pos );
}
}
}
return res;
}

///@cond PRIVATE
QString QgsTransaction::connectionString( const QString &layerName )
{
QString connString = QgsDataSourceUri( layerName ).connectionInfo( false );
// In the case of a OGR datasource, connectionInfo() will return an empty
// string. In that case, use the layer->source() itself, and strip any
// reference to layers from it.
if ( connString.isEmpty() )
{
connString = removeLayerIdOrName( layerName );
}
return connString;
}
///@endcond

bool QgsTransaction::addLayer( QgsVectorLayer *layer )
{
if ( !layer )
@@ -90,17 +130,18 @@ bool QgsTransaction::addLayer( QgsVectorLayer *layer )
return false;

//test if provider supports transactions
if ( !layer->dataProvider() || ( layer->dataProvider()->capabilities() & QgsVectorDataProvider::TransactionSupport ) == 0 )
if ( !supportsTransaction( layer ) )
return false;

if ( layer->dataProvider()->transaction() )
return false;

//connection string not compatible
if ( QgsDataSourceUri( layer->source() ).connectionInfo( false ) != mConnString )

if ( connectionString( layer->source() ) != mConnString )
{
QgsDebugMsg( QString( "Couldn't start transaction because connection string for layer %1 : '%2' does not match '%3'" ).arg(
layer->id(), QgsDataSourceUri( layer->source() ).connectionInfo( false ), mConnString ) );
layer->id(), connectionString( layer->source() ), mConnString ) );
return false;
}

@@ -162,11 +203,11 @@ bool QgsTransaction::rollback( QString &errorMsg )

bool QgsTransaction::supportsTransaction( const QgsVectorLayer *layer )
{
std::unique_ptr< QLibrary > lib( QgsProviderRegistry::instance()->createProviderLibrary( layer->providerType() ) );
if ( !lib )
//test if provider supports transactions
if ( !layer->dataProvider() || ( layer->dataProvider()->capabilities() & QgsVectorDataProvider::TransactionSupport ) == 0 )
return false;

return lib->resolve( "createTransaction" );
return true;
}

void QgsTransaction::onLayerDeleted()
@@ -158,6 +158,11 @@ class CORE_EXPORT QgsTransaction : public QObject SIP_ABSTRACT
*/
bool lastSavePointIsDirty() const { return mLastSavePointIsDirty; }

///@cond PRIVATE
// For internal use only, or by QgsTransactionGroup
static QString connectionString( const QString &layerName ) SIP_SKIP;
///@endcond

signals:

/**
@@ -188,6 +193,8 @@ class CORE_EXPORT QgsTransaction : public QObject SIP_ABSTRACT

void setLayerTransactionIds( QgsTransaction *transaction );

static QString removeLayerIdOrName( const QString &str );

virtual bool beginTransaction( QString &error, int statementTimeout ) = 0;
virtual bool commitTransaction( QString &error ) = 0;
virtual bool rollbackTransaction( QString &error ) = 0;
@@ -33,8 +33,7 @@ bool QgsTransactionGroup::addLayer( QgsVectorLayer *layer )
if ( !QgsTransaction::supportsTransaction( layer ) )
return false;

QString connString = QgsDataSourceUri( layer->source() ).connectionInfo();

QString connString = QgsTransaction::connectionString( layer->source() );
if ( mConnString.isEmpty() )
{
mConnString = connString;
@@ -74,6 +73,8 @@ void QgsTransactionGroup::onEditingStarted()
return;

mTransaction.reset( QgsTransaction::create( mConnString, mProviderKey ) );
if ( !mTransaction )
return;

QString errorMsg;
mTransaction->begin( errorMsg );
@@ -9,6 +9,7 @@ SET (OGR_SRCS
qgsgeopackagerasterwritertask.cpp
qgsogrdbconnection.cpp
qgsogrdbtablemodel.cpp
qgsogrtransaction.cpp
)

SET(OGR_MOC_HDRS
@@ -19,6 +20,7 @@ SET(OGR_MOC_HDRS
qgsgeopackagerasterwritertask.h
qgsogrdbconnection.h
qgsogrdbtablemodel.h
qgsogrtransaction.h
)

IF (WITH_GUI)
@@ -26,6 +26,7 @@
#include "qgssettings.h"
#include "qgsexception.h"
#include "qgswkbtypes.h"
#include "qgsogrtransaction.h"

#include <QTextCodec>
#include <QFile>
@@ -42,38 +43,51 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
: QgsAbstractFeatureIteratorFromSource<QgsOgrFeatureSource>( source, ownSource, request )
, mFilterFids( mRequest.filterFids() )
, mFilterFidsIt( mFilterFids.constBegin() )
, mSharedDS( source->mSharedDS )
{
//QgsDebugMsg( "Feature iterator of " + mSource->mLayerName + ": acquiring connection");
mConn = QgsOgrConnPool::instance()->acquireConnection( QgsOgrProviderUtils::connectionPoolId( mSource->mDataSource ) );
if ( !mConn->ds )
if ( mSharedDS )
{
return;
}

if ( mSource->mLayerName.isNull() )
{
mOgrLayer = GDALDatasetGetLayer( mConn->ds, mSource->mLayerIndex );
mOgrLayer = mSharedDS->createSQLResultLayer( mSource->mEncoding, mSource->mLayerName, mSource->mLayerIndex );
if ( !mOgrLayer )
{
return;
}
}
else
{
mOgrLayer = GDALDatasetGetLayerByName( mConn->ds, mSource->mLayerName.toUtf8().constData() );
}
if ( !mOgrLayer )
{
return;
}
//QgsDebugMsg( "Feature iterator of " + mSource->mLayerName + ": acquiring connection");
mConn = QgsOgrConnPool::instance()->acquireConnection( QgsOgrProviderUtils::connectionPoolId( mSource->mDataSource ) );
if ( !mConn->ds )
{
return;
}

if ( !mSource->mSubsetString.isEmpty() )
{
mOgrOrigLayer = mOgrLayer;
mOgrLayerWithFid = QgsOgrProviderUtils::setSubsetString( mOgrLayer, mConn->ds, mSource->mEncoding, QString(), true, &mOrigFidAdded );
mOgrLayer = QgsOgrProviderUtils::setSubsetString( mOgrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString, true, &mOrigFidAdded );
if ( mSource->mLayerName.isNull() )
{
mOgrLayer = GDALDatasetGetLayer( mConn->ds, mSource->mLayerIndex );
}
else
{
mOgrLayer = GDALDatasetGetLayerByName( mConn->ds, mSource->mLayerName.toUtf8().constData() );
}
if ( !mOgrLayer )
{
close();
return;
}

if ( !mSource->mSubsetString.isEmpty() )
{
mOgrOrigLayer = mOgrLayer;
mOgrLayerWithFid = QgsOgrProviderUtils::setSubsetString( mOgrLayer, mConn->ds, mSource->mEncoding, QString(), true, &mOrigFidAdded );
mOgrLayer = QgsOgrProviderUtils::setSubsetString( mOgrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString, true, &mOrigFidAdded );
if ( !mOgrLayer )
{
close();
return;
}
}
}
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );

if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mSource->mCrs )
{
@@ -237,6 +251,8 @@ bool QgsOgrFeatureIterator::fetchFeatureWithId( QgsFeatureId id, QgsFeature &fea

bool QgsOgrFeatureIterator::fetchFeature( QgsFeature &feature )
{
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );

feature.setValid( false );

if ( mClosed || !mOgrLayer )
@@ -286,6 +302,8 @@ bool QgsOgrFeatureIterator::fetchFeature( QgsFeature &feature )

bool QgsOgrFeatureIterator::rewind()
{
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );

if ( mClosed || !mOgrLayer )
return false;

@@ -299,6 +317,20 @@ bool QgsOgrFeatureIterator::rewind()

bool QgsOgrFeatureIterator::close()
{
if ( mSharedDS )
{
iteratorClosed();

if ( mOgrLayer )
{
mSharedDS->releaseResultSet( mOgrLayer );
mOgrLayer = nullptr;
}
mSharedDS.reset();
mClosed = true;
return true;
}

if ( !mConn )
return false;

@@ -444,7 +476,12 @@ QgsOgrFeatureSource::QgsOgrFeatureSource( const QgsOgrProvider *p )
, mDriverName( p->mGDALDriverName )
, mCrs( p->crs() )
, mWkbType( p->wkbType() )
, mSharedDS( nullptr )
{
if ( p->mTransaction )
{
mSharedDS = p->mTransaction->sharedDS();
}
for ( int i = ( p->mFirstFieldIsFid ) ? 1 : 0; i < mFields.size(); i++ )
mFieldsWithoutFid.append( mFields.at( i ) );
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( mDataSource ) );
@@ -21,8 +21,12 @@

#include <ogr_api.h>

#include <memory>

class QgsOgrFeatureIterator;
class QgsOgrProvider;
class QgsOgrDataset;
using QgsOgrDatasetSharedPtr = std::shared_ptr< QgsOgrDataset>;

class QgsOgrFeatureSource : public QgsAbstractFeatureSource
{
@@ -45,6 +49,7 @@ class QgsOgrFeatureSource : public QgsAbstractFeatureSource
QString mDriverName;
QgsCoordinateReferenceSystem mCrs;
QgsWkbTypes::Type mWkbType = QgsWkbTypes::Unknown;
QgsOgrDatasetSharedPtr mSharedDS = nullptr;

friend class QgsOgrFeatureIterator;
friend class QgsOgrExpressionCompiler;
@@ -87,6 +92,7 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr

QgsRectangle mFilterRect;
QgsCoordinateTransform mTransform;
QgsOgrDatasetSharedPtr mSharedDS = nullptr;

bool fetchFeatureWithId( QgsFeatureId id, QgsFeature &feature ) const;
};

0 comments on commit 6a987f9

Please sign in to comment.
You can’t perform that action at this time.