Skip to content

Commit bdc1119

Browse files
rouaultrouault
authored
Merge pull request #7982 from rouault/fix_19477
[OGR provider] Fix reading of OSM datasets when opening several layers at the same time (fixes #19477)
2 parents 77be7ea + fabdc04 commit bdc1119

File tree

6 files changed

+243
-80
lines changed

6 files changed

+243
-80
lines changed

‎src/providers/ogr/qgsogrfeatureiterator.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
5858
else
5959
{
6060
//QgsDebugMsg( "Feature iterator of " + mSource->mLayerName + ": acquiring connection");
61-
mConn = QgsOgrConnPool::instance()->acquireConnection( QgsOgrProviderUtils::connectionPoolId( mSource->mDataSource ), mRequest.timeout(), mRequest.requestMayBeNested() );
61+
mConn = QgsOgrConnPool::instance()->acquireConnection( QgsOgrProviderUtils::connectionPoolId( mSource->mDataSource, mSource->mShareSameDatasetAmongLayers ), mRequest.timeout(), mRequest.requestMayBeNested() );
6262
if ( !mConn || !mConn->ds )
6363
{
6464
return;
@@ -468,6 +468,7 @@ bool QgsOgrFeatureIterator::readFeature( gdal::ogr_feature_unique_ptr fet, QgsFe
468468

469469
QgsOgrFeatureSource::QgsOgrFeatureSource( const QgsOgrProvider *p )
470470
: mDataSource( p->dataSourceUri( true ) )
471+
, mShareSameDatasetAmongLayers( p->mShareSameDatasetAmongLayers )
471472
, mLayerName( p->layerName() )
472473
, mLayerIndex( p->layerIndex() )
473474
, mSubsetString( p->mSubsetString )
@@ -486,12 +487,12 @@ QgsOgrFeatureSource::QgsOgrFeatureSource( const QgsOgrProvider *p )
486487
}
487488
for ( int i = ( p->mFirstFieldIsFid ) ? 1 : 0; i < mFields.size(); i++ )
488489
mFieldsWithoutFid.append( mFields.at( i ) );
489-
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( mDataSource ) );
490+
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( mDataSource, mShareSameDatasetAmongLayers ) );
490491
}
491492

492493
QgsOgrFeatureSource::~QgsOgrFeatureSource()
493494
{
494-
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( mDataSource ) );
495+
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( mDataSource, mShareSameDatasetAmongLayers ) );
495496
}
496497

497498
QgsFeatureIterator QgsOgrFeatureSource::getFeatures( const QgsFeatureRequest &request )

‎src/providers/ogr/qgsogrfeatureiterator.h

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class QgsOgrFeatureSource : public QgsAbstractFeatureSource
3838

3939
private:
4040
QString mDataSource;
41+
bool mShareSameDatasetAmongLayers;
4142
QString mLayerName;
4243
int mLayerIndex;
4344
QString mSubsetString;

‎src/providers/ogr/qgsogrprovider.cpp

+97-75
Original file line numberDiff line numberDiff line change
@@ -538,15 +538,15 @@ QgsOgrProvider::QgsOgrProvider( QString const &uri, const ProviderOptions &optio
538538

539539
setNativeTypes( nativeTypes );
540540

541-
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
541+
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
542542
}
543543

