Don't cast PGSQL bigint PKs to text on queries.

PostgreSQL bigint primary keys are no longer cast to text on queries/updates. Internally we keep a FID map between the real database value and the internal QGIS feature ID. Thus, we preserve the QGIS semantics of using negative FIDs for newly added features, while still allowing users to edit attributes whose PKs are bigint non-positive.
qgis · Apr 16, 2020 · 0d0fe48 · 0d0fe48
1 parent 71ad8c8
commit 0d0fe48
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 27 deletions.
diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp
@@ -1625,6 +1625,10 @@ QString QgsPostgresConn::fieldExpression( const QgsField &fld, QString expr )
   {
     return QStringLiteral( "st_astext(%1)" ).arg( expr );
   }
+  else if ( type == QLatin1String( "int8" ) )
+  {
+    return expr;
+  }
   //TODO: add support for hstore
   //TODO: add support for json/jsonb
   else

diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp
@@ -664,7 +664,6 @@ bool QgsPostgresFeatureIterator::declareCursor( const QString &whereClause, long
   return true;
 }
 
-
 bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int row, QgsFeature &feature )
 {
   feature.initAttributes( mSource->mFields.count() );
@@ -745,21 +744,37 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
       break;
 
     case PktInt:
-    case PktInt64:
-    case PktUint64:
       fid = mConn->getBinaryInt( queryResult, row, col++ );
       if ( !subsetOfAttributes || fetchAttributes.contains( mSource->mPrimaryKeyAttrs.at( 0 ) ) )
       {
         feature.setAttribute( mSource->mPrimaryKeyAttrs[0], fid );
       }
-      if ( mSource->mPrimaryKeyType == PktInt )
+      // NOTE: this needs be done _after_ the setAttribute call
+      // above as we want the attribute value to be 1:1 with
+      // database value
+      fid = QgsPostgresUtils::int32pk_to_fid( fid );
+      break;
+
+    case PktUint64:
+    case PktInt64:
+    {
+      QVariantList pkVal;
+
+      int idx = mSource->mPrimaryKeyAttrs[0];
+      QgsField fld = mSource->mFields.at( idx );
+
+      QVariant v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), QString::number( mConn->getBinaryInt( queryResult, row, col ) ), fld.typeName() );
+      pkVal << v;
+
+      if ( !subsetOfAttributes || fetchAttributes.contains( idx ) )
       {
-        // NOTE: this needs be done _after_ the setAttribute call
-        // above as we want the attribute value to be 1:1 with
-        // database value
-        fid = QgsPostgresUtils::int32pk_to_fid( fid );
+        feature.setAttribute( idx, v );
       }
