Bug report #15741

PostGIS issue when using 'Merge selected features' tool (Geometry type does not match column type)

Added by Reinhard Reiterer almost 3 years ago. Updated almost 2 years ago.

Status:Closed
Priority:High
Assignee:Luigi Pirelli
Category:Digitising
Affected QGIS version:2.18.11 Regression?:Yes
Operating System: Easy fix?:No
Pull Request or Patch supplied:Yes Resolution:
Crashes QGIS or corrupts data:No Copied to github as #:23663

Description

The 'Merge selected features' tool is also available for non touching features. This may cause data loss when using PostGIS single geometry data types!

15741.mp4 - screencast (2.19 MB) Reinhard Reiterer, 2016-10-22 12:21 AM

15741_2.wmv - screencast #02 (754 KB) Reinhard Reiterer, 2016-10-27 12:30 AM

15741_3.mp4 (3.15 MB) Reinhard Reiterer, 2016-10-27 11:10 AM

15741_4.mp4 (2.24 MB) Reinhard Reiterer, 2016-10-27 11:10 AM

Associated revisions

Revision 53d90b54
Added by Luigi Pirelli over 2 years ago

fix Postgis Merge selected features regression: port 2.18 fixes #15741

Revision 87116abd
Added by Luigi Pirelli over 2 years ago

fix Postgis Merge selected features regression: port 3.0 fixes #15741

Revision e850c82c
Added by Alexander Bruy over 2 years ago

Merge pull request #4604 from boundlessgeo/postgis_merge_features_port3_fix#15741

Fix Postgis Merge selected features regression (fix #15741)

Revision 57e122bb
Added by Alexander Bruy over 2 years ago

Merge pull request #4594 from boundlessgeo/postgis_merge_features_fix#15741

Fix Postgis Merge selected features regression (fix #15741)

Revision d3d8f4dd
Added by Larry Shaffer almost 2 years ago

Merge pull request #5223 from boundlessgeo/geom_compatibility_check_release-2_18-fix15741

On behalf of Giovanni: Geom compatibility check release 2.18 fixes #15741 #16927

History

#1 Updated by Reinhard Reiterer almost 3 years ago

#2 Updated by Reinhard Reiterer almost 3 years ago

Merging touching features also fails in some cases (see screencast 15741_3.mp4, 15741_4.mp4).

#3 Updated by Giovanni Manghi over 2 years ago

  • Crashes QGIS or corrupts data changed from No to Yes
  • Category set to Digitising
  • Priority changed from High to Severe/Regression
  • Affected QGIS version changed from 2.16.3 to 2.18.2

Confirmed also on 2.18.2, and raising its priority as this was not an issue in the past, in fact trying such operation on 2.14.* it gives

Merge cancelled: The union operation would result in a geometry type that is not compatible with the current layer.

#4 Updated by Giovanni Manghi over 2 years ago

  • Target version set to Version 2.18
  • Affected QGIS version changed from 2.18.2 to 2.18.4

2.18.4 also affected.

#5 Updated by Giovanni Manghi over 2 years ago

  • Regression? set to Yes

#6 Updated by Giovanni Manghi over 2 years ago

  • Priority changed from Severe/Regression to High

#7 Updated by Giovanni Manghi over 2 years ago

  • Easy fix? set to No

#8 Updated by Luigi Pirelli over 2 years ago

studing the issue

#9 Updated by Luigi Pirelli over 2 years ago

this issue would affect any provider due to:

https://github.com/qgis/QGIS/blob/release-2_18/src/app/qgisapp.cpp#L7177

delete is done before add feature and there is no way to specify a transaction (btw not all providers support transactions)

#10 Updated by Luigi Pirelli over 2 years ago

my fault, if provider support transaction it can be managed with QgsTransaction

#11 Updated by Luigi Pirelli over 2 years ago

the error is generated here:
https://qgis.org/api/2.18/qgsvectorlayereditbuffer_8cpp_source.html#l00520
e.g. the the update process is in two phases:
1) remove features to merge
2) add the new merged feature
because of https://qgis.org/api/2.18/qgsvectorlayereditbuffer_8cpp_source.html#l00520
and as specified in documentation, if some of the composed steps fail (delete success, and add fail), then we can recover only the last one (the add) during rollback because editingBuffer has already removed from the rollback cache the first step (delete)