544544
QgsOgrProvider::~QgsOgrProvider()
545545
{
546-
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
546+
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
547547
// We must also make sure to flush unusef cached connections so that
548548
// the file can be removed (#15137)
549-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
549+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
550550

551551
// Do that as last step for final cleanup that might be prevented by
552552
// still opened datasets.
@@ -948,7 +948,7 @@ OGRwkbGeometryType QgsOgrProvider::getOgrGeomType( OGRLayerH ogrLayer )
948948

949949
void QgsOgrProvider::loadFields()
950950
{
951-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
951+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
952952
//the attribute fields need to be read again when the encoding changes
953953
mAttributeFields.clear();
954954
mDefaultValues.clear();
@@ -1642,7 +1642,7 @@ bool QgsOgrProvider::addAttributes( const QList<QgsField> &attributes )
16421642
{
16431643
// adding attributes in mapinfo requires to be able to delete the .dat file
16441644
// so drop any cached connections.
1645-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
1645+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
16461646
}
16471647

16481648
bool returnvalue = true;
@@ -1885,10 +1885,10 @@ bool QgsOgrProvider::_setSubsetString( const QString &theSQL, bool updateFeature
18851885
if ( uri != dataSourceUri() )
18861886
{
18871887
if ( hasExistingRef )
1888-
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
1888+
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
18891889
setDataSourceUri( uri );
18901890
if ( hasExistingRef )
1891-
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
1891+
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
18921892
}
18931893

18941894
mOgrLayer->ResetReading();
@@ -2061,7 +2061,7 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
20612061
{
20622062
pushError( tr( "OGR error syncing to disk: %1" ).arg( CPLGetLastErrorMsg() ) );
20632063
}
2064-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
2064+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
20652065
return true;
20662066
}
20672067

@@ -2140,7 +2140,7 @@ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
21402140
if ( mTransaction )
21412141
mTransaction->dirtyLastSavePoint();
21422142

2143-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
2143+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
21442144
return syncToDisc();
21452145
}
21462146

@@ -3595,21 +3595,24 @@ QByteArray QgsOgrProvider::quotedIdentifier( const QByteArray &field ) const
35953595

35963596
void QgsOgrProvider::forceReload()
35973597
{
3598-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
3598+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
35993599
}
36003600

3601-
QString QgsOgrProviderUtils::connectionPoolId( const QString &dataSourceURI )
3601+
QString QgsOgrProviderUtils::connectionPoolId( const QString &dataSourceURI, bool shareSameDatasetAmongLayers )
36023602
{
3603-
// If the file part of the URI is really a file, then use it as the
3604-
// connection pool id (for example, so that all layers of a .gpkg file can
3605-
// use the same GDAL dataset object)
3606-
// Otherwise use the datasourceURI
3607-
// Not completely sure about this logic. But at least, for GeoPackage this
3608-
// works fine with multi layer datasets.
3609-
QString filePath = dataSourceURI.left( dataSourceURI.indexOf( QLatin1String( "|" ) ) );
3610-
QFileInfo fi( filePath );
3611-
if ( fi.isFile() )
3612-
return filePath;
3603+
if ( shareSameDatasetAmongLayers )
3604+
{
3605+
// If the file part of the URI is really a file, then use it as the
3606+
// connection pool id (for example, so that all layers of a .gpkg file can
3607+
// use the same GDAL dataset object)
3608+
// Otherwise use the datasourceURI
3609+
// Not completely sure about this logic. But at least, for GeoPackage this
3610+
// works fine with multi layer datasets.
3611+
QString filePath = dataSourceURI.left( dataSourceURI.indexOf( QLatin1String( "|" ) ) );
3612+
QFileInfo fi( filePath );
3613+
if ( fi.isFile() )
3614+
return filePath;
3615+
}
36133616
return dataSourceURI;
36143617
}
36153618

@@ -3877,7 +3880,7 @@ QString QgsOgrProviderUtils::quotedValue( const QVariant &value )
38773880
bool QgsOgrProvider::syncToDisc()
38783881
{
38793882
//for shapefiles, remove spatial index files and create a new index
3880-
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
3883+
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
38813884
bool shapeIndex = false;
38823885
if ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) )
38833886
{
@@ -3892,7 +3895,7 @@ bool QgsOgrProvider::syncToDisc()
38923895
{
38933896
shapeIndex = true;
38943897
close();
3895-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
3898+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
38963899
QFile::remove( sbnIndexFile );
38973900
open( OpenModeSameAsCurrent );
38983901
if ( !mValid )
@@ -3916,7 +3919,7 @@ bool QgsOgrProvider::syncToDisc()
39163919
}
39173920
#endif
39183921

3919-
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
3922+
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
39203923
if ( shapeIndex )
39213924
{
39223925
return createSpatialIndex();
@@ -3978,7 +3981,7 @@ void QgsOgrProvider::recalculateFeatureCount()
39783981
mOgrLayer->SetSpatialFilter( filter );
39793982
}
39803983

3981-
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
3984+
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
39823985
}
39833986

