From aa812716104b3fbd787dae4483341894c9e5ed9a Mon Sep 17 00:00:00 2001
From: Hari Babu <kommi.haribabu@gmail.com>
Date: Thu, 21 Feb 2019 23:11:55 +1100
Subject: [PATCH 1/8] Restructure the code to remove duplicate code

The duplicate code logic of checking for the server version
before issuing the SHOW transaction_readonly to find out whether
the server is read-write or not is restructured under a new
connection status, so that duplicate code is removed. This is
required for the next set of patches
---
 doc/src/sgml/libpq.sgml           | 26 +++++---
 src/interfaces/libpq/fe-connect.c | 99 ++++++++++++-------------------
 src/interfaces/libpq/libpq-fe.h   |  3 +-
 3 files changed, 57 insertions(+), 71 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index c1d1b6b2db..6221e359f0 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1576,17 +1576,27 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
      <varlistentry id="libpq-connect-target-session-attrs" xreflabel="target_session_attrs">
       <term><literal>target_session_attrs</literal></term>
       <listitem>
+       <para>
+        The supported options for this parameter are, <literal>any</literal> and
+        <literal>read-write</literal>. The default value of this parameter,
+        <literal>any</literal>, regards all connections as acceptable.
+        If multiple hosts were specified in the connection string, based on the
+        specified value, any remaining servers will be tried before confirming
+        succesful connection or failure.
+       </para>
+
        <para>
         If this parameter is set to <literal>read-write</literal>, only a
         connection in which read-write transactions are accepted by default
-        is considered acceptable.  The query
-        <literal>SHOW transaction_read_only</literal> will be sent upon any
-        successful connection; if it returns <literal>on</literal>, the connection
-        will be closed.  If multiple hosts were specified in the connection
-        string, any remaining servers will be tried just as if the connection
-        attempt had failed.  The default value of this parameter,
-        <literal>any</literal>, regards all connections as acceptable.
-      </para>
+        is considered acceptable.
+       </para>
+
+       <para>
+        To find out whether the server supports read-write transactions are not,
+        query <literal>SHOW transaction_read_only</literal> will be sent upon any
+        successful connection; if it returns <literal>on</literal>, it means server
+        doesn't support read-write transactions.
+       </para>
       </listitem>
     </varlistentry>
     </variablelist>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e3bf6a7449..c68448786d 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3188,6 +3188,43 @@ keep_going:						/* We will come back to here until there is
 					return PGRES_POLLING_WRITING;
 				}
 
+				conn->status = CONNECTION_CHECK_TARGET;
+				goto keep_going;
+			}
+
+		case CONNECTION_SETENV:
+			{
+
+				/*
+				 * Do post-connection housekeeping (only needed in protocol 2.0).
+				 *
+				 * We pretend that the connection is OK for the duration of these
+				 * queries.
+				 */
+				conn->status = CONNECTION_OK;
+
+				switch (pqSetenvPoll(conn))
+				{
+					case PGRES_POLLING_OK:	/* Success */
+						break;
+
+					case PGRES_POLLING_READING: /* Still going */
+						conn->status = CONNECTION_SETENV;
+						return PGRES_POLLING_READING;
+
+					case PGRES_POLLING_WRITING: /* Still going */
+						conn->status = CONNECTION_SETENV;
+						return PGRES_POLLING_WRITING;
+
+					default:
+						goto error_return;
+				}
+			}
+
+			/* Intentional fall through */
+
+		case CONNECTION_CHECK_TARGET:
+			{
 				/*
 				 * If a read-write connection is required, see if we have one.
 				 *
@@ -3229,68 +3266,6 @@ keep_going:						/* We will come back to here until there is
 				return PGRES_POLLING_OK;
 			}
 
-		case CONNECTION_SETENV:
-
-			/*
-			 * Do post-connection housekeeping (only needed in protocol 2.0).
-			 *
-			 * We pretend that the connection is OK for the duration of these
-			 * queries.
-			 */
-			conn->status = CONNECTION_OK;
-
-			switch (pqSetenvPoll(conn))
-			{
-				case PGRES_POLLING_OK:	/* Success */
-					break;
-
-				case PGRES_POLLING_READING: /* Still going */
-					conn->status = CONNECTION_SETENV;
-					return PGRES_POLLING_READING;
-
-				case PGRES_POLLING_WRITING: /* Still going */
-					conn->status = CONNECTION_SETENV;
-					return PGRES_POLLING_WRITING;
-
-				default:
-					goto error_return;
-			}
-
-			/*
-			 * If a read-write connection is required, see if we have one.
-			 * (This should match the stanza in the CONNECTION_AUTH_OK case
-			 * above.)
-			 *
-			 * Servers before 7.4 lack the transaction_read_only GUC, but by
-			 * the same token they don't have any read-only mode, so we may
-			 * just skip the test in that case.
-			 */
-			if (conn->sversion >= 70400 &&
-				conn->target_session_attrs != NULL &&
-				strcmp(conn->target_session_attrs, "read-write") == 0)
-			{
-				if (!saveErrorMessage(conn, &savedMessage))
-					goto error_return;
-
-				conn->status = CONNECTION_OK;
-				if (!PQsendQuery(conn,
-								 "SHOW transaction_read_only"))
-				{
-					restoreErrorMessage(conn, &savedMessage);
-					goto error_return;
-				}
-				conn->status = CONNECTION_CHECK_WRITABLE;
-				restoreErrorMessage(conn, &savedMessage);
-				return PGRES_POLLING_READING;
-			}
-
-			/* We can release the address list now. */
-			release_conn_addrinfo(conn);
-
-			/* We are open for business! */
-			conn->status = CONNECTION_OK;
-			return PGRES_POLLING_OK;
-
 		case CONNECTION_CONSUME:
 			{
 				conn->status = CONNECTION_OK;
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 97bc98b1f3..50cfe266c6 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -65,8 +65,9 @@ typedef enum
 	CONNECTION_NEEDED,			/* Internal state: connect() needed */
 	CONNECTION_CHECK_WRITABLE,	/* Check if we could make a writable
 								 * connection. */
-	CONNECTION_CONSUME			/* Wait for any pending message and consume
+	CONNECTION_CONSUME,			/* Wait for any pending message and consume
 								 * them. */
+	CONNECTION_CHECK_TARGET		/* Check if we have a proper target connection */
 } ConnStatusType;
 
 typedef enum
-- 
2.20.1.windows.1

