Skip to content

Commit

Permalink
[Offline editing] Unbreak TestQgsOfflineEditing with GDAL >= 3.4.2
Browse files Browse the repository at this point in the history
Fixes qgis#48012

Basically, for offline editing, we ask QgsOgrProvider to adopt the
behaviour it had before qgis#47098, that is
use SQLite3 journal_mode = WAL even in read-only mode.

Sorry, this "fix" is horrible, but I couldn't get with something better after
many hours of investigation. The root cause seems to be that QgsOfflineEditing
creates layers using direct OGR API or sqlite3 API, and also uses
QgsOgrProvider which has a GDAL dataset reuse mechanism. And all that does
not place nicely at all. Maybe the root root cause is that the OGR GPKG driver
establishes the list of layers at its opening, and if creates a new one behind its
back, which is prone to happen with the dataset caching metchanism, the
GDALDataset::GetLayerByName() method doesn't see the newly created layer.

With that in mind, I tried to add calls to
QgsOgrProviderUtils::invalidateCachedDatasets() but to no avail.
Also QgsOgrProviderUtils::invalidateCachedLastModifiedDate() which is
used by QgsOgrProvider::createEmptyLayer(), but to no avail.

So basically I don't understand why offline editing worked before with a
GPKG backend. I'd say it is pure luck.

Note: I also saw that qgsofflineediting.cpp at line 315 calls
offlineLayer->dataProvider()->invalidateConnections() but that method is
only implemented in the Spatialite provider. I tried to implement it in
the OGR one, but to no avail too.
  • Loading branch information
rouault committed Apr 2, 2022
1 parent 9924607 commit 6697c03
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 3 deletions.
13 changes: 11 additions & 2 deletions src/core/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -3434,6 +3434,10 @@ void QgsOgrProvider::open( OpenMode mode )

const bool bIsGpkg = QFileInfo( mFilePath ).suffix().compare( QLatin1String( "gpkg" ), Qt::CaseInsensitive ) == 0;

#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3,4,2)
bool forceWAL = false;
#endif

// first try to open in update mode (unless specified otherwise)
QString errCause;
if ( !openReadOnly )
Expand All @@ -3445,7 +3449,12 @@ void QgsOgrProvider::open( OpenMode mode )
}

#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3,4,2)
if ( bIsGpkg && mode == OpenModeInitial )
if ( bIsGpkg && options.contains( "QGIS_FORCE_WAL=ON" ) )
{
// Hack use for qgsofflineediting / https://github.com/qgis/QGIS/issues/48012
forceWAL = true;
}
if ( bIsGpkg && mode == OpenModeInitial && !forceWAL )
{
// A hint to QgsOgrProviderUtils::GDALOpenWrapper() to not force WAL
// as in OpenModeInitial we are not going to do anything besides getting capabilities
Expand Down Expand Up @@ -3574,7 +3583,7 @@ void QgsOgrProvider::open( OpenMode mode )
( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) ||
mGDALDriverName == QLatin1String( "MapInfo File" )
#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3,4,2)
|| mGDALDriverName == QLatin1String( "GPKG" )
|| ( !forceWAL && mGDALDriverName == QLatin1String( "GPKG" ) )
#endif
) )
{
Expand Down
6 changes: 6 additions & 0 deletions src/core/providers/ogr/qgsogrproviderutils.cpp
Expand Up @@ -1000,6 +1000,12 @@ GDALDatasetH QgsOgrProviderUtils::GDALOpenWrapper( const char *pszPath, bool bUp
bool bIsGpkg = QFileInfo( filePath ).suffix().compare( QLatin1String( "gpkg" ), Qt::CaseInsensitive ) == 0;
bool bIsLocalGpkg = false;

if ( bIsGpkg )
{
// Hack use for qgsofflineediting / https://github.com/qgis/QGIS/issues/48012
papszOpenOptions = CSLSetNameValue( papszOpenOptions, "QGIS_FORCE_WAL", nullptr );
}

#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3,4,2)
if ( bIsGpkg && !bUpdate )
{
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsofflineediting.cpp
Expand Up @@ -779,7 +779,7 @@ void QgsOfflineEditing::convertToOfflineLayer( QgsVectorLayer *layer, sqlite3 *d
}
hDS.reset();

const QString uri = QStringLiteral( "%1|layername=%2" ).arg( offlineDbPath, tableName );
const QString uri = QStringLiteral( "%1|layername=%2|option:QGIS_FORCE_WAL=ON" ).arg( offlineDbPath, tableName );
const QgsVectorLayer::LayerOptions layerOptions { QgsProject::instance()->transformContext() };
newLayer = std::make_unique<QgsVectorLayer>( uri, layer->name() + layerNameSuffix, QStringLiteral( "ogr" ), layerOptions );
break;
Expand Down

0 comments on commit 6697c03

Please sign in to comment.