Bug report #14262

Overflow on primary key with negative values; cannot save edits

Added by Sandro Santilli over 1 year ago. Updated about 1 year ago.

Status:Closed
Priority:Normal
Assignee:Sandro Santilli
Category:Data Provider/PostGIS
Affected QGIS version:2.14.3 Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:Yes Resolution:fixed/implemented
Crashes QGIS or corrupts data:No

Description

Spin off of #13958

Create a simple PostGIS table:


CREATE table my_lines(gid integer primary key, col integer);
SELECT AddGeometryColumn('my_lines', 'geom', 4326, 'MULTILINESTRING', 2);

INSERT INTO my_lines(gid, col, geom)
SELECT -1, 2, 'SRID=4326;MULTILINESTRING((0 0, 1 1))';

Note that the primary key has a negative value -1, which should be completely fine and previous versions of QGIS were OK with this.

This table can be added to QGIS, which shows the simple geometry on the main canvas normally, and the Layer properties correctly describes both gid and col fields as int4.

However, the attribute table shows the values of gid and col as ERROR, and clicking the feature with the Identify Features tool show gid of 4294967295, which is the overflow version of an unsigned integer value -1. Perhaps the primary key was internally cast in QGIS to unsigned long?

Editing the layer, e.g. with the Node tool fail on save, with this message:

src/providers/postgres/qgspostgresprovider.cpp: 2322: (changeGeometryValues) [1279ms] entering.
src/providers/postgres/qgspostgresprovider.cpp: 2395: (changeGeometryValues) [0ms] updating: UPDATE "public"."my_lines" SET "geom"=st_multi(st_geom
fromwkb($1::bytea,4326)) WHERE "gid"=$2
src/providers/postgres/qgspostgresprovider.cpp: 2405: (changeGeometryValues) [1ms] iterating over the map of changed geometries...
src/providers/postgres/qgspostgresprovider.cpp: 2411: (changeGeometryValues) [0ms] iterating over feature id 4294967295
src/core/qgsvectordataprovider.cpp: 560: (pushError) [0ms] PostGIS error while changing geometry values: ERROR:  value "4294967295" is out of range
 for type integer

src/providers/postgres/qgspostgresprovider.cpp: 2501: (changeGeometryValues) [0ms] leaving.
src/core/qgsmessagelog.cpp: 45: (logMessage) [1ms] 2016-02-09T17:20:03 [1] Commit errors:
  ERROR: 1 geometries not changed.

  Provider errors:
      PostGIS error while changing geometry values: ERROR:  value "4294967295" is out of range for type integer

Affected: master branch 2.14.0dev (b9726d7285733c27d42456c115e28d5a37f3e0be)

Note that 2.8.3 crashes on first edit attempt. The crash is fixed in current master.

History

#1 Updated by Giovanni Manghi over 1 year ago

  • Status changed from Open to Feedback

if it worked ok on previous qgis releases this should be tagged as blocker and have 2.14 has target. Cheers!

#2 Updated by Sandro Santilli over 1 year ago

As mentioned, it crashed with 2.8.4. I hadn't tested any previous release, but 2.8 being an LTR I wouldn't go any earlier

#3 Updated by Giovanni Manghi over 1 year ago

Sandro Santilli wrote:

As mentioned, it crashed with 2.8.4. I hadn't tested any previous release, but 2.8 being an LTR I wouldn't go any earlier

I'm not speaking about the crash, but about the subject of this ticket ("Note that the primary key has a negative value -1, which should be completely fine and previous versions of QGIS were OK with this.").

#4 Updated by Sandro Santilli over 1 year ago

Mike Toews did not specify which previous version worked. I did test with 2.8.4 and the feature id is still reported as an unsigned and attribute values reported as ERROR. Do you confirm, Giovanni ?

#5 Updated by Sandro Santilli over 1 year ago

I just tried 2.6.1-Brighton and it also displays ERROR in attribute values, max unsigned integer on identify and crashes on edit.

#6 Updated by Sandro Santilli over 1 year ago

2.4.0-Chugiak same behavior as 2.6.1-Brighton and 2.8.4

#7 Updated by Sandro Santilli over 1 year ago

It looks like QgsPostgresConn::getBinaryInt is responsible of converting the signed integer to an unsigned one:

qint64 QgsPostgresConn::getBinaryInt( QgsPostgresResult &queryResult, int row, int col )
{                                                                               
  quint64 oid; 

Jurgen, what's the rationale for that ?

#8 Updated by Sandro Santilli over 1 year ago

  • Pull Request or Patch supplied changed from No to Yes
  • % Done changed from 0 to 80
  • Target version changed from Future Release - High Priority to Version 2.14
  • Assignee set to Sandro Santilli
  • Status changed from Feedback to In Progress

#9 Updated by Sandro Santilli over 1 year ago

Saving edits is handled by not treating integer fields in any special way by
https://github.com/qgis/QGIS/pull/2797

#10 Updated by Sandro Santilli over 1 year ago

  • Category set to Data Provider/PostGIS

#11 Updated by Sandro Santilli over 1 year ago

Saves can be edited with https://github.com/qgis/QGIS/pull/2797, that is using a field map based feature id for the case of integer primary key.

#12 Updated by Sandro Santilli about 1 year ago

Current focus for this bug is here: https://github.com/qgis/QGIS/pull/3036

#13 Updated by Sandro Santilli about 1 year ago

  • % Done changed from 80 to 100
  • Target version changed from Version 2.14 to Version 2.16
  • Resolution set to fixed/implemented
  • Affected QGIS version changed from master to 2.14.3
  • Status changed from In Progress to Closed

Fixed as of d1cac84 -- I suspect this didn't work in 2.8 either.

#14 Updated by Sandro Santilli about 1 year ago

Question: should this be backported to 2.14 ?

#15 Updated by Sandro Santilli about 1 year ago

  • Status changed from Closed to Reopened

Will test against 2.14, but given 2.14 is the LTR, it'd be best fixed there (please correct me if I'm wrong)

#16 Updated by Sandro Santilli about 1 year ago

  • Status changed from Reopened to Closed

Tested: the crash is fixed in 2.14(.3) while the "cannot save edits" part is only fixed in 2.16.
As we aim for stability, I'd not backport the ability to edit (or save edits) in 2.14. Feel free to reopen if you think it should be done instead.

Also available in: Atom PDF