Skip to content

Commit baea1e8

Browse files
elpasoelpaso
authoredMar 9, 2023
Merge pull request #52150 from elpaso/bugfix-gh51934-filtered-gpkg-transaction-group
OGR: fix transaction issue with filtered GPKGs
·
ltr-3_40final-3_32_0
2 parents 96f5d6a + 8a5f2c5 commit baea1e8

File tree

4 files changed

+74
-7
lines changed

4 files changed

+74
-7
lines changed
 

‎src/core/providers/ogr/qgsogrfeatureiterator.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
287287
// when the logger was disabled.
288288
// There is currently no API to connect the change of state of the
289289
// logger to the data provider.
290-
if ( QgsApplication::databaseQueryLog()->enabled() )
290+
if ( QgsApplication::databaseQueryLog()->enabled() && mConn )
291291
{
292292
GDALDatasetSetQueryLoggerFunc( mConn->ds, [ ]( const char *pszSQL, const char *pszError, int64_t lNumRecords, int64_t lExecutionTimeMilliseconds, void *pQueryLoggerArg )
293293
{
@@ -517,6 +517,13 @@ bool QgsOgrFeatureIterator::rewind()
517517

518518
bool QgsOgrFeatureIterator::close()
519519
{
520+
// Finally reset the data source filter, in case it was changed by a previous request
521+
// this fixes https://github.com/qgis/QGIS/issues/51934
522+
if ( mOgrLayer && ! mSource->mSubsetString.isEmpty() )
523+
{
524+
OGR_L_SetAttributeFilter( mOgrLayer, mSource->mEncoding->fromUnicode( mSource->mSubsetString ).constData() );
525+
}
526+
520527
if ( mSharedDS )
521528
{
522529
iteratorClosed();

‎src/core/qgstransaction.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,21 @@ QString QgsTransaction::connectionString() const
7272
}
7373

7474
// For the needs of the OGR provider with GeoPackage datasources, remove
75-
// any reference to layers in the connection string
76-
QString QgsTransaction::removeLayerIdOrName( const QString &str )
75+
// any reference to layers and filters in the connection string
76+
QString QgsTransaction::cleanupConnectionString( const QString &str )
7777
{
7878
QString res( str );
7979

80-
for ( int i = 0; i < 2; i++ )
80+
static const QStringList toRemove
8181
{
82-
const int pos = res.indexOf( i == 0 ? QLatin1String( "|layername=" ) : QLatin1String( "|layerid=" ) );
82+
{ QStringLiteral( "|layername=" )},
83+
{ QStringLiteral( "|layerid=" )},
84+
{ QStringLiteral( "|subset=" )},
85+
};
86+
87+
for ( const auto &strToRm : std::as_const( toRemove ) )
88+
{
89+
const int pos = res.indexOf( strToRm );
8390
if ( pos >= 0 )
8491
{
8592
const int end = res.indexOf( '|', pos + 1 );
@@ -105,7 +112,7 @@ QString QgsTransaction::connectionString( const QString &layerUri )
105112
// reference to layers from it.
106113
if ( connString.isEmpty() )
107114
{
108-
connString = removeLayerIdOrName( layerUri );
115+
connString = cleanupConnectionString( layerUri );
109116
}
110117
return connString;
111118
}

‎src/core/qgstransaction.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ class CORE_EXPORT QgsTransaction : public QObject SIP_ABSTRACT
201201

202202
void setLayerTransactionIds( QgsTransaction *transaction );
203203

204-
static QString removeLayerIdOrName( const QString &str );
204+
static QString cleanupConnectionString( const QString &str );
205205

206206
virtual bool beginTransaction( QString &error, int statementTimeout ) = 0;
207207
virtual bool commitTransaction( QString &error ) = 0;

‎tests/src/python/test_provider_ogr_gpkg.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2638,6 +2638,59 @@ def testDateTimeTimeZoneMilliseconds(self):
26382638
got = [feat for feat in vl.getFeatures()]
26392639
self.assertEqual(got[0]["dt"], new_dt)
26402640

2641+
def testTransactionModeAutoWithFilter(self):
2642+
2643+
temp_dir = QTemporaryDir()
2644+
temp_path = temp_dir.path()
2645+
filename = os.path.join(temp_path, "test.gpkg")
2646+
ds = ogr.GetDriverByName("GPKG").CreateDataSource(filename)
2647+
lyr = ds.CreateLayer("points", geom_type=ogr.wkbPoint)
2648+
lyr.CreateField(ogr.FieldDefn('name', ogr.OFTString))
2649+
f = ogr.Feature(lyr.GetLayerDefn())
2650+
f['name'] = 'a'
2651+
f.SetGeometry(ogr.CreateGeometryFromWkt('POINT(1 2)'))
2652+
lyr.CreateFeature(f)
2653+
f = None
2654+
ds = None
2655+
ds = None
2656+
2657+
vl = QgsVectorLayer(filename)
2658+
self.assertTrue(vl.isValid())
2659+
self.assertEqual(vl.featureCount(), 1)
2660+
self.assertEqual(next(vl.getFeatures())['name'], 'a')
2661+
2662+
p = QgsProject()
2663+
p.setAutoTransaction(True)
2664+
self.assertTrue(p.addMapLayers([vl]))
2665+
self.assertTrue(vl.setSubsetString('"name" IN (\'a\', \'b\')'))
2666+
self.assertEqual(vl.featureCount(), 1)
2667+
self.assertTrue(vl.startEditing())
2668+
2669+
# Add feature
2670+
f = QgsFeature(vl.fields())
2671+
f.setAttribute('name', 'b')
2672+
g = QgsGeometry.fromWkt('POINT(3 4)')
2673+
f.setGeometry(g)
2674+
2675+
# This triggers the issue because it sets the subset filter
2676+
req = QgsFeatureRequest()
2677+
req.setFilterExpression('"name" IS NULL')
2678+
self.assertEqual([f for f in vl.getFeatures(req)], [])
2679+
2680+
self.assertTrue(vl.addFeatures([f]))
2681+
self.assertTrue(vl.commitChanges())
2682+
self.assertEqual(vl.featureCount(), 2)
2683+
attrs = [f['name'] for f in vl.getFeatures()]
2684+
self.assertEqual(attrs, ['a', 'b'])
2685+
2686+
# verify
2687+
del p
2688+
vl2 = QgsVectorLayer(filename)
2689+
self.assertTrue(vl2.isValid())
2690+
self.assertEqual(vl2.featureCount(), 2)
2691+
attrs = [f['name'] for f in vl2.getFeatures()]
2692+
self.assertEqual(attrs, ['a', 'b'])
2693+
26412694

26422695
if __name__ == '__main__':
26432696
unittest.main()

0 commit comments

Comments
 (0)
Please sign in to comment.