Instead of sending application_name as a SET command after the connection

is made, include it in the startup-packet options.  This makes it work more
like every other libpq connection option, in particular it now has the same
response to RESET ALL as the rest.  This also saves one network round trip
for new applications using application_name.  The cost is that if the server
is pre-8.5, it'll reject the startup packet altogether, forcing us to retry
the entire connection cycle.  But on balance we shouldn't be optimizing that
case in preference to the behavior with a new server, especially when doing
so creates visible behavioral oddities.  Per discussion.

Author: tglsfdc

src/interfaces/libpq/fe-connect.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.380 2009/11/29 20:14:53 petere Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.381 2009/12/02 04:38:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -83,8 +83,18 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
 #define PGPASSFILE "pgpass.conf"
 #endif
 
-/* fall back options if they are not specified by arguments or defined
-   by environment variables */
+/*
+ * Pre-8.5 servers will return this SQLSTATE if asked to set
+ * application_name in a startup packet.  We hard-wire the value rather
+ * than looking into errcodes.h since it reflects historical behavior
+ * rather than that of the current code.
+ */
+#define ERRCODE_APPNAME_UNKNOWN "42704"
+
+/*
+ * fall back options if they are not specified by arguments or defined
+ * by environment variables
+ */
 #define DefaultHost		"localhost"
 #define DefaultTty		""
 #define DefaultOption	""
@@ -262,7 +272,6 @@ static int parseServiceInfo(PQconninfoOption *options,
 static char *pwdfMatchesString(char *buf, char *token);
 static char *PasswordFromFile(char *hostname, char *port, char *dbname,
 				 char *username);
-static PostgresPollingStatusType pqAppnamePoll(PGconn *conn);
 static void default_threadlock(int acquire);
 
 
@@ -901,6 +910,7 @@ connectDBStart(PGconn *conn)
 	conn->addr_cur = addrs;
 	conn->addrlist_family = hint.ai_family;
 	conn->pversion = PG_PROTOCOL(3, 0);
+	conn->send_appname = true;
 	conn->status = CONNECTION_NEEDED;
 
 	/*
@@ -1075,7 +1085,7 @@ PQconnectPoll(PGconn *conn)
 		case CONNECTION_MADE:
 			break;
 
-			/* pqSetenvPoll/pqAppnamePoll will decide whether to proceed. */
+			/* We allow pqSetenvPoll to decide whether to proceed. */
 		case CONNECTION_SETENV:
 			break;
 
@@ -1880,6 +1890,35 @@ PQconnectPoll(PGconn *conn)
 					if (res->resultStatus != PGRES_FATAL_ERROR)
 						appendPQExpBuffer(&conn->errorMessage,
 										  libpq_gettext("unexpected message from server during startup\n"));
+					else if (conn->send_appname &&
+							 (conn->appname || conn->fbappname))
+					{
+						/*
+						 * If we tried to send application_name, check to see
+						 * if the error is about that --- pre-8.5 servers will
+						 * reject it at this stage of the process.  If so,
+						 * close the connection and retry without sending
+						 * application_name.  We could possibly get a false
+						 * SQLSTATE match here and retry uselessly, but there
+						 * seems no great harm in that; we'll just get the
+						 * same error again if it's unrelated.
+						 */
+						const char *sqlstate;
+
+						sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+						if (sqlstate &&
+							strcmp(sqlstate, ERRCODE_APPNAME_UNKNOWN) == 0)
+						{
+							PQclear(res);
+							conn->send_appname = false;
+							/* Must drop the old connection */
+							pqsecure_close(conn);
+							closesocket(conn->sock);
+							conn->sock = -1;
+							conn->status = CONNECTION_NEEDED;
+							goto keep_going;
+						}
+					}
 
 					/*
 					 * if the resultStatus is FATAL, then conn->errorMessage
@@ -1899,12 +1938,6 @@ PQconnectPoll(PGconn *conn)
 				conn->addrlist = NULL;
 				conn->addr_cur = NULL;
 
-				/*
-				 * Note: To avoid changing the set of application-visible
-				 * connection states, v2 environment setup and v3 application
-				 * name setup both happen in the CONNECTION_SETENV state.
-				 */
-
 				/* Fire up post-connection housekeeping if needed */
 				if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
 				{
@@ -1913,59 +1946,43 @@ PQconnectPoll(PGconn *conn)
 					conn->next_eo = EnvironmentOptions;
 					return PGRES_POLLING_WRITING;
 				}
-				else if (conn->sversion >= 80500 &&
-						 (conn->appname || conn->fbappname))
-				{
-					conn->status = CONNECTION_SETENV;
-					conn->appname_state = APPNAME_STATE_CMD_SEND;
-					return PGRES_POLLING_WRITING;
-				}
 
 				/* Otherwise, we are open for business! */
 				conn->status = CONNECTION_OK;
 				return PGRES_POLLING_OK;
 			}
 
 		case CONNECTION_SETENV:
