Changes on geometry get discarded on save edits when layer filter is enabled (gpkg layer)

[nina1706]

What is the bug or the crash?

I have added the filter on the polygon layer and then used the split features tool and couldn't save the changes. When removing the filter and again using the split features tool, all the changes made are saved. I'm attaching the short video where it's easier to see the problem: example 1: splitting feature on filtered layer and example 2: deleting feature on filtered layer and repeatedly editing layer-penultimate change is saved?!

Steps to reproduce the issue

Create a polygon layer and add couple of polygons. Then add some filter on that layer and use the split features tool. After splitting the polygons click on the save edits button.

You can use my example data (from video demo):
buildings.zip
with filter: "desc" IN ('a','c')

Versions

QGIS version
3.28.2-Firenze
QGIS code revision
b47e00b
Qt version
5.15.3
Python version
3.9.5
Compiled against GDAL/OGR
3.6.1
Running against GDAL/OGR
3.6.2
PROJ version
9.1.1
EPSG Registry database version
v10.076 (2022-08-31)
GEOS version
3.11.1-CAPI-1.17.1
SQLite version
3.39.4
PDAL version
2.4.3
PostgreSQL client version
unknown
SpatiaLite version
5.0.1
QWT version
6.1.6
QScintilla2 version
2.13.1
OS version
Windows 10 Version 2009

Active Python plugins
db_manager
0.1.20
grassprovider
2.12.99
MetaSearch
0.3.6
processing
2.12.99
sagaprovider
2.12.99

Supported QGIS version

• I'm running a supported QGIS version according to the roadmap.

New profile

• I tried with a new QGIS profile

Additional context

No response

[elpaso]

Videos are private, cannot see them.

[nina1706]

@elpaso sorry, now it's fixed

[agiudiceandrea]

@nina1706 it seems to me the issue doesn't occur on my Windows system using QGIS 3.28.3:

split.mp4

[elpaso]

Cannot reproduce on Linux master.

[nina1706]

@agiudiceandrea @elpaso thank you for your replies, I will do some more testing with new profile and on different machine. I will reply as soon as possible.

[nina1706]

Hi @elpaso and @agiudiceandrea, thank you for waiting! We did thorough testing and didn't find the cause of the problem which is still present on one of our computers. I am now closing the bug report. If and when we find out the cause we will let you know.

[nina1706]

Hi @elpaso and @agiudiceandrea,
We have found the right steps to reproduce the problem.
Transaction mode in project properties (Data sources) needs to be set on Automatic Transaction Groups and also project needs to be saved without having filter on. When opening a project and putting a filter on the layer it's not possible to save changes when using the split features tool.
Here's the project example which you can use for testing:

Buildings.zip

[agiudiceandrea]

Thanks @nina1706. I can confirm the described behaviour using QGIS 3.28.3 on Windows.
It doesn't affect only the Split Features tool, but also the other editing tools like Move Features or Rotate Features or the Vertex tool...

[elpaso]

@agiudiceandrea so, it's windows specific?

[agiudiceandrea]

@elpaso have you tested the latest provided project: setting a layer filter, editing the layer, closing the edit mode -> the edits are not saved?

[elpaso]

No, I'll test it now.

[elpaso]

I've done some debugging, apparently the bug can be reproduced by just adding a new feature (no split necessary).

If a second new feature is added the transaction is closed and the first one appears as well.

My feeling is that it is caused by the filter not being removed from the connection string when searching for open datasets, this fixes it:

index 28cb1726fc..b9dca05283 100644
--- a/src/core/qgstransaction.cpp
+++ b/src/core/qgstransaction.cpp
@@ -72,14 +72,20 @@ QString QgsTransaction::connectionString() const
 }
 
 // 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 )
+// any reference to layers and filters in the connection string
+QString QgsTransaction::cleanupConnectionString( const QString &str )
 {
   QString res( str );
 
-  for ( int i = 0; i < 2; i++ )
+  static const QStringList toRemove {
+    { QStringLiteral( "|layername=" )},
+    { QStringLiteral( "|layerid=" )},
+    { QStringLiteral( "|subset=" )},
+  };
+
+  for (const auto &strToRm: std::as_const( toRemove ) )
   {
-    const int pos = res.indexOf( i == 0 ? QLatin1String( "|layername=" ) :  QLatin1String( "|layerid=" ) );
+    const int pos = res.indexOf( strToRm );
     if ( pos >= 0 )
     {
       const int end = res.indexOf( '|', pos + 1 );
@@ -105,7 +111,7 @@ QString QgsTransaction::connectionString( const QString &layerUri )
   // reference to layers from it.
   if ( connString.isEmpty() )
   {
-    connString = removeLayerIdOrName( layerUri );
+    connString = cleanupConnectionString( layerUri );
   }
   return connString;
 }
diff --git a/src/core/qgstransaction.h b/src/core/qgstransaction.h
index 8dee8842ba..a13e533649 100644
--- a/src/core/qgstransaction.h
+++ b/src/core/qgstransaction.h
@@ -201,7 +201,7 @@ class CORE_EXPORT QgsTransaction : public QObject SIP_ABSTRACT
 
     void setLayerTransactionIds( QgsTransaction *transaction );
 
-    static QString removeLayerIdOrName( const QString &str );
+    static QString cleanupConnectionString( const QString &str );
 
     virtual bool beginTransaction( QString &error, int statementTimeout ) = 0;
     virtual bool commitTransaction( QString &error ) = 0;

But there is a catch: while this restores the expected behavior in the transaction handling if the layer is not switched to read-only mode OGR seems to "forget" whatever was in the layer before the last SAVEPOINT.