Skip to content

Commit

Permalink
Support negative short integer primary keys on PostgreSQL
Browse files Browse the repository at this point in the history
See http://hub.qgis.org/issues/14262

NOTE: signed int64 keys would still fail
  • Loading branch information
Sandro Santilli authored and strk committed Jun 9, 2016
1 parent 6d8f5c4 commit a7a0eef
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 16 deletions.
1 change: 1 addition & 0 deletions src/providers/postgres/qgspostgresconn.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ enum QgsPostgresPrimaryKeyType
{
pktUnknown,
pktInt,
pktInt64,
pktTid,
pktOid,
pktFidMap
Expand Down
10 changes: 8 additions & 2 deletions src/providers/postgres/qgspostgresfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ bool QgsPostgresFeatureIterator::declareCursor( const QString& whereClause, long
break;

case pktInt:
case pktInt64:
query += delim + QgsPostgresConn::quotedIdentifier( mSource->mFields.at( mSource->mPrimaryKeyAttrs.at( 0 ) ).name() );
delim = ',';
break;
Expand Down Expand Up @@ -711,11 +712,16 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
{
case pktOid:
case pktTid:
fid = mConn->getBinaryInt( queryResult, row, col++ );
break;

case pktInt:
case pktInt64:
fid = mConn->getBinaryInt( queryResult, row, col++ );
if ( mSource->mPrimaryKeyType == pktInt &&
( !subsetOfAttributes || fetchAttributes.contains( mSource->mPrimaryKeyAttrs.at( 0 ) ) ) )
if ( !subsetOfAttributes || fetchAttributes.contains( mSource->mPrimaryKeyAttrs.at( 0 ) ) )
{
feature.setAttribute( mSource->mPrimaryKeyAttrs[0], fid );
}
break;

case pktFidMap:
Expand Down
66 changes: 52 additions & 14 deletions src/providers/postgres/qgspostgresprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@
const QString POSTGRES_KEY = "postgres";
const QString POSTGRES_DESCRIPTION = "PostgreSQL/PostGIS data provider";

// Add this offset to the database integer primary keys
// to support the whole range of values (including negative)
// See http://hub.qgis.org/issues/14262
#define PKINT_OFFSET (2^32)

QgsPostgresProvider::QgsPostgresProvider( QString const & uri )
: QgsVectorDataProvider( uri )
Expand Down Expand Up @@ -194,6 +198,7 @@ QgsPostgresProvider::QgsPostgresProvider( QString const & uri )
key = "tid";
break;
case pktInt:
case pktInt64:
Q_ASSERT( mPrimaryKeyAttrs.size() == 1 );
Q_ASSERT( mPrimaryKeyAttrs[0] >= 0 && mPrimaryKeyAttrs[0] < mAttributeFields.count() );
key = mAttributeFields.at( mPrimaryKeyAttrs.at( 0 ) ).name();
Expand Down Expand Up @@ -402,6 +407,7 @@ QString QgsPostgresProvider::pkParamWhereClause( int offset, const char *alias )
break;

case pktInt:
case pktInt64:
Q_ASSERT( mPrimaryKeyAttrs.size() == 1 );
whereClause = QString( "%3%1=$%2" ).arg( quotedIdentifier( field( mPrimaryKeyAttrs[0] ).name() ) ).arg( offset ).arg( aliased );
break;
Expand Down Expand Up @@ -442,10 +448,14 @@ void QgsPostgresProvider::appendPkParams( QgsFeatureId featureId, QStringList &p
switch ( mPrimaryKeyType )
{
case pktOid:
case pktInt:
case pktInt64:
params << QString::number( featureId );
break;

case pktInt:
params << QString::number( featureId - PKINT_OFFSET );
break;

case pktTid:
params << QString( "'(%1,%2)'" ).arg( FID_TO_NUMBER( featureId ) >> 16 ).arg( FID_TO_NUMBER( featureId ) & 0xffff );
break;
Expand Down Expand Up @@ -507,6 +517,11 @@ QString QgsPostgresUtils::whereClause( QgsFeatureId featureId, const QgsFields&
break;

case pktInt:
Q_ASSERT( pkAttrs.size() == 1 );
whereClause = QString( "%1=%2" ).arg( QgsPostgresConn::quotedIdentifier( fields[ pkAttrs[0] ].name() ) ).arg( featureId - PKINT_OFFSET );
break;

case pktInt64:
Q_ASSERT( pkAttrs.size() == 1 );
whereClause = QString( "%1=%2" ).arg( QgsPostgresConn::quotedIdentifier( fields[ pkAttrs[0] ].name() ) ).arg( featureId );
break;
Expand Down Expand Up @@ -558,6 +573,7 @@ QString QgsPostgresUtils::whereClause( const QgsFeatureIds& featureIds, const Qg
{
case pktOid:
case pktInt:
case pktInt64:
{
QString expr;

Expand All @@ -569,7 +585,7 @@ QString QgsPostgresUtils::whereClause( const QgsFeatureIds& featureIds, const Qg

Q_FOREACH ( const QgsFeatureId featureId, featureIds )
{
expr += delim + FID_TO_STRING( featureId );
expr += delim + FID_TO_STRING( pkType == pktOid ? featureId : pkType == pktInt64 ? featureId : featureId - PKINT_OFFSET );
delim = ',';
}
expr += ')';
Expand Down Expand Up @@ -1285,11 +1301,11 @@ bool QgsPostgresProvider::determinePrimaryKey()
res = connectionRO()->PQexec( sql );
QgsDebugMsg( QString( "Got %1 rows." ).arg( res.PQntuples() ) );

bool isInt = true;
bool mightBeNull = false;
QString primaryKey;
QString delim = "";

mPrimaryKeyType = pktFidMap; // int by default, will downgrade if needed
for ( int i = 0; i < res.PQntuples(); i++ )
{
QString name = res.PQgetvalue( i, 0 );
Expand All @@ -1310,16 +1326,22 @@ bool QgsPostgresProvider::determinePrimaryKey()
}
const QgsField& fld = mAttributeFields.at( idx );

if ( isInt &&
fld.type() != QVariant::Int &&
fld.type() != QVariant::LongLong )
isInt = false;
if ( i )
{
mPrimaryKeyType = pktFidMap; // multi-field
}
else if ( fld.type() == QVariant::LongLong )
{
mPrimaryKeyType = pktInt64; // 64bit integer
}
else if ( fld.type() == QVariant::Int )
{
mPrimaryKeyType = pktInt;
}

mPrimaryKeyAttrs << idx;
}

mPrimaryKeyType = ( mPrimaryKeyAttrs.size() == 1 && isInt ) ? pktInt : pktFidMap;

if (( mightBeNull || isParentTable ) && !mUseEstimatedMetadata && !uniqueData( primaryKey ) )
{
QgsMessageLog::logMessage( tr( "Ignoring key candidate because of NULL values or inheritance" ), tr( "PostGIS" ) );
Expand Down Expand Up @@ -1407,7 +1429,19 @@ void QgsPostgresProvider::determinePrimaryKeyFromUriKeyColumn()
{
if ( mUseEstimatedMetadata || uniqueData( primaryKey ) )
{
mPrimaryKeyType = ( mPrimaryKeyAttrs.size() == 1 && ( mAttributeFields.at( mPrimaryKeyAttrs.at( 0 ) ).type() == QVariant::Int || mAttributeFields.at( mPrimaryKeyAttrs.at( 0 ) ).type() == QVariant::LongLong ) ) ? pktInt : pktFidMap;
mPrimaryKeyType = pktFidMap; // Map by default
if ( mPrimaryKeyAttrs.size() == 1 )
{
const QgsField& fld = mAttributeFields.at( 0 );
if ( fld.type() == QVariant::LongLong )
{
mPrimaryKeyType = pktInt64; // 64bit integer
}
else if ( fld.type() == QVariant::Int )
{
mPrimaryKeyType = pktInt; // account for signed
}
}
}
else
{
Expand Down Expand Up @@ -1835,7 +1869,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist )
delim = ',';
}

if ( mPrimaryKeyType == pktInt || mPrimaryKeyType == pktFidMap )
if ( mPrimaryKeyType == pktInt || mPrimaryKeyType == pktFidMap || mPrimaryKeyType == pktInt64 )
{
Q_FOREACH ( int idx, mPrimaryKeyAttrs )
{
Expand Down Expand Up @@ -1944,7 +1978,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist )

insert += values + ')';

if ( mPrimaryKeyType == pktFidMap || mPrimaryKeyType == pktInt )
if ( mPrimaryKeyType == pktFidMap || mPrimaryKeyType == pktInt || mPrimaryKeyType == pktInt64 )
{
insert += " RETURNING ";

Expand Down Expand Up @@ -2020,16 +2054,20 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist )
}

// update feature ids
if ( mPrimaryKeyType == pktInt || mPrimaryKeyType == pktFidMap )
if ( mPrimaryKeyType == pktInt || mPrimaryKeyType == pktFidMap || mPrimaryKeyType == pktInt64 )
{
for ( QgsFeatureList::iterator features = flist.begin(); features != flist.end(); ++features )
{
QgsAttributes attrs = features->attributes();

if ( mPrimaryKeyType == pktInt )
if ( mPrimaryKeyType == pktInt64 )
{
features->setFeatureId( STRING_TO_FID( attrs.at( mPrimaryKeyAttrs.at( 0 ) ) ) );
}
else if ( mPrimaryKeyType == pktInt )
{
features->setFeatureId( STRING_TO_FID( attrs.at( mPrimaryKeyAttrs.at( 0 ) ) ) + PKINT_OFFSET );
}
else
{
QList<QVariant> primaryKeyVals;
Expand Down

0 comments on commit a7a0eef

Please sign in to comment.