39843987
bool QgsOgrProvider::doesStrictFeatureTypeCheck() const
@@ -4010,13 +4013,13 @@ OGRwkbGeometryType QgsOgrProvider::ogrWkbSingleFlatten( OGRwkbGeometryType type
40104013
OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds, QTextCodec *encoding, const QString &subsetString, bool addOriginalFid, bool *origFidAdded )
40114014
{
40124015
QByteArray layerName = OGR_FD_GetName( OGR_L_GetLayerDefn( layer ) );
4013-
GDALDriverH mGDALDriver = GDALGetDatasetDriver( ds );
4014-
QString mGDALDriverName = GDALGetDriverShortName( mGDALDriver );
4016+
GDALDriverH driver = GDALGetDatasetDriver( ds );
4017+
QString driverName = GDALGetDriverShortName( driver );
40154018
bool origFidAddAttempted = false;
40164019
if ( origFidAdded )
40174020
*origFidAdded = false;
40184021

4019-
if ( mGDALDriverName == QLatin1String( "ODBC" ) ) //the odbc driver does not like schema names for subset
4022+
if ( driverName == QLatin1String( "ODBC" ) ) //the odbc driver does not like schema names for subset
40204023
{
40214024
QString layerNameString = encoding->toUnicode( layerName );
40224025
int dotIndex = layerNameString.indexOf( '.' );
@@ -4037,7 +4040,7 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds
40374040
else
40384041
{
40394042
QByteArray sqlPart1 = "SELECT *";
4040-
QByteArray sqlPart3 = " FROM " + quotedIdentifier( layerName, mGDALDriverName );
4043+
QByteArray sqlPart3 = " FROM " + quotedIdentifier( layerName, driverName );
40414044
if ( !subsetString.isEmpty() )
40424045
sqlPart3 += " WHERE " + encoding->fromUnicode( subsetString );
40434046

@@ -4179,6 +4182,7 @@ void QgsOgrProvider::open( OpenMode mode )
41794182
if ( mOgrOrigLayer )
41804183
{
41814184
mGDALDriverName = mOgrOrigLayer->driverName();
4185+
mShareSameDatasetAmongLayers = QgsOgrProviderUtils::canDriverShareSameDatasetAmongLayers( mGDALDriverName );
41824186

41834187
QgsDebugMsg( "OGR opened using Driver " + mGDALDriverName );
41844188

@@ -4493,6 +4497,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
44934497
auto &datasetList = iter.value();
44944498
Q_FOREACH ( QgsOgrProviderUtils::DatasetWithLayers *ds, datasetList )
44954499
{
4500+
if ( !ds->canBeShared )
4501+
continue;
44964502
Q_ASSERT( ds->refCount > 0 );
44974503

44984504
QString layerName;
@@ -4546,6 +4552,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
45464552
auto datasetList = iter.value();
45474553
Q_FOREACH ( QgsOgrProviderUtils::DatasetWithLayers *ds, datasetList )
45484554
{
4555+
if ( !ds->canBeShared )
4556+
continue;
45494557
Q_ASSERT( ds->refCount > 0 );
45504558

45514559
QString layerName;
@@ -4590,6 +4598,10 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
45904598
new QgsOgrProviderUtils::DatasetWithLayers;
45914599
ds->hDS = hDS;
45924600

4601+
GDALDriverH driver = GDALGetDatasetDriver( hDS );
4602+
QString driverName = GDALGetDriverShortName( driver );
4603+
ds->canBeShared = canDriverShareSameDatasetAmongLayers( driverName );
4604+
45934605
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
45944606
ident, layerName, ds, hLayer );
45954607
ds->setLayers[layerName] = layer.get();
@@ -4616,6 +4628,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
46164628
auto &datasetList = iter.value();
46174629
Q_FOREACH ( QgsOgrProviderUtils::DatasetWithLayers *ds, datasetList )
46184630
{
4631+
if ( !ds->canBeShared )
4632+
continue;
46194633
Q_ASSERT( ds->refCount > 0 );
46204634

46214635
auto iter2 = ds->setLayers.find( layerName );
@@ -4979,6 +4993,45 @@ bool QgsOgrProviderUtils::canUseOpenedDatasets( const QString &dsName )
49794993
return getLastModified( dsName ) <= iter.value();
49804994
}
49814995

4996+
QgsOgrProviderUtils::DatasetWithLayers *QgsOgrProviderUtils::createDatasetWithLayers(
4997+
const QString &dsName,
4998+
bool updateMode,
4999+
const QStringList &options,
5000+
const QString &layerName,
5001+
const DatasetIdentification &ident,
5002+
QgsOgrLayerUniquePtr &layer,
5003+
QString &errCause )
5004+
{
5005+
GDALDatasetH hDS = OpenHelper( dsName, updateMode, options );
5006+
if ( !hDS )
5007+
{
5008+
errCause = QObject::tr( "Cannot open %1." ).arg( dsName );
5009+
return nullptr;
5010+
}
5011+
sMapDSNameToLastModifiedDate[dsName] = getLastModified( dsName );
5012+
5013+
OGRLayerH hLayer = GDALDatasetGetLayerByName(
5014+
hDS, layerName.toUtf8().constData() );
5015+
if ( !hLayer )
5016+
{
5017+
errCause = QObject::tr( "Cannot find layer %1." ).arg( layerName );
5018+
QgsOgrProviderUtils::GDALCloseWrapper( hDS );
5019+
return nullptr;
5020+
}
5021+
5022+
QgsOgrProviderUtils::DatasetWithLayers *ds =
5023+
new QgsOgrProviderUtils::DatasetWithLayers;
5024+
ds->hDS = hDS;
5025+
5026+
GDALDriverH driver = GDALGetDatasetDriver( hDS );
5027+
QString driverName = GDALGetDriverShortName( driver );
5028+
ds->canBeShared = canDriverShareSameDatasetAmongLayers( driverName );
5029+
5030+
layer = QgsOgrLayer::CreateForLayer(
5031+
ident, layerName, ds, hLayer );
5032+
ds->setLayers[layerName] = layer.get();
5033+
return ds;
5034+
}
49825035

49835036
QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
49845037
bool updateMode,
@@ -5018,6 +5071,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
50185071
auto &datasetList = iter.value();
50195072
Q_FOREACH ( QgsOgrProviderUtils::DatasetWithLayers *ds, datasetList )
50205073
{
5074+
if ( !ds->canBeShared )
5075+
continue;
50215076
Q_ASSERT( ds->refCount > 0 );
50225077

50235078
auto iter2 = ds->setLayers.find( layerName );
@@ -5045,60 +5100,22 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
50455100

50465101
// All existing DatasetWithLayers* already reference our layer of
50475102
// interest, so instantiate a new DatasetWithLayers*
5048-
GDALDatasetH hDS = OpenHelper( dsName, updateMode, options );
5049-
if ( !hDS )
5050-
{
5051-
errCause = QObject::tr( "Cannot open %1." ).arg( dsName );
5052-
return nullptr;
5053-
}
5054-
sMapDSNameToLastModifiedDate[dsName] = getLastModified( dsName );
5055-
5056-
OGRLayerH hLayer = GDALDatasetGetLayerByName(
5057-
hDS, layerName.toUtf8().constData() );
5058-
if ( !hLayer )
5059-
{
5060-
QgsOgrProviderUtils::GDALCloseWrapper( hDS );
5061-
errCause = QObject::tr( "Cannot find layer %1." ).arg( layerName );
5062-
return nullptr;
5063-
}
5064-
5103+
QgsOgrLayerUniquePtr layer;
50655104
QgsOgrProviderUtils::DatasetWithLayers *ds =
5066-
new QgsOgrProviderUtils::DatasetWithLayers;
5105+
createDatasetWithLayers( dsName, updateMode, options, layerName, ident, layer, errCause );
5106+
if ( !ds )
5107+
return nullptr;
50675108

50685109
datasetList.push_back( ds );
50695110

5070-
ds->hDS = hDS;
5071-
5072-
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
5073-
ident, layerName, ds, hLayer );
5074-
ds->setLayers[layerName] = layer.get();
50755111
return layer;
50765112
}
50775113

5078-
GDALDatasetH hDS = OpenHelper( dsName, updateMode, options );
5079-
if ( !hDS )
5080-
{
5081-
errCause = QObject::tr( "Cannot open %1." ).arg( dsName );
5082-
return nullptr;
5083-
}
5084-
sMapDSNameToLastModifiedDate[dsName] = getLastModified( dsName );
5085-
5086-
OGRLayerH hLayer = GDALDatasetGetLayerByName(
5087-
hDS, layerName.toUtf8().constData() );
5088-
if ( !hLayer )
5089-
{
5090-
errCause = QObject::tr( "Cannot find layer %1." ).arg( layerName );
5091-
QgsOgrProviderUtils::GDALCloseWrapper( hDS );
5092-
return nullptr;
5093-
}
5094-
5114+
QgsOgrLayerUniquePtr layer;
50955115
QgsOgrProviderUtils::DatasetWithLayers *ds =
5096-
new QgsOgrProviderUtils::DatasetWithLayers;
5097-
ds->hDS = hDS;
5098-
5099-
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
5100-
ident, layerName, ds, hLayer );
5101-
ds->setLayers[layerName] = layer.get();
5116+
createDatasetWithLayers( dsName, updateMode, options, layerName, ident, layer, errCause );
5117+
if ( !ds )
5118+
return nullptr;
51025119

51035120
QList<DatasetWithLayers *> datasetList;
51045121
datasetList.push_back( ds );
@@ -5192,6 +5209,11 @@ void QgsOgrProviderUtils::releaseDataset( QgsOgrDataset *&ds )
51925209
ds = nullptr;
51935210
}
51945211

5212+
bool QgsOgrProviderUtils::canDriverShareSameDatasetAmongLayers( const QString &driverName )
5213+
{
5214+
return driverName != QStringLiteral( "OSM" );
5215+
}
5216+
51955217

51965218
QgsOgrDatasetSharedPtr QgsOgrDataset::create( const QgsOgrProviderUtils::DatasetIdentification &ident,
51975219
QgsOgrProviderUtils::DatasetWithLayers *ds )

0 commit comments

Comments
 (0)