[QGIS-Developer] editbuffet::commitChanges limitations found working on #15741... suggestions?

[Luigi Pirelli]

just a short note, solution 1) fix tre issue without introduction more
regression tests (btw coverage is poor in this area)


Luigi Pirelli

**************************************************************************************************
* Boundless QGIS Support/Development: lpirelli AT boundlessgeo DOT com
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
**************************************************************************************************


On 16 May 2017 at 11:25, Luigi Pirelli <luipir at gmail.com> wrote:
> Hi (especially MarcoH)
>
> Analysis:
> working on #15741 [1] I found that the the issue is comes mainly from
> the following commit:
> https://github.com/qgis/QGIS/commit/8d70a5141b0f56c40fc19e32d5d55475e22ec10f
> and secondary from:
> https://github.com/qgis/QGIS/commit/7b64feb7e101ac7f205737dce241c9d8fa03c452#diff-989db6afac5dc91e42cf6d3e404431b3L6656
> that do a better cleanup of the previous commit.
>
> btw these commits just allowed to raise a limitation of the:
> QgsVectorLayerEditBuffer::commitChanges [2] class (see doc [3])
> commitChanges is a multi staged commit task with a limitation, each
> stage, if success, clean it's stage buffer => if can be recovered if
> one of the next stages fail.
>
> This is exactly what happen in the issue. Merge Features is done doing:
>
> 1) deleted features to merge
> 2) add the new merged feature
>
> stage 2 will fail (due the fact that if postgis layer is linestring,
> the merged is multi type => incompatible). but failing the roll back
> (done aborting the editing session) rollback only the stage 2, because
> stage 1 was success and cleaned from editBuffer.
>
> Splutions:
> There is no a unique solution. These are the options:
> 1) revert or partially revert the above  commit
> 8d70a5141b0f56c40fc19e32d5d55475e22ec10f
> 2) don't clean the stage buffers, and do them only if no error is
> found (opening to other regressions)
> 3) refactor editing buffer (would make sense integrating
> QgsTransaction and QgsTransactionGroups where possible)... but IMHO
> out of the scope of a bug fix
> 4) anything else?
>
> I would prefer (my personal style) for a soft solution (1)... any opinion?
>
> Luigi Pirelli
>
> [1] https://issues.qgis.org/issues/15741
> [2] https://github.com/qgis/QGIS/blob/release-2_18/src/core/qgsvectorlayereditbuffer.cpp#L292
> [3] https://github.com/qgis/QGIS/blob/release-2_18/src/core/qgsvectorlayereditbuffer.h#L93
>
> **************************************************************************************************
> * Boundless QGIS Support/Development: lpirelli AT boundlessgeo DOT com
> * LinkedIn: https://www.linkedin.com/in/luigipirelli
> * Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
> * GitHub: https://github.com/luipir
> * Mastering QGIS 2nd Edition:
> * https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
> **************************************************************************************************