From 99179d1caf32fd044a9681b9f0ea7ff733b5855a Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 10 May 2024 07:20:09 +0200 Subject: [PATCH] Reduce queries to detect select privileges for PostgreSQL relations (#57389) Reduce queries to detect select privileges for PostgreSQL relations Use a single query rather than 3 to determine recovery status, select privilege and update/insert/delete privileges. Also drops the need to evaluate view definitions to determine selectability. Co-authored-by: Juergen E. Fischer --- .../postgres/qgspostgresprovider.cpp | 139 +++++++++--------- .../python/test_provider_postgres_latency.py | 2 +- 2 files changed, 69 insertions(+), 72 deletions(-) diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index 9d716742887d..c964d57e0ea2 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -1492,113 +1492,110 @@ bool QgsPostgresProvider::hasSufficientPermsAndCapabilities() mEnabledCapabilities = QgsVectorDataProvider::Capability::ReloadData; + QString sql; QgsPostgresResult testAccess; + + bool forceReadOnly = ( mReadFlags & QgsDataProvider::ForceReadOnly ); + bool inRecovery = false; + sql = QStringLiteral( "SELECT " + "has_table_privilege(%1,'SELECT')," // 0 + "pg_is_in_recovery()," // 1 + "current_schema(), " // 2 + "has_table_privilege(%1,'INSERT')," // 3 + "has_table_privilege(%1,'DELETE')" ) // 4 + .arg( quotedValue( mQuery ) ); + if ( !mIsQuery ) { - // Check that we can read from the table (i.e., we have select permission). - QString sql = QStringLiteral( "SELECT * FROM %1 LIMIT 1" ).arg( mQuery ); - QgsPostgresResult testAccess( connectionRO()->LoggedPQexec( "QgsPostgresProvider", sql ) ); + + // postgres has fast access to features at id (thanks to primary key / unique index) + // the latter flag is here just for compatibility + if ( !mSelectAtIdDisabled ) + { + mEnabledCapabilities |= QgsVectorDataProvider::SelectAtId; + } + + if ( connectionRO()->pgVersion() >= 80400 ) + { + sql += QString( ",has_any_column_privilege(%1,'UPDATE')" // 5 + ",%2" ) // 6 + .arg( quotedValue( mQuery ), + mGeometryColumn.isNull() + ? QStringLiteral( "'f'" ) + : QStringLiteral( "has_column_privilege(%1,%2,'UPDATE')" ) + .arg( quotedValue( mQuery ), + quotedValue( mGeometryColumn ) ) + ); + } + else + { + sql += QString( ",has_table_privilege(%1,'UPDATE')" // 5 + ",has_table_privilege(%1,'UPDATE')" ) // 6 + .arg( quotedValue( mQuery ) ); + } + + testAccess = connectionRO()->LoggedPQexec( "QgsPostgresProvider", sql ); if ( testAccess.PQresultStatus() != PGRES_TUPLES_OK ) { - QgsMessageLog::logMessage( tr( "Unable to access the %1 relation.\nThe error message from the database was:\n%2.\nSQL: %3" ) + QgsMessageLog::logMessage( tr( "Unable to determine table access privileges for the %1 relation.\nThe error message from the database was:\n%2.\nSQL: %3" ) .arg( mQuery, testAccess.PQresultErrorMessage(), - sql ), tr( "PostGIS" ) ); + sql ), + tr( "PostGIS" ) ); return false; } - bool forceReadOnly = ( mReadFlags & QgsDataProvider::ForceReadOnly ); - bool inRecovery = false; - // Check if the database is still in recovery after a database crash - // or if you are connected to a (read-only) standby server - // only if the provider has not been force to be in read-only mode - if ( !forceReadOnly && connectionRO()->pgVersion() >= 90000 ) + if ( testAccess.PQgetvalue( 0, 0 ) != QLatin1String( "t" ) ) { - testAccess = connectionRO()->LoggedPQexec( "QgsPostgresProvider", QStringLiteral( "SELECT pg_is_in_recovery()" ) ); - if ( testAccess.PQresultStatus() != PGRES_TUPLES_OK || testAccess.PQgetvalue( 0, 0 ) == QLatin1String( "t" ) ) - { - QgsMessageLog::logMessage( tr( "PostgreSQL is still in recovery after a database crash\n(or you are connected to a (read-only) standby server).\nWrite accesses will be denied." ), tr( "PostGIS" ) ); - inRecovery = true; - } + // SELECT + QgsMessageLog::logMessage( tr( "User has no SELECT privilege on %1 relation." ) + .arg( mQuery ), tr( "PostGIS" ) ); + return false; } - // postgres has fast access to features at id (thanks to primary key / unique index) - // the latter flag is here just for compatibility - if ( !mSelectAtIdDisabled ) + if ( testAccess.PQgetvalue( 0, 1 ) == QLatin1String( "t" ) ) { - mEnabledCapabilities |= QgsVectorDataProvider::SelectAtId; + // RECOVERY + QgsMessageLog::logMessage( + tr( "PostgreSQL is still in recovery after a database crash\n(or you are connected to a (read-only) standby server).\nWrite accesses will be denied." ), + tr( "PostGIS" ) + ); + inRecovery = true; } - // Do not check the editable capabilities if the provider has been forced to be + // CURRENT SCHEMA + if ( mSchemaName.isEmpty() ) + mSchemaName = testAccess.PQgetvalue( 0, 2 ); + + // Do not set editable capabilities if the provider has been forced to be // in read-only mode or if the database is still in recovery if ( !forceReadOnly && !inRecovery ) { - if ( connectionRO()->pgVersion() >= 80400 ) - { - sql = QString( "SELECT " - "has_table_privilege(%1,'DELETE')," - "has_any_column_privilege(%1,'UPDATE')," - "%2" - "has_table_privilege(%1,'INSERT')," - "current_schema()" ) - .arg( quotedValue( mQuery ), - mGeometryColumn.isNull() - ? QStringLiteral( "'f'," ) - : QStringLiteral( "has_column_privilege(%1,%2,'UPDATE')," ) - .arg( quotedValue( mQuery ), - quotedValue( mGeometryColumn ) ) - ); - } - else - { - sql = QString( "SELECT " - "has_table_privilege(%1,'DELETE')," - "has_table_privilege(%1,'UPDATE')," - "has_table_privilege(%1,'UPDATE')," - "has_table_privilege(%1,'INSERT')," - "current_schema()" ) - .arg( quotedValue( mQuery ) ); - } - - testAccess = connectionRO()->LoggedPQexec( "QgsPostgresProvider", sql ); - if ( testAccess.PQresultStatus() != PGRES_TUPLES_OK ) + if ( testAccess.PQgetvalue( 0, 3 ) == QLatin1String( "t" ) ) { - QgsMessageLog::logMessage( tr( "Unable to determine table access privileges for the %1 relation.\nThe error message from the database was:\n%2.\nSQL: %3" ) - .arg( mQuery, - testAccess.PQresultErrorMessage(), - sql ), - tr( "PostGIS" ) ); - return false; + // INSERT + mEnabledCapabilities |= QgsVectorDataProvider::AddFeatures; } - - if ( testAccess.PQgetvalue( 0, 0 ) == QLatin1String( "t" ) ) + if ( testAccess.PQgetvalue( 0, 4 ) == QLatin1String( "t" ) ) { // DELETE mEnabledCapabilities |= QgsVectorDataProvider::DeleteFeatures | QgsVectorDataProvider::FastTruncate; } - if ( testAccess.PQgetvalue( 0, 1 ) == QLatin1String( "t" ) ) + if ( testAccess.PQgetvalue( 0, 5 ) == QLatin1String( "t" ) ) { // UPDATE mEnabledCapabilities |= QgsVectorDataProvider::ChangeAttributeValues; } - if ( testAccess.PQgetvalue( 0, 2 ) == QLatin1String( "t" ) ) + if ( testAccess.PQgetvalue( 0, 6 ) == QLatin1String( "t" ) ) { - // UPDATE + // UPDATE (geom column specific) mEnabledCapabilities |= QgsVectorDataProvider::ChangeGeometries; } - if ( testAccess.PQgetvalue( 0, 3 ) == QLatin1String( "t" ) ) - { - // INSERT - mEnabledCapabilities |= QgsVectorDataProvider::AddFeatures; - } - - if ( mSchemaName.isEmpty() ) - mSchemaName = testAccess.PQgetvalue( 0, 4 ); - + // TODO: merge this with the previous query sql = QString( "SELECT 1 FROM pg_class,pg_namespace WHERE " "pg_class.relnamespace=pg_namespace.oid AND " "%3 AND " diff --git a/tests/src/python/test_provider_postgres_latency.py b/tests/src/python/test_provider_postgres_latency.py index ce138c82601c..707a24e62a09 100644 --- a/tests/src/python/test_provider_postgres_latency.py +++ b/tests/src/python/test_provider_postgres_latency.py @@ -156,7 +156,7 @@ def testProjectOpenTime(self): settings = QgsSettingsTree.node('core').childSetting('provider-parallel-loading') settings.setVariantValue(False) - davg = 8.67 + davg = 7.61 dmin = round(davg - 0.2, 2) dmax = round(davg + 0.3, 2) error_string = 'expected from {0}s to {1}s, got {2}s\nHINT: set davg={2} to pass the test :)'