#12 Updated by Luigi Pirelli over 2 years ago

in this use case no use og QgsTransaction or QgsTransactionGroup is used

#13 Updated by Luigi Pirelli over 2 years ago

  • Assignee set to Luigi Pirelli

#14 Updated by Luigi Pirelli over 2 years ago

asked in dev list what would be a better solution... especially asked to MarcoH about his commit

#15 Updated by Luigi Pirelli over 2 years ago

reverting (solution 1) does not generate more test regressions (btw coverage in this area is poor)
tested and solution 1 fix the problem. Waiting for community comments to pack PR if solution 1 is accepted

#16 Updated by Luigi Pirelli over 2 years ago

the issue affect qgis3 too

#17 Updated by Luigi Pirelli over 2 years ago

solution 1 works for 3.x

#18 Updated by Luigi Pirelli over 2 years ago

without any answer in list... I'll implement solution 1). Preparing PR

#19 Updated by Luigi Pirelli over 2 years ago

  • Pull Request or Patch supplied changed from No to Yes

#20 Updated by Luigi Pirelli over 2 years ago

  • % Done changed from 0 to 100
  • Status changed from Open to Closed

#21 Updated by Luigi Pirelli over 2 years ago

Luigi Pirelli wrote:

Applied in changeset qgis|87116abd72ac52be042c3bff195a3cc931b035e6.

fix applied also to 3.0 e850c82cb01b0ae66d7d3c1f7781fd677ba2ec1f

#22 Updated by Giovanni Manghi about 2 years ago

  • Crashes QGIS or corrupts data changed from Yes to No
  • Status changed from Closed to Open
  • Affected QGIS version changed from 2.18.4 to 2.18.11

This is not completely fixed yet.

The good new is that it seems that the data loss is fixed BUT when trying to merge two geometries in a layer that does not suppurt MULTI geometries do NOT immediately stop the operation as it happens in 2.14. There a dialog shows with the message "Merge cancelled: The union operation would result in a geometry type that is not compatible with the current layer.".

Now the user is not warned, the operation seemingly is ok, but then on save edits QGIS throws an error. If the user meanwhile did more legit digitizing operations he/she has to discard everything, possibly losing a lot of work.

#23 Updated by Peteris Bruns about 2 years ago

The same behavior in dev version (revision 313ec55) as described in #16974( using the same attached test data). Difference only in error message:

Errors: SUCCESS: 4 feature(s) deleted.
  ERROR: 2 feature(s) not added.

  Provider errors:
      OGR error creating feature -2: failed to prepare SQL: INSERT INTO bug_examples ( "geom", "name") VALUES (?, ?)
      OGR error creating feature -3: failed to prepare SQL: INSERT INTO bug_examples ( "geom", "name") VALUES (?, ?)

#24 Updated by Luigi Pirelli about 2 years ago

I've to take it again... generally speaking there are two step editing

step1... all edits are in vector editbuffer, thyat does not have any relation with the real data provider. But there are checks about provider capabilities.
step2... changes are saved => the data provider start to play a role

Giovanni Manghi wrote:

This is not completely fixed yet.

The good new is that it seems that the data loss is fixed BUT when trying to merge two geometries in a layer that does not suppurt MULTI geometries do NOT immediately stop the operation as it happens in 2.14. There a dialog shows with the message "Merge cancelled: The union operation would result in a geometry type that is not compatible with the current layer.".

Now the user is not warned, the operation seemingly is ok, but then on save edits QGIS throws an error. If the user meanwhile did more legit digitizing operations he/she has to discard everything, possibly losing a lot of work.

your request is to remove this last action from the editbuffer?

regards

#25 Updated by Giovanni Manghi about 2 years ago

your request is to remove this last action from the editbuffer?

clarified in a chat session.

#26 Updated by Giovanni Manghi almost 2 years ago

https://github.com/qgis/QGIS/pull/5223 seems to have addressed the issue. Thanks!

#27 Updated by Anonymous almost 2 years ago

  • Status changed from Open to Closed

Also available in: Atom PDF