Skip to content

Commit

Permalink
Fix handling of int64 primary keys to use a FidMap
Browse files Browse the repository at this point in the history
There was a code duplication where only one path did the right
thing. This commit also removes the duplication.
  • Loading branch information
strk committed Jun 9, 2016
1 parent 8bd6ffc commit 8b9a035
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 28 deletions.
49 changes: 24 additions & 25 deletions src/providers/postgres/qgspostgresprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,26 @@ inline int32_t FID2PKINT( int64_t x )
return QgsPostgresUtils::fid_to_int32pk( x );
}

QgsPostgresPrimaryKeyType
QgsPostgresProvider::pkType( const QgsField& f ) const
{
switch ( f.type() )
{
case QVariant::LongLong:
// unless we can guarantee all values are unsigned
// (in which case we could use pktUint64)
// we'll have to use a Map type.
// See http://hub.qgis.org/issues/14262
return pktFidMap; // pktUint64

case QVariant::Int:
return pktInt;

default:
return pktFidMap;
}
}



QgsPostgresProvider::QgsPostgresProvider( QString const & uri )
Expand Down Expand Up @@ -1312,7 +1332,7 @@ bool QgsPostgresProvider::determinePrimaryKey()
QString primaryKey;
QString delim = "";

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

if ( i )
{
mPrimaryKeyType = pktFidMap; // multi-field
}
else if ( fld.type() == QVariant::LongLong )
{
// unless we can guarantee all values are unsigned
// (in which case we could use pktUint64)
// we'll have to use a Map type.
// See http://hub.qgis.org/issues/14262
mPrimaryKeyType = pktFidMap; // pktUint64
}
else if ( fld.type() == QVariant::Int )
{
mPrimaryKeyType = pktInt;
}
// Always use pktFidMap for multi-field keys
mPrimaryKeyType = i ? pktFidMap : pkType( fld );

mPrimaryKeyAttrs << idx;
}
Expand Down Expand Up @@ -1444,14 +1450,7 @@ void QgsPostgresProvider::determinePrimaryKeyFromUriKeyColumn()
if ( mPrimaryKeyAttrs.size() == 1 )
{
const QgsField& fld = mAttributeFields.at( 0 );
if ( fld.type() == QVariant::LongLong )
{
mPrimaryKeyType = pktUint64; // 64bit integer
}
else if ( fld.type() == QVariant::Int )
{
mPrimaryKeyType = pktInt; // account for signed
}
mPrimaryKeyType = pkType( fld );
}
}
else
Expand Down
11 changes: 11 additions & 0 deletions src/providers/postgres/qgspostgresprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,17 @@ class QgsPostgresProvider : public QgsVectorDataProvider
*/
bool parseDomainCheckConstraint( QStringList& enumValues, const QString& attributeName ) const;

/** Return the type of primary key for a PK field
*
* @param fld the field to determine PK type of
* @return the PrimaryKeyType
*
* @note that this only makes sense for single-field primary keys,
* whereas multi-field keys always need the pktFidMap
* primary key type.
*/
QgsPostgresPrimaryKeyType pkType( const QgsField& fld ) const;

QgsFields mAttributeFields;
QString mDataComment;

Expand Down
4 changes: 1 addition & 3 deletions tests/src/python/test_provider_postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,7 @@ def test_query_attribute(dbconn, query, att, val, fidval):
self.assertEqual(count, 1)
test_query_attribute(self.dbconn, '(SELECT -1::int4 i, NULL::geometry(Point) g)', 'i', -1, 4294967295)
test_query_attribute(self.dbconn, '(SELECT -2::int2 i, NULL::geometry(Point) g)', 'i', -2, 4294967294)
# Unfortunately negative int8 identifiers are still not supported at this moment
# TODO: fix expected fidval to be positive !
test_query_attribute(self.dbconn, '(SELECT -3::int8 i, NULL::geometry(Point) g)', 'i', -3, -3)
test_query_attribute(self.dbconn, '(SELECT -3::int8 i, NULL::geometry(Point) g)', 'i', -3, 1)

def testPktIntInsert(self):
vl = QgsVectorLayer('{} table="qgis_test"."{}" key="pk" sql='.format(self.dbconn, 'bikes_view'), "bikes_view", "postgres")
Expand Down

0 comments on commit 8b9a035

Please sign in to comment.