Do not loose signed semantic on converting int32 pk to fid

Fixes again editing of features with negative identifiers. Now a test fails, but it is the test being broken this time (testSignedIdentifiers). The test was added as part of 2bd7f44, which fixed a crash when using signed identifiers.
qgis · Jun 9, 2016 · 92dda42 · 92dda42
1 parent f4a135b
commit 92dda42
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 6 deletions.
diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp
@@ -718,6 +718,10 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
     case pktInt:
     case pktUint64:
       fid = mConn->getBinaryInt( queryResult, row, col++ );
+      if ( mSource->mPrimaryKeyType == pktInt )
+      {
+        fid = QgsPostgresUtils::int32pk_to_fid( fid );
+      }
       if ( !subsetOfAttributes || fetchAttributes.contains( mSource->mPrimaryKeyAttrs.at( 0 ) ) )
       {
         feature.setAttribute( mSource->mPrimaryKeyAttrs[0], fid );

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -42,17 +42,14 @@
 const QString POSTGRES_KEY = "postgres";
 const QString POSTGRES_DESCRIPTION = "PostgreSQL/PostGIS data provider";
 
-// We convert signed 32bit integers to unsigned 64bit integers
-// to support the whole range of int32 values (including negative)
-// See http://hub.qgis.org/issues/14262
-inline uint64_t PKINT2FID( int32_t x )
+inline int64_t PKINT2FID( int32_t x )
 {
-  return static_cast<uint64_t>( x );
+  return QgsPostgresUtils::int32pk_to_fid( x );
 }
 
 inline int32_t FID2PKINT( int64_t x )
 {
-  return static_cast<int32_t>( x );
+  return QgsPostgresUtils::fid_to_int32pk( x );
 }
 
 

diff --git a/src/providers/postgres/qgspostgresprovider.h b/src/providers/postgres/qgspostgresprovider.h
@@ -502,6 +502,21 @@ class QgsPostgresUtils
                                 QSharedPointer<QgsPostgresSharedData> sharedData );
 
     static QString andWhereClauses( const QString& c1, const QString& c2 );
+
+    static const int64_t int32pk_offset = 4294967296;
+
+    // We shift negative 32bit integers to above the max 32bit
+    // positive integer to support the whole range of int32 values
+    // See http://hub.qgis.org/issues/14262
+    static int64_t int32pk_to_fid( int32_t x )
+    {
+      return x >= 0 ? x : x + int32pk_offset;
+    }
+
+    static int32_t fid_to_int32pk( int64_t x )
+    {
+      return x <= (( int32pk_offset ) / 2.0 ) ? x : -( int32pk_offset - x );
+    }
 };
 
 /** Data shared between provider class and its feature sources. Ideally there should