From bf13d09d5c34fbd72be6afec430ed51b8042ab77 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 20 May 2019 13:56:07 +1000 Subject: [PATCH] Revert "postgres provider: performance improvements when loading layers" This reverts commit 2220b86e2e3d26a84b7533cb6c4fb0ee87d7fdc4. The commit broke existing unit tests --- src/core/qgsvectorlayer.cpp | 5 +- src/providers/postgres/qgspostgresconn.cpp | 32 +-------- src/providers/postgres/qgspostgresconn.h | 12 ---- .../postgres/qgspostgresprovider.cpp | 65 ++++++++++++++----- .../services/wms/qgswmsgetcapabilities.cpp | 6 +- 5 files changed, 57 insertions(+), 63 deletions(-) diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp index 9058782d0a6..d536bcf1be2 100644 --- a/src/core/qgsvectorlayer.cpp +++ b/src/core/qgsvectorlayer.cpp @@ -773,6 +773,7 @@ QgsRectangle QgsVectorLayer::extent() const if ( !isSpatial() ) return rect; + if ( !mValidExtent && mLazyExtent && mDataProvider && !mDataProvider->hasMetadata() && mReadExtentFromXml && !mXmlExtent.isNull() ) { mExtent = mXmlExtent; @@ -780,7 +781,7 @@ QgsRectangle QgsVectorLayer::extent() const mLazyExtent = false; } - if ( !mValidExtent && mLazyExtent && !mReadExtentFromXml && mDataProvider && mDataProvider->isValid() ) + if ( !mValidExtent && mLazyExtent && mDataProvider && mDataProvider->isValid() ) { // get the extent QgsRectangle mbr = mDataProvider->extent(); @@ -811,7 +812,7 @@ QgsRectangle QgsVectorLayer::extent() const // get the extent of the layer from the provider // but only when there are some features already - if ( !mReadExtentFromXml && mDataProvider->featureCount() != 0 ) + if ( mDataProvider->featureCount() != 0 ) { QgsRectangle r = mDataProvider->extent(); rect.combineExtentWith( r ); diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp index 475b60598ff..ced20e7e6b3 100644 --- a/src/providers/postgres/qgspostgresconn.cpp +++ b/src/providers/postgres/qgspostgresconn.cpp @@ -149,9 +149,8 @@ QgsPostgresConn *QgsPostgresConn::connectDb( const QString &conninfo, bool reado // This is called from may places where shared parameter cannot be forced to false (QgsVectorLayerExporter) // and which is run in a different thread (drag and drop in browser) - if ( shared && QApplication::instance()->thread() != QThread::currentThread() ) + if ( QApplication::instance()->thread() != QThread::currentThread() ) { - QgsDebugMsg( QStringLiteral( "Connection cannot be shared between threads" ) ); shared = false; } @@ -170,6 +169,7 @@ QgsPostgresConn *QgsPostgresConn::connectDb( const QString &conninfo, bool reado } QgsPostgresConn *conn = new QgsPostgresConn( conninfo, readonly, shared, transaction ); + if ( conn->mRef == 0 ) { delete conn; @@ -1933,31 +1933,3 @@ QString QgsPostgresConn::currentDatabase() const return database; } - -const QgsPostgresConn::PGTypeInfo &QgsPostgresConn::type( int oid ) const -{ - if ( mTypeMap.isEmpty() ) - { - QMutexLocker locker( &mLock ); - if ( mTypeMap.isEmpty() ) - { - QString sql = QStringLiteral( "SELECT oid,typname,typtype,typelem,typlen FROM pg_type" ); - QgsPostgresResult typeResult( PQexec( sql ) ); - - for ( int i = 0; i < typeResult.PQntuples(); ++i ) - { - PGTypeInfo typeInfo = - { - /* typeName = */ typeResult.PQgetvalue( i, 1 ), - /* typeType = */ typeResult.PQgetvalue( i, 2 ), - /* typeElem = */ typeResult.PQgetvalue( i, 3 ), - /* typeLen = */ typeResult.PQgetvalue( i, 4 ).toInt() - }; - mTypeMap.insert( typeResult.PQgetvalue( i, 0 ).toInt(), typeInfo ); - } - QgsDebugMsg( "types retrieved" ); - } - } - - return mTypeMap[oid]; -} diff --git a/src/providers/postgres/qgspostgresconn.h b/src/providers/postgres/qgspostgresconn.h index 2f71ffc306a..b71a13c84ea 100644 --- a/src/providers/postgres/qgspostgresconn.h +++ b/src/providers/postgres/qgspostgresconn.h @@ -364,16 +364,6 @@ class QgsPostgresConn : public QObject void lock() { mLock.lock(); } void unlock() { mLock.unlock(); } - struct PGTypeInfo - { - QString typeName; - QString typeType; - QString typeElem; - int typeLen; - }; - - const PGTypeInfo &type( int oid ) const; - private: QgsPostgresConn( const QString &conninfo, bool readOnly, bool shared, bool transaction ); ~QgsPostgresConn() override; @@ -446,8 +436,6 @@ class QgsPostgresConn : public QObject bool mTransaction; - mutable QMap mTypeMap; - mutable QMutex mLock; }; diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index 809f8c80c61..50598413ac8 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -106,6 +106,19 @@ QgsPostgresProvider::QgsPostgresProvider( QString const &uri, const ProviderOpti mRequestedSrid = mUri.srid(); mRequestedGeomType = mUri.wkbType(); + if ( mUri.hasParam( QStringLiteral( "checkPrimaryKeyUnicity" ) ) ) + { + + if ( mUri.param( QStringLiteral( "checkPrimaryKeyUnicity" ) ).compare( QLatin1String( "0" ) ) == 0 ) + { + mCheckPrimaryKeyUnicity = false; + } + else + { + mCheckPrimaryKeyUnicity = true; + } + } + if ( mSchemaName.isEmpty() && mTableName.startsWith( '(' ) && mTableName.endsWith( ')' ) ) { mIsQuery = true; @@ -130,15 +143,12 @@ QgsPostgresProvider::QgsPostgresProvider( QString const &uri, const ProviderOpti mUseEstimatedMetadata = mUri.useEstimatedMetadata(); mSelectAtIdDisabled = mUri.selectAtIdDisabled(); - mCheckPrimaryKeyUnicity = !mUri.hasParam( QStringLiteral( "checkPrimaryKeyUnicity" ) ) || mUri.param( QStringLiteral( "checkPrimaryKeyUnicity" ) ).compare( QLatin1String( "0" ) ) != 0; - QgsDebugMsg( QStringLiteral( "Connection info is %1" ).arg( mUri.connectionInfo( false ) ) ); QgsDebugMsg( QStringLiteral( "Geometry column is: %1" ).arg( mGeometryColumn ) ); QgsDebugMsg( QStringLiteral( "Schema is: %1" ).arg( mSchemaName ) ); QgsDebugMsg( QStringLiteral( "Table name is: %1" ).arg( mTableName ) ); QgsDebugMsg( QStringLiteral( "Query is: %1" ).arg( mQuery ) ); QgsDebugMsg( QStringLiteral( "Where clause is: %1" ).arg( mSqlWhereClause ) ); - QgsDebugMsg( QStringLiteral( "Using estimated metadata: %1" ).arg( mUseEstimatedMetadata ) ); // no table/query passed, the provider could be used to get tables if ( mQuery.isEmpty() ) @@ -695,6 +705,14 @@ QString QgsPostgresProvider::endianString() } +struct PGTypeInfo +{ + QString typeName; + QString typeType; + QString typeElem; + int typeLen; +}; + bool QgsPostgresProvider::loadFields() { @@ -726,6 +744,24 @@ bool QgsPostgresProvider::loadFields() QgsPostgresResult result( connectionRO()->PQexec( sql ) ); + // Collect type info + sql = QStringLiteral( "SELECT oid,typname,typtype,typelem,typlen FROM pg_type" ); + QgsPostgresResult typeResult( connectionRO()->PQexec( sql ) ); + + QMap typeMap; + for ( int i = 0; i < typeResult.PQntuples(); ++i ) + { + PGTypeInfo typeInfo = + { + /* typeName = */ typeResult.PQgetvalue( i, 1 ), + /* typeType = */ typeResult.PQgetvalue( i, 2 ), + /* typeElem = */ typeResult.PQgetvalue( i, 3 ), + /* typeLen = */ typeResult.PQgetvalue( i, 4 ).toInt() + }; + typeMap.insert( typeResult.PQgetvalue( i, 0 ).toInt(), typeInfo ); + } + + QMap > fmtFieldTypeMap, descrMap, defValMap, identityMap; QMap > attTypeIdMap; QMap > notNullMap, uniqueMap; @@ -804,13 +840,12 @@ bool QgsPostgresProvider::loadFields() int attnum = result.PQftablecol( i ); int atttypid = attTypeIdMap[tableoid][attnum]; - const QgsPostgresConn::PGTypeInfo &typeInfo = connectionRO()->type( fldtyp ); + const PGTypeInfo &typeInfo = typeMap.value( fldtyp ); QString fieldTypeName = typeInfo.typeName; QString fieldTType = typeInfo.typeType; int fieldSize = typeInfo.typeLen; - const QgsPostgresConn::PGTypeInfo &attTypeInfo = connectionRO()->type( atttypid ); - bool isDomain = ( attTypeInfo.typeType == QLatin1String( "d" ) ); + bool isDomain = ( typeMap.value( atttypid ).typeType == QLatin1String( "d" ) ); QString formattedFieldType = fmtFieldTypeMap[tableoid][attnum]; QString originalFormattedFieldType = formattedFieldType; @@ -1757,8 +1792,7 @@ bool QgsPostgresProvider::parseDomainCheckConstraint( QStringList &enumValues, c enumValues.clear(); //is it a domain type with a check constraint? - QString domainSql = QStringLiteral( "SELECT domain_name,domain_schema FROM information_schema.columns WHERE table_schema=%1 AND table_name=%2 AND column_name=%3" ) - .arg( quotedValue( mSchemaName ), quotedValue( mTableName ), quotedValue( attributeName ) ); + QString domainSql = QStringLiteral( "SELECT domain_name, domain_schema FROM information_schema.columns WHERE table_name=%1 AND column_name=%2" ).arg( quotedValue( mTableName ), quotedValue( attributeName ) ); QgsPostgresResult domainResult( connectionRO()->PQexec( domainSql ) ); if ( domainResult.PQresultStatus() == PGRES_TUPLES_OK && domainResult.PQntuples() > 0 && !domainResult.PQgetvalue( 0, 0 ).isNull() ) { @@ -3376,7 +3410,7 @@ bool QgsPostgresProvider::getGeometryDetails() result = connectionRO()->PQexec( sql ); if ( tableoid > 0 && PGRES_TUPLES_OK == result.PQresultStatus() ) { - sql = QStringLiteral( "SELECT pg_namespace.nspname,pg_class.relname FROM pg_class JOIN pg_namespace ON pg_class.relnamespace=pg_namespace.oid WHERE pg_class.oid=%1" ).arg( tableoid ); + sql = QStringLiteral( "SELECT pg_namespace.nspname,pg_class.relname FROM pg_class,pg_namespace WHERE pg_class.relnamespace=pg_namespace.oid AND pg_class.oid=%1" ).arg( tableoid ); result = connectionRO()->PQexec( sql ); if ( PGRES_TUPLES_OK == result.PQresultStatus() && 1 == result.PQntuples() ) @@ -3384,7 +3418,7 @@ bool QgsPostgresProvider::getGeometryDetails() schemaName = result.PQgetvalue( 0, 0 ); tableName = result.PQgetvalue( 0, 1 ); - sql = QStringLiteral( "SELECT a.attname, t.typname FROM pg_attribute a JOIN pg_type t ON a.atttypid = t.oid WHERE a.attrelid=%1 AND a.attnum=%2" ).arg( tableoid ).arg( column ); + sql = QStringLiteral( "SELECT a.attname, t.typname FROM pg_attribute a, pg_type t WHERE a.attrelid=%1 AND a.attnum=%2 AND a.atttypid = t.oid" ).arg( tableoid ).arg( column ); result = connectionRO()->PQexec( sql ); if ( PGRES_TUPLES_OK == result.PQresultStatus() && 1 == result.PQntuples() ) { @@ -3533,12 +3567,11 @@ bool QgsPostgresProvider::getGeometryDetails() if ( mSpatialColType == SctNone ) { - sql = QString( "SELECT t.typname" - " FROM pg_attribute a" - " JOIN pg_class c ON a.attrelid=c.oid" - " JOIN pg_namespace n ON c.relnamespace=n.oid" - " JOIN pg_type t ON a.atttypid=t.oid" - " WHERE n.nspname=%3 AND c.relname=%1 AND a.attname=%2" ) + sql = QString( "SELECT t.typname FROM " + "pg_attribute a, pg_class c, pg_namespace n, pg_type t " + "WHERE a.attrelid=c.oid AND c.relnamespace=n.oid " + "AND a.atttypid=t.oid " + "AND n.nspname=%3 AND c.relname=%1 AND a.attname=%2" ) .arg( quotedValue( tableName ), quotedValue( geomCol ), quotedValue( schemaName ) ); diff --git a/src/server/services/wms/qgswmsgetcapabilities.cpp b/src/server/services/wms/qgswmsgetcapabilities.cpp index 9d1329a89fe..f1c9a9c9712 100644 --- a/src/server/services/wms/qgswmsgetcapabilities.cpp +++ b/src/server/services/wms/qgswmsgetcapabilities.cpp @@ -1062,10 +1062,10 @@ namespace QgsWms if ( l->type() == QgsMapLayerType::VectorLayer ) { QgsVectorLayer *vl = qobject_cast( l ); - if ( vl && ( project->trustLayerMetadata() || vl->featureCount() == 0 ) ) + if ( vl && vl->featureCount() == 0 ) { - // if we trust the project or there's no feature, use the wms - // extent defined in the project... + // if there's no feature, use the wms extent defined in the + // project... extent = QgsServerProjectUtils::wmsExtent( *project ); if ( extent.isNull() ) {