-			{
-				PostgresPollingStatusType ret;
 
-				/*
-				 * Do post-connection housekeeping (only needed in protocol
-				 * 2.0), or send the application name in PG8.5+.
-				 *
-				 * We pretend that the connection is OK for the duration of
-				 * these queries.
-				 */
-				conn->status = CONNECTION_OK;
-
-				if (PG_PROTOCOL_MAJOR(conn->pversion) < 3)
-					ret = pqSetenvPoll(conn);
-				else				/* must be here to send app name */
-					ret = pqAppnamePoll(conn);
-
-				switch (ret)
-				{
-					case PGRES_POLLING_OK:	/* Success */
-						break;
+			/*
+			 * 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;
 
-					case PGRES_POLLING_READING:		/* Still going */
-						conn->status = CONNECTION_SETENV;
-						return PGRES_POLLING_READING;
+			switch (pqSetenvPoll(conn))
+			{
+				case PGRES_POLLING_OK:	/* Success */
+					break;
 
-					case PGRES_POLLING_WRITING:		/* Still going */
-						conn->status = CONNECTION_SETENV;
-						return PGRES_POLLING_WRITING;
+				case PGRES_POLLING_READING:		/* Still going */
+					conn->status = CONNECTION_SETENV;
+					return PGRES_POLLING_READING;
 
-					default:
-						goto error_return;
-				}
+				case PGRES_POLLING_WRITING:		/* Still going */
+					conn->status = CONNECTION_SETENV;
+					return PGRES_POLLING_WRITING;
 
-				/* We are open for business! */
-				conn->status = CONNECTION_OK;
-				return PGRES_POLLING_OK;
+				default:
+					goto error_return;
 			}
 
+			/* We are open for business! */
+			conn->status = CONNECTION_OK;
+			return PGRES_POLLING_OK;
+
 		default:
 			appendPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("invalid connection state %d, "
@@ -2031,7 +2048,6 @@ makeEmptyPGconn(void)
 	conn->options_valid = false;
 	conn->nonblocking = false;
 	conn->setenv_state = SETENV_STATE_IDLE;
-	conn->appname_state = APPNAME_STATE_IDLE;
 	conn->client_encoding = PG_SQL_ASCII;
 	conn->std_strings = false;	/* unless server says differently */
 	conn->verbosity = PQERRORS_DEFAULT;
@@ -4048,129 +4064,6 @@ pqGetHomeDirectory(char *buf, int bufsize)
 #endif
 }
 
-/*
- *		pqAppnamePoll
- *
- * Polls the process of passing the application name to the backend.
- *
- * Ideally, we'd include the appname in the startup packet, but that would
- * cause old backends to reject the unknown parameter.  So we send it in a
- * separate query after we have determined the backend version.  Once there
- * is no interest in pre-8.5 backends, this should be folded into the startup
- * packet logic.
- */
-static PostgresPollingStatusType
-pqAppnamePoll(PGconn *conn)
-{
-	PGresult   *res;
-
-	if (conn == NULL || conn->status == CONNECTION_BAD)
-		return PGRES_POLLING_FAILED;
-
-	/* Check whether there is any data for us */
-	switch (conn->appname_state)
-	{
-			/* This is a reading state. */
-		case APPNAME_STATE_CMD_WAIT:
-		{
-			/* Load waiting data */
-			int			n = pqReadData(conn);
-
-			if (n < 0)
-				goto error_return;
-			if (n == 0)
-				return PGRES_POLLING_READING;
-
-			break;
-		}
-
-			/* This is a writing state, so we just proceed. */
-		case APPNAME_STATE_CMD_SEND:
-			break;
-
-			/* Should we raise an error if called when not active? */
-		case APPNAME_STATE_IDLE:
-			return PGRES_POLLING_OK;
-
-		default:
-			printfPQExpBuffer(&conn->errorMessage,
-							  libpq_gettext("invalid appname state %d, "
-											"probably indicative of memory corruption\n"),
-							  conn->appname_state);
-			goto error_return;
-	}
-
-	/* We will loop here until there is nothing left to do in this call. */
-	for (;;)
-	{
-		switch (conn->appname_state)
-		{
-			case APPNAME_STATE_CMD_SEND:
-			{
-				const char *val;
-				char	escVal[NAMEDATALEN*2 + 1];
-				char	setQuery[NAMEDATALEN*2 + 26 + 1];
-
-				/* Use appname if present, otherwise use fallback */
-				val = conn->appname ? conn->appname : conn->fbappname;
-
-				/*
-				 * Escape the data as needed.  We can truncate to NAMEDATALEN,
-				 * so there's no need to cope with malloc.
-				 */
-				PQescapeStringConn(conn, escVal, val, NAMEDATALEN, NULL);
-
-				sprintf(setQuery, "SET application_name = '%s'", escVal);
-
-				if (!PQsendQuery(conn, setQuery))
-					goto error_return;
-
-				conn->appname_state = APPNAME_STATE_CMD_WAIT;
-				break;
-			}
-
-			case APPNAME_STATE_CMD_WAIT:
-			{
-				if (PQisBusy(conn))
-					return PGRES_POLLING_READING;
-
-				res = PQgetResult(conn);
-
-				if (res)
-				{
-					if (PQresultStatus(res) != PGRES_COMMAND_OK)
-					{
-						PQclear(res);
-						goto error_return;
-					}
-					PQclear(res);
-					/* Keep reading until PQgetResult returns NULL */
-				}
-				else
-				{
-					/* Query finished, so we're done */
-					conn->appname_state = APPNAME_STATE_IDLE;
-					return PGRES_POLLING_OK;
-				}
-				break;
-			}
-
-			default:
-				printfPQExpBuffer(&conn->errorMessage,
-								  libpq_gettext("invalid appname state %d, "
-												"probably indicative of memory corruption\n"),
-								  conn->appname_state);
-				goto error_return;
-		}
-	}
-
-	/* Unreachable */
-
-error_return:
-	conn->appname_state = APPNAME_STATE_IDLE;
-	return PGRES_POLLING_FAILED;
-}
-
 /*
  * To keep the API consistent, the locking stubs are always provided, even
  * if they are not required.