Fix Postgresql connection reset not being called

AchilleAsh · AchilleAsh · commit f30a15ccecfd · 2019-02-06T01:07:56.000+01:00
PQreset was never called if the query was made using mConnectionRO from the PostgresProvider, resulting in an always failing state. Fixes #20170
diff --git 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,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 )
@@ -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
@@ -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;
