Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix postgres pkey map in Qt5 (fixes #15223)
Switching from QVariant to QVariantList solves the underlying Qt issue:
- comparison of QVariantList objects works fine
- comparison of QVariantList objects wrapped in QVariant does not work

The extra wrapping of QVariantList into another QVariant seems unnecessary anyway,
so we may as well save a tiny bit of memory and cpu
  • Loading branch information
wonder-sk committed Aug 11, 2016
1 parent a72485c commit 46f7c64
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 32 deletions.
4 changes: 2 additions & 2 deletions src/providers/postgres/qgspostgresfeatureiterator.cpp
Expand Up @@ -733,7 +733,7 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int

case pktFidMap:
{
QList<QVariant> primaryKeyVals;
QVariantList primaryKeyVals;

Q_FOREACH ( int idx, mSource->mPrimaryKeyAttrs )
{
Expand All @@ -748,7 +748,7 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
col++;
}

fid = mSource->mShared->lookupFid( QVariant( primaryKeyVals ) );
fid = mSource->mShared->lookupFid( primaryKeyVals );

}
break;
Expand Down
42 changes: 18 additions & 24 deletions src/providers/postgres/qgspostgresprovider.cpp
Expand Up @@ -319,6 +319,8 @@ QString QgsPostgresProvider::storageType() const
}

// Qt5 has that built in
// ... BUT it does not behave exactly the same way as our implementation
// (e.g. comparison of QVariantList does not work)
#if QT_VERSION < 0x050000
static bool operator<( const QVariant &a, const QVariant &b )
{
Expand Down Expand Up @@ -488,11 +490,9 @@ void QgsPostgresProvider::appendPkParams( QgsFeatureId featureId, QStringList &p

case pktFidMap:
{
QVariant pkValsVariant = mShared->lookupKey( featureId );
QList<QVariant> pkVals;
if ( !pkValsVariant.isNull() )
QVariantList pkVals = mShared->lookupKey( featureId );
if ( !pkVals.isEmpty() )
{
pkVals = pkValsVariant.toList();
Q_ASSERT( pkVals.size() == mPrimaryKeyAttrs.size() );
}

Expand Down Expand Up @@ -554,11 +554,9 @@ QString QgsPostgresUtils::whereClause( QgsFeatureId featureId, const QgsFields&

case pktFidMap:
{
QVariant pkValsVariant = sharedData->lookupKey( featureId );
if ( !pkValsVariant.isNull() )
QVariantList pkVals = sharedData->lookupKey( featureId );
if ( !pkVals.isEmpty() )
{
QList<QVariant> pkVals = pkValsVariant.toList();

Q_ASSERT( pkVals.size() == pkAttrs.size() );

QString delim = "";
Expand Down Expand Up @@ -2058,14 +2056,14 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist )
}
else
{
QList<QVariant> primaryKeyVals;
QVariantList primaryKeyVals;

Q_FOREACH ( int idx, mPrimaryKeyAttrs )
{
primaryKeyVals << attrs.at( idx );
}

features->setFeatureId( mShared->lookupFid( QVariant( primaryKeyVals ) ) );
features->setFeatureId( mShared->lookupFid( primaryKeyVals ) );
}
QgsDebugMsgLevel( QString( "new fid=%1" ).arg( features->id() ), 4 );
}
Expand Down Expand Up @@ -2416,9 +2414,7 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
// update feature id map if key was changed
if ( pkChanged && mPrimaryKeyType == pktFidMap )
{
QVariant v = mShared->removeFid( fid );

QList<QVariant> k = v.toList();
QVariantList k = mShared->removeFid( fid );

for ( int i = 0; i < mPrimaryKeyAttrs.size(); i++ )
{
Expand Down Expand Up @@ -2766,9 +2762,7 @@ bool QgsPostgresProvider::changeFeatures( const QgsChangedAttributesMap &attr_ma
// update feature id map if key was changed
if ( pkChanged && mPrimaryKeyType == pktFidMap )
{
QVariant v = mShared->removeFid( fid );

QList<QVariant> k = v.toList();
QVariantList k = mShared->removeFid( fid );

for ( int i = 0; i < mPrimaryKeyAttrs.size(); i++ )
{
Expand Down Expand Up @@ -4291,11 +4285,11 @@ void QgsPostgresSharedData::setFeaturesCounted( long count )
}


QgsFeatureId QgsPostgresSharedData::lookupFid( const QVariant &v )
QgsFeatureId QgsPostgresSharedData::lookupFid( const QVariantList& v )
{
QMutexLocker locker( &mMutex );

QMap<QVariant, QgsFeatureId>::const_iterator it = mKeyToFid.constFind( v );
QMap<QVariantList, QgsFeatureId>::const_iterator it = mKeyToFid.constFind( v );

if ( it != mKeyToFid.constEnd() )
{
Expand All @@ -4309,30 +4303,30 @@ QgsFeatureId QgsPostgresSharedData::lookupFid( const QVariant &v )
}


QVariant QgsPostgresSharedData::removeFid( QgsFeatureId fid )
QVariantList QgsPostgresSharedData::removeFid( QgsFeatureId fid )
{
QMutexLocker locker( &mMutex );

QVariant v = mFidToKey[ fid ];
QVariantList v = mFidToKey[ fid ];
mFidToKey.remove( fid );
mKeyToFid.remove( v );
return v;
}

void QgsPostgresSharedData::insertFid( QgsFeatureId fid, const QVariant& k )
void QgsPostgresSharedData::insertFid( QgsFeatureId fid, const QVariantList& k )
{
QMutexLocker locker( &mMutex );

mFidToKey.insert( fid, k );
mKeyToFid.insert( k, fid );
}

QVariant QgsPostgresSharedData::lookupKey( QgsFeatureId featureId )
QVariantList QgsPostgresSharedData::lookupKey( QgsFeatureId featureId )
{
QMutexLocker locker( &mMutex );

QMap<QgsFeatureId, QVariant>::const_iterator it = mFidToKey.constFind( featureId );
QMap<QgsFeatureId, QVariantList>::const_iterator it = mFidToKey.constFind( featureId );
if ( it != mFidToKey.constEnd() )
return it.value();
return QVariant();
return QVariantList();
}
12 changes: 6 additions & 6 deletions src/providers/postgres/qgspostgresprovider.h
Expand Up @@ -508,19 +508,19 @@ class QgsPostgresSharedData
void ensureFeaturesCountedAtLeast( long fetched );

// FID lookups
QgsFeatureId lookupFid( const QVariant &v ); // lookup existing mapping or add a new one
QVariant removeFid( QgsFeatureId fid );
void insertFid( QgsFeatureId fid, const QVariant& k );
QVariant lookupKey( QgsFeatureId featureId );
QgsFeatureId lookupFid( const QVariantList& v ); // lookup existing mapping or add a new one
QVariantList removeFid( QgsFeatureId fid );
void insertFid( QgsFeatureId fid, const QVariantList& k );
QVariantList lookupKey( QgsFeatureId featureId );

protected:
QMutex mMutex; //!< Access to all data members is guarded by the mutex

long mFeaturesCounted; //! Number of features in the layer

QgsFeatureId mFidCounter; // next feature id if map is used
QMap<QVariant, QgsFeatureId> mKeyToFid; // map key values to feature id
QMap<QgsFeatureId, QVariant> mFidToKey; // map feature back to fea
QMap<QVariantList, QgsFeatureId> mKeyToFid; // map key values to feature id
QMap<QgsFeatureId, QVariantList> mFidToKey; // map feature id back to key values
};

#endif

0 comments on commit 46f7c64

Please sign in to comment.