Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
AchilleAsh committed Feb 6, 2019
1 parent 06d5b99 commit f30a15c
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
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 )
{
QMutexLocker locker( &mLock ); // to protect access to mOpenCursors

Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/providers/postgres/qgspostgresconn.h
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 f30a15c

Please sign in to comment.