Bug report #16927

Can't commit changes to postgis 2D linestring layer when snapping to 3D point layer

Added by Jochen Huber about 3 years ago. Updated almost 3 years ago.

Status:Closed
Priority:High
Assignee:Luigi Pirelli
Category:Data Provider/PostGIS
Affected QGIS version:2.18.11 Regression?:Yes
Operating System:Windows 7 Easy fix?:No
Pull Request or Patch supplied:Yes Resolution:
Crashes QGIS or corrupts data:No Copied to github as #:24826

Description

Situation:
  • 2D linestring layer from postgis db in editing mode
  • New line is drawn, the points are selected by snapping to a 3D point layer in the same db
  • Commiting the changes fails with the error "geometry type is not compatible with the current layer."
    -> When drawing a line with arbitrary points (snapping may be active, but the new points are not created by snapping to a point layer)no error occurs;
    -> When snapping to points of a 2D point layer, no error occurs
    QGIS seems to try to add 3D coordinates to the new 2D line feature instead of using only the first two dimensions.

Associated revisions

Revision d3d8f4dd
Added by Larry Shaffer almost 3 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 Giovanni Manghi about 3 years ago

  • Status changed from Open to Feedback

Did it happened in an older qgis release?

#2 Updated by Jochen Huber about 3 years ago

  • Status changed from Feedback to Open

With 2.18.4 on Windows, the issue is basically the same, but the error message is marked as a PostGIS data provider error and is more useful: "ERROR: Geometry has Z dimension but column does not".
With 2.10.1 on Windows, the issue does NOT occur - so it seems to be a regression.
These are the only older version I currently have available, but I could install a specific version and check again if this would be useful.

#3 Updated by Giovanni Manghi about 3 years ago

Jochen Huber wrote:

With 2.18.4 on Windows, the issue is basically the same, but the error message is marked as a PostGIS data provider error and is more useful: "ERROR: Geometry has Z dimension but column does not".
With 2.10.1 on Windows, the issue does NOT occur - so it seems to be a regression.
These are the only older version I currently have available, but I could install a specific version and check again if this would be useful.

it would be nice if you could test on the actual LTR release, 2.14.17. Thanks!

#4 Updated by Giovanni Manghi about 3 years ago

  • Priority changed from Normal to High
  • Regression? changed from No to Yes

#5 Updated by Jochen Huber about 3 years ago

Tested on LTR release (2.14.17): Issue confirmed, error message: "Geometry has Z dimension but column does not"
Tested on Master (2.99.0, code version cb088a2): Issue confirmed, error message: "geometry type is not compatible with the current layer."

#6 Updated by Jürgen Fischer about 3 years ago

  • Description updated (diff)

#7 Updated by Tudor Bărăscu almost 3 years ago

Just hit this issue today. Tested it under 2.14 and it gives this error:

PostGIS error while adding features: ERROR:  Geometry has Z dimension but column does not

It's also present under 2.18 and master but with a different error message:
Just hit this issue today. Tested it under 2.14 and it gives this error:
ERROR: 1 feature(s) not added - geometry type is not compatible with the current layer.

I would like to add that the issue only appears if you start from a 3d point (it isn't present if you clicked somewhere without snapping and then end your snap to a 3d point).

#8 Updated by Luigi Pirelli almost 3 years ago

so what is the error here? IMHO seems correct that a 3D feature shouldn't be commited on 2D layer. Is it the problem related only to the better message?
In case of 2.18, the message is more generic because QGIS prevent to send features to provider if it is not compatible. So the more detailed provider message is never rised.
It has to be considered a regression the change in message detail?
BTW We can add the geometry type of source and target geometry if this can give more useful feedback. Any opinion?

#9 Updated by Giovanni Manghi almost 3 years ago

Luigi Pirelli wrote:

so what is the error here? IMHO seems correct that a 3D feature shouldn't be commited on 2D layer.

they are two different layers, from the description:

2D linestring layer from postgis db in editing mode
New line is drawn, the points are selected by snapping to a 3D point layer in the same db

#10 Updated by Luigi Pirelli almost 3 years ago

Sincerly I almost scared if the commitChanges function have to decide what would be the user intention. IMHO the fix shouldn't be inside commitChanges (that can give more detailed message). e.g. a compatible geometry would arrive during commit, if not, commitChanges would still fail (as inthis case).
The fix should be in some other places. e.g:
A) during snap (e.g. addidng an option to strictly follow in source gem type => no 3d type conversion)
B) outside snap, but inside addFeature (I didn't investigte the code)... in this way the error is rised as soon a feature is added (but not yet committed)

any opinion? I would prefer B to leave Snapping more general and hide checks at "business function" level

Giovanni Manghi wrote:

Luigi Pirelli wrote:

so what is the error here? IMHO seems correct that a 3D feature shouldn't be commited on 2D layer.

they are two different layers, from the description:

2D linestring layer from postgis db in editing mode
New line is drawn, the points are selected by snapping to a 3D point layer in the same db

#11 Updated by Jochen Huber almost 3 years ago

As described, in this case no error should be raised, since snapping delivers 3D coordinates for a 2D feature, so the third dimension should simply be ignored.
The "regression" mark is not there because the kind of error has changed, but because it used to work without error (at least until version 2.10.1). Probably addFeature should discard the Z coordinate (might apply to M values as well) if it is not needed.

#12 Updated by Luigi Pirelli almost 3 years ago

IMHO a solution should leave the snapping to modify 2D to 3D. The reason is to allow user to rise up a flat vector if this is the user intention.
some considerations:
1) IMHO if there is a single-2-multy type conversion have to be notified
2) We can add a snapping option like "mantain type cardinality" (True by default) that avoid to change geom dimension
3) add a generic check in the editing buffer that check geom compatibility as soon as a geom is changed.

3) can be solved as stated here: https://qgis.org/api/2.18/qgsvectorlayereditbuffer_8cpp_source.html#l00197
"// TODO: check compatible geometry"
in this place can be added the same controls set in commitChanges() of the same class... or moved these controls in a reusable methods.
Would be greate to have this control outside EditBuffed or as static method and with SIP biunding to allow other application to do this check for their business functions.-

#13 Updated by Jürgen Fischer almost 3 years ago

  • Assignee deleted (Jürgen Fischer)

#14 Updated by Luigi Pirelli almost 3 years ago

  • Assignee set to Luigi Pirelli

#15 Updated by Anonymous almost 3 years ago

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

#16 Updated by Luigi Pirelli almost 3 years ago

  • Pull Request or Patch supplied changed from No to Yes

Also available in: Atom PDF