Skip to content

Commit

Permalink
Fix Postgresql connection reset not being called
Browse files Browse the repository at this point in the history
PQreset was never called if the query was made using mConnectionRO
from the PostgresProvider, resulting in an always failing state.
Fixes #20170

(cherry picked from commit f30a15c)
  • Loading branch information
AchilleAsh authored and nyalldawson committed Feb 20, 2019
1 parent 235a667 commit 31adb15
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 57 deletions.
113 changes: 58 additions & 55 deletions src/providers/postgres/qgspostgresconn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand All @@ -1057,45 +1083,48 @@ 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 ( logError )
{
QgsMessageLog::logMessage( tr( "Query failed: %1\nError: no result buffer" ).arg( query ), tr( "PostGIS" ) );
}
else
{
QgsDebugMsg( QStringLiteral( "Not logged query failed: %1\nError: no result buffer" ).arg( query ) );
}
}

if ( res )
if ( retry )
{
QgsMessageLog::logMessage( tr( "resetting bad connection." ), tr( "PostGIS" ) );
::PQreset( mConn );
res = PQexec( query, logError, false );
if ( PQstatus() == CONNECTION_OK )
{
int errorStatus = PQresultStatus( res );
if ( errorStatus != PGRES_COMMAND_OK && errorStatus != PGRES_TUPLES_OK )
if ( res )
{
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 ) ) );
}
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 if ( logError )
{
QgsMessageLog::logMessage( tr( "Query failed: %1\nError: no result buffer" ).arg( query ), tr( "PostGIS" ) );
}
else
{
QgsDebugMsg( QStringLiteral( "Not logged query failed: %1\nError: no result buffer" ).arg( query ) );
QgsMessageLog::logMessage( tr( "connection still bad after reset." ), tr( "PostGIS" ) );
}
}
else
{
QgsMessageLog::logMessage( tr( "bad connection, not retrying." ), tr( "PostGIS" ) );
}
return nullptr;

return res;
}

bool QgsPostgresConn::openCursor( const QString &cursorName, const QString &sql )
Expand Down Expand Up @@ -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 )
{
QgsPostgresResult res( PQexec( query, false ) );

Expand All @@ -1163,32 +1192,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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/providers/postgres/qgspostgresconn.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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;
Expand Down

0 comments on commit 31adb15

Please sign in to comment.