From f30a15ccecfd0e94828a82e33b38260623c2eeec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9=20Perez?= Date: Wed, 6 Feb 2019 00:40:39 +0100 Subject: [PATCH] Fix Postgresql connection reset not being called PQreset was never called if the query was made using mConnectionRO from the PostgresProvider, resulting in an always failing state. Fixes #20170 --- src/providers/postgres/qgspostgresconn.cpp | 115 +++++++++++---------- src/providers/postgres/qgspostgresconn.h | 4 +- 2 files changed, 61 insertions(+), 58 deletions(-) diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp index 0578ffc8cc1..e6d214f304f 100644 --- a/src/providers/postgres/qgspostgresconn.cpp +++ b/src/providers/postgres/qgspostgresconn.cpp @@ -1042,8 +1042,34 @@ QString QgsPostgresConn::quotedValue( const QVariant &value ) } } -PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError ) const +PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError, bool retry ) const { + QMutexLocker locker( &mLock ); + + QgsDebugMsgLevel( QStringLiteral( "Executing SQL: %1" ).arg( query ), 3 ); + + PGresult *res = ::PQexec( mConn, query.toUtf8() ); + + // libpq may return a non null ptr with conn status not OK so we need to check for it to allow a retry below + if ( res && PQstatus() == CONNECTION_OK ) + { + int errorStatus = PQresultStatus( res ); + if ( errorStatus != PGRES_COMMAND_OK && errorStatus != PGRES_TUPLES_OK ) + { + if ( logError ) + { + QgsMessageLog::logMessage( tr( "Erroneous query: %1 returned %2 [%3]" ) + .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ), + tr( "PostGIS" ) ); + } + else + { + QgsDebugMsg( QStringLiteral( "Not logged erroneous query: %1 returned %2 [%3]" ) + .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ) ); + } + } + return res; + } if ( PQstatus() != CONNECTION_OK ) { if ( logError ) @@ -1057,35 +1083,10 @@ PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError ) const QgsDebugMsg( QStringLiteral( "Connection error: %1 returned %2 [%3]" ) .arg( query ).arg( PQstatus() ).arg( PQerrorMessage() ) ); } - - return nullptr; } - - QgsDebugMsgLevel( QStringLiteral( "Executing SQL: %1" ).arg( query ), 3 ); - PGresult *res = nullptr; + else { - QMutexLocker locker( &mLock ); - res = ::PQexec( mConn, query.toUtf8() ); - - if ( res ) - { - int errorStatus = PQresultStatus( res ); - if ( errorStatus != PGRES_COMMAND_OK && errorStatus != PGRES_TUPLES_OK ) - { - if ( logError ) - { - QgsMessageLog::logMessage( tr( "Erroneous query: %1 returned %2 [%3]" ) - .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ), - tr( "PostGIS" ) ); - } - else - { - QgsDebugMsg( QStringLiteral( "Not logged erroneous query: %1 returned %2 [%3]" ) - .arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ) ); - } - } - } - else if ( logError ) + if ( logError ) { QgsMessageLog::logMessage( tr( "Query failed: %1\nError: no result buffer" ).arg( query ), tr( "PostGIS" ) ); } @@ -1095,7 +1096,35 @@ PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError ) const } } - return res; + if ( retry ) + { + QgsMessageLog::logMessage( tr( "resetting bad connection." ), tr( "PostGIS" ) ); + ::PQreset( mConn ); + res = PQexec( query, logError, false ); + if ( PQstatus() == CONNECTION_OK ) + { + if ( res ) + { + QgsMessageLog::logMessage( tr( "retry after reset succeeded." ), tr( "PostGIS" ) ); + return res; + } + else + { + QgsMessageLog::logMessage( tr( "retry after reset failed again." ), tr( "PostGIS" ) ); + return nullptr; + } + } + else + { + QgsMessageLog::logMessage( tr( "connection still bad after reset." ), tr( "PostGIS" ) ); + } + } + else + { + QgsMessageLog::logMessage( tr( "bad connection, not retrying." ), tr( "PostGIS" ) ); + } + return nullptr; + } bool QgsPostgresConn::openCursor( const QString &cursorName, const QString &sql ) @@ -1137,7 +1166,7 @@ QString QgsPostgresConn::uniqueCursorName() return QStringLiteral( "qgis_%1" ).arg( ++mNextCursorId ); } -bool QgsPostgresConn::PQexecNR( const QString &query, bool retry ) +bool QgsPostgresConn::PQexecNR( const QString &query ) { QMutexLocker locker( &mLock ); // to protect access to mOpenCursors @@ -1165,32 +1194,6 @@ bool QgsPostgresConn::PQexecNR( const QString &query, bool retry ) { PQexecNR( QStringLiteral( "ROLLBACK" ) ); } - else if ( retry ) - { - QgsMessageLog::logMessage( tr( "resetting bad connection." ), tr( "PostGIS" ) ); - ::PQreset( mConn ); - if ( PQstatus() == CONNECTION_OK ) - { - if ( PQexecNR( query, false ) ) - { - QgsMessageLog::logMessage( tr( "retry after reset succeeded." ), tr( "PostGIS" ) ); - return true; - } - else - { - QgsMessageLog::logMessage( tr( "retry after reset failed again." ), tr( "PostGIS" ) ); - return false; - } - } - else - { - QgsMessageLog::logMessage( tr( "connection still bad after reset." ), tr( "PostGIS" ) ); - } - } - else - { - QgsMessageLog::logMessage( tr( "bad connection, not retrying." ), tr( "PostGIS" ) ); - } return false; } diff --git a/src/providers/postgres/qgspostgresconn.h b/src/providers/postgres/qgspostgresconn.h index 1f95a477b69..837831b2b54 100644 --- a/src/providers/postgres/qgspostgresconn.h +++ b/src/providers/postgres/qgspostgresconn.h @@ -228,7 +228,7 @@ class QgsPostgresConn : public QObject int pgVersion() { return mPostgresqlVersion; } //! run a query and free result buffer - bool PQexecNR( const QString &query, bool retry = true ); + bool PQexecNR( const QString &query ); //! cursor handling bool openCursor( const QString &cursorName, const QString &declare ); @@ -245,7 +245,7 @@ class QgsPostgresConn : public QObject // // run a query and check for errors, thread-safe - PGresult *PQexec( const QString &query, bool logError = true ) const; + PGresult *PQexec( const QString &query, bool logError = true, bool retry = true ) const; void PQfinish(); QString PQerrorMessage() const; int PQstatus() const;