-      break;
+      col++;
+
+      fid = mSource->mShared->lookupFid( pkVal );
+    }
+    break;
 
     case PktFidMap:
     {

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -280,12 +280,12 @@ QgsPostgresProvider::QgsPostgresProvider( QString const &uri, const ProviderOpti
       key = QStringLiteral( "tid" );
       break;
     case PktInt:
-    case PktInt64:
-    case PktUint64:
       Q_ASSERT( mPrimaryKeyAttrs.size() == 1 );
       Q_ASSERT( mPrimaryKeyAttrs[0] >= 0 && mPrimaryKeyAttrs[0] < mAttributeFields.count() );
       key = mAttributeFields.at( mPrimaryKeyAttrs.at( 0 ) ).name();
       break;
+    case PktInt64:
+    case PktUint64:
     case PktFidMap:
     {
       QString delim;
@@ -453,12 +453,12 @@ QString QgsPostgresProvider::pkParamWhereClause( int offset, const char *alias )
       break;
 
     case PktInt:
-    case PktInt64:
-    case PktUint64:
       Q_ASSERT( mPrimaryKeyAttrs.size() == 1 );
       whereClause = QStringLiteral( "%3%1=$%2" ).arg( quotedIdentifier( field( mPrimaryKeyAttrs[0] ).name() ) ).arg( offset ).arg( aliased );
       break;
 
+    case PktInt64:
+    case PktUint64:
     case PktFidMap:
     {
       QString delim;
@@ -495,8 +495,6 @@ void QgsPostgresProvider::appendPkParams( QgsFeatureId featureId, QStringList &p
   switch ( mPrimaryKeyType )
   {
     case PktOid:
-    case PktInt64:
-    case PktUint64:
       params << QString::number( featureId );
       break;
 
@@ -508,6 +506,8 @@ void QgsPostgresProvider::appendPkParams( QgsFeatureId featureId, QStringList &p
       params << QStringLiteral( "'(%1,%2)'" ).arg( FID_TO_NUMBER( featureId ) >> 16 ).arg( FID_TO_NUMBER( featureId ) & 0xffff );
       break;
 
+    case PktInt64:
+    case PktUint64:
     case PktFidMap:
     {
       QVariantList pkVals = mShared->lookupKey( featureId );
@@ -567,11 +567,22 @@ QString QgsPostgresUtils::whereClause( QgsFeatureId featureId, const QgsFields &
       whereClause = QStringLiteral( "%1=%2" ).arg( QgsPostgresConn::quotedIdentifier( fields.at( pkAttrs[0] ).name() ) ).arg( FID2PKINT( featureId ) );
       break;
 
-    case PktUint64:
     case PktInt64:
+    case PktUint64:
+    {
       Q_ASSERT( pkAttrs.size() == 1 );
-      whereClause = QStringLiteral( "%1=%2" ).arg( QgsPostgresConn::quotedIdentifier( fields.at( pkAttrs[0] ).name() ) ).arg( featureId );
-      break;
+      QVariantList pkVals = sharedData->lookupKey( featureId );
+      if ( !pkVals.isEmpty() )
+      {
+        QgsField fld = fields.at( pkAttrs[0] );
+        whereClause = conn->fieldExpression( fld );
+        if ( !pkVals[0].isNull() )
+          whereClause += "=" + pkVals[0].toString();
+        else
+          whereClause += QLatin1String( " IS NULL" );
+      }
+    }
+    break;
 
     case PktFidMap:
     {
@@ -618,8 +629,6 @@ QString QgsPostgresUtils::whereClause( const QgsFeatureIds &featureIds, const Qg
   {
     case PktOid:
     case PktInt:
-    case PktInt64:
-    case PktUint64:
     {
       QString expr;
 
@@ -632,14 +641,16 @@ QString QgsPostgresUtils::whereClause( const QgsFeatureIds &featureIds, const Qg
         const auto constFeatureIds = featureIds;
         for ( const QgsFeatureId featureId : constFeatureIds )
         {
-          expr += delim + FID_TO_STRING( ( pkType == PktOid ? featureId : pkType == PktUint64 ? featureId : FID2PKINT( featureId ) ) );
+          expr += delim + FID_TO_STRING( ( pkType == PktOid ? featureId : FID2PKINT( featureId ) ) );
           delim = ',';
         }
         expr += ')';
       }
 
       return expr;
     }
+    case PktInt64:
+    case PktUint64:
     case PktFidMap:
     case PktTid:
     case PktUnknown:
@@ -2459,11 +2470,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
         {
           QgsAttributes attrs = features->attributes();
 
-          if ( mPrimaryKeyType == PktUint64 || mPrimaryKeyType == PktInt64 )
-          {
-            features->setId( STRING_TO_FID( attrs.at( mPrimaryKeyAttrs.at( 0 ) ) ) );
-          }
-          else if ( mPrimaryKeyType == PktInt )
+          if ( mPrimaryKeyType == PktInt )
           {
             features->setId( PKINT2FID( STRING_TO_FID( attrs.at( mPrimaryKeyAttrs.at( 0 ) ) ) ) );
           }

diff --git a/src/providers/postgres/qgspostgresprovider.h b/src/providers/postgres/qgspostgresprovider.h
@@ -504,7 +504,7 @@ class QgsPostgresUtils
 
     static QString andWhereClauses( const QString &c1, const QString &c2 );
 
-    static const qint64 INT32PK_OFFSET = 4294967296;
+    static const qint64 INT32PK_OFFSET = 4294967296; // 2^32
 
     // We shift negative 32bit integers to above the max 32bit
     // positive integer to support the whole range of int32 values