Revert "psql: fix \connect with URIs and conninfo strings"

This reverts commit fcef1617295c074f2684c887627184d2fc26ac04, about
which both the buildfarm and my local machine are very unhappy.

doc/src/sgml/ref/psql-ref.sgml

@@ -796,31 +796,23 @@ testdb=>
       </varlistentry>
 
       <varlistentry>
-        <term><literal>\c</literal> or <literal>\connect</literal> <literal>[ <replaceable class="parameter">dbname</replaceable> [ <replaceable class="parameter">username</replaceable> ] [ <replaceable class="parameter">host</replaceable> ] [ <replaceable class="parameter">port</replaceable> ] ] | <replaceable class="parameter">conninfo</replaceable> </literal></term>
+        <term><literal>\c</literal> or <literal>\connect</literal> <literal>[ <replaceable class="parameter">dbname</replaceable> [ <replaceable class="parameter">username</replaceable> ] [ <replaceable class="parameter">host</replaceable> ] [ <replaceable class="parameter">port</replaceable> ] ]</literal></term>
         <listitem>
         <para>
         Establishes a new connection to a <productname>PostgreSQL</>
-        server.  The connection parameters to use can be specified either
+        server. If the new connection is successfully made, the
-        using a positional syntax, or using <literal>conninfo</> connection
+        previous connection is closed. If any of <replaceable
-        strings as detailed in <xref linkend="libpq-connstring">.
+        class="parameter">dbname</replaceable>, <replaceable
+        class="parameter">username</replaceable>, <replaceable
+        class="parameter">host</replaceable> or <replaceable
+        class="parameter">port</replaceable> are omitted or specified
+        as <literal>-</literal>, the value of that parameter from the
+        previous connection is used. If there is no previous
+        connection, the <application>libpq</application> default for
+        the parameter's value is used.
         </para>
 
         <para>
-        When using positional parameters, if any of
-        <replaceable class="parameter">dbname</replaceable>,
-        <replaceable class="parameter">username</replaceable>,
-        <replaceable class="parameter">host</replaceable> or
-        <replaceable class="parameter">port</replaceable> are omitted or
-        specified as <literal>-</literal>, the value of that parameter from
-        the previous connection is used; if there is no previous connection,
-        the <application>libpq</application> default for the parameter's value
-        is used.  When using <literal>conninfo</> strings, no values from the
-        previous connection are used for the new connection.
-        </para>
-
-        <para>
-        If the new connection is successfully made, the previous
-        connection is closed.
         If the connection attempt failed (wrong user name, access
         denied, etc.), the previous connection will only be kept if
         <application>psql</application> is in interactive mode. When
@@ -830,16 +822,6 @@ testdb=&gt;
         mechanism that scripts are not accidentally acting on the
         wrong database on the other hand.
         </para>
-
-        <para>
-        Examples:
-        </para>
-<programlisting>
-=&gt; \c mydb myuser host.dom 6432
-=&gt; \c service=foo
-=&gt; \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
-=&gt; \c postgresql://tom@localhost/mydb?application_name=myapp
-</programlisting>
         </listitem>
       </varlistentry>
 

src/bin/psql/command.c

@@ -31,7 +31,6 @@
 #include <sys/stat.h>			/* for stat() */
 #endif
 
-#include "common/connstrings.h"
 #include "portability/instr_time.h"
 
 #include "libpq-fe.h"
@@ -1609,8 +1608,6 @@ do_connect(char *dbname, char *user, char *host, char *port)
 	PGconn	   *o_conn = pset.db,
 			   *n_conn;
 	char	   *password = NULL;
-	bool		keep_password;
-	bool		has_connection_string;
 
 	if (!o_conn && (!dbname || !user || !host || !port))
 	{
@@ -1624,7 +1621,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
 		return false;
 	}
 
-	/* grab values from the old connection, unless supplied by caller */
+	if (!dbname)
+		dbname = PQdb(o_conn);
 	if (!user)
 		user = PQuser(o_conn);
 	if (!host)
@@ -1632,27 +1630,6 @@ do_connect(char *dbname, char *user, char *host, char *port)
 	if (!port)
 		port = PQport(o_conn);
 
-	has_connection_string =
-		dbname ? libpq_connstring_is_recognized(dbname) : false;
-
-	/*
-	 * Any change in the parameters read above makes us discard the password.
-	 * We also discard it if we're to use a conninfo rather than the positional
-	 * syntax.
-	 */
-	keep_password =
-		((strcmp(user, PQuser(o_conn)) == 0) &&
-		 (!host || strcmp(host, PQhost(o_conn)) == 0) &&
-		 (strcmp(port, PQport(o_conn)) == 0) &&
-		 !has_connection_string);
-
-	/*
-	 * Grab dbname from old connection unless supplied by caller.  No password
-	 * discard if this changes: passwords aren't (usually) database-specific.
-	 */
-	if (!dbname)
-		dbname = PQdb(o_conn);
-
 	/*
 	 * If the user asked to be prompted for a password, ask for one now. If
 	 * not, use the password from the old connection, provided the username
@@ -1667,13 +1644,9 @@ do_connect(char *dbname, char *user, char *host, char *port)
 	{
 		password = prompt_for_password(user);
 	}
-	else if (o_conn && keep_password)
+	else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
 	{
-		password = PQpass(o_conn);
+		password = pg_strdup(PQpass(o_conn));
-		if (password && *password)
-			password = pg_strdup(password);
-		else
-			password = NULL;
 	}
 
 	while (true)
@@ -1681,39 +1654,32 @@ do_connect(char *dbname, char *user, char *host, char *port)
 #define PARAMS_ARRAY_SIZE	8
 		const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
 		const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
-		int			paramnum = 0;
 
-		keywords[0] = "dbname";
+		keywords[0] = "host";
-		values[0] = dbname;
+		values[0] = host;
-
+		keywords[1] = "port";
-		if (!has_connection_string)
+		values[1] = port;
-		{
+		keywords[2] = "user";
-			keywords[++paramnum] = "host";
+		values[2] = user;
-			values[paramnum] = host;
+		keywords[3] = "password";
-			keywords[++paramnum] = "port";
+		values[3] = password;
-			values[paramnum] = port;
+		keywords[4] = "dbname";
-			keywords[++paramnum] = "user";
+		values[4] = dbname;
-			values[paramnum] = user;
+		keywords[5] = "fallback_application_name";
-		}
+		values[5] = pset.progname;
-		keywords[++paramnum] = "password";
+		keywords[6] = "client_encoding";
-		values[paramnum] = password;
+		values[6] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
-		keywords[++paramnum] = "fallback_application_name";
+		keywords[7] = NULL;
-		values[paramnum] = pset.progname;
+		values[7] = NULL;
-		keywords[++paramnum] = "client_encoding";
-		values[paramnum] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
-
-		/* add array terminator */
-		keywords[++paramnum] = NULL;
-		values[paramnum] = NULL;
 
 		n_conn = PQconnectdbParams(keywords, values, true);
 
-		pg_free(keywords);
+		free(keywords);
-		pg_free(values);
+		free(values);
 
 		/* We can immediately discard the password -- no longer needed */
 		if (password)
-			pg_free(password);
+			free(password);
 
 		if (PQstatus(n_conn) == CONNECTION_OK)
 			break;

src/bin/psql/help.c

@@ -260,11 +260,11 @@ slashUsage(unsigned short int pager)
 
 	fprintf(output, _("Connection\n"));
 	if (currdb)
-		fprintf(output, _("  \\c[onnect] {[DBNAME|- USER|- HOST|- PORT|-] | conninfo}\n"
+		fprintf(output, _("  \\c[onnect] [DBNAME|- USER|- HOST|- PORT|-]\n"
 						  "                         connect to new database (currently \"%s\")\n"),
 				currdb);
 	else
-		fprintf(output, _("  \\c[onnect] {[DBNAME|- USER|- HOST|- PORT|-] | conninfo}\n"
+		fprintf(output, _("  \\c[onnect] [DBNAME|- USER|- HOST|- PORT|-]\n"
 						  "                         connect to new database (currently no connection)\n"));
 	fprintf(output, _("  \\encoding [ENCODING]   show or set client encoding\n"));
 	fprintf(output, _("  \\password [USERNAME]   securely change the password for a user\n"));

src/bin/psql/tab-complete.c

@@ -52,7 +52,6 @@
 #include "libpq-fe.h"
 #include "pqexpbuffer.h"
 #include "common.h"
-#include "common/connstrings.h"
 #include "settings.h"
 #include "stringutils.h"
 
@@ -3707,18 +3706,10 @@ psql_completion(const char *text, int start, int end)
 
 		COMPLETE_WITH_LIST_CS(my_list);
 	}
-
 	else if (strcmp(prev_wd, "\\connect") == 0 || strcmp(prev_wd, "\\c") == 0)
-	{
+		COMPLETE_WITH_QUERY(Query_for_list_of_databases);
-		if (!libpq_connstring_is_recognized(text))
-			COMPLETE_WITH_QUERY(Query_for_list_of_databases);
-		/* TODO: URI/service completion.  Nothing for now */
-	}
 	else if (strcmp(prev2_wd, "\\connect") == 0 || strcmp(prev2_wd, "\\c") == 0)
-	{
+		COMPLETE_WITH_QUERY(Query_for_list_of_roles);
-		if (!libpq_connstring_is_recognized(prev_wd))
-			COMPLETE_WITH_QUERY(Query_for_list_of_roles);
-	}
 
 	else if (strncmp(prev_wd, "\\da", strlen("\\da")) == 0)
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_aggregates, NULL);

src/common/Makefile

@@ -26,7 +26,7 @@ LIBS += $(PTHREAD_LIBS)
 OBJS_COMMON = exec.o pg_crc.o pg_lzcompress.o pgfnames.o psprintf.o relpath.o \
 	rmtree.o string.o username.o wait_error.o
 
-OBJS_FRONTEND = $(OBJS_COMMON) fe_memutils.o restricted_token.o connstrings.o
+OBJS_FRONTEND = $(OBJS_COMMON) fe_memutils.o restricted_token.o
 
 OBJS_SRV = $(OBJS_COMMON:%.o=%_srv.o)
 

src/common/connstrings.c

@@ -1,53 +0,0 @@
-/*
- * connstrings.c
- *		connecting string processing functions
- *
- *	Copyright (c) 2012-2015, PostgreSQL Global Development Group
- *
- *	src/include/common/connstrings.c
- */
-#include "postgres_fe.h"
-
-#include <string.h>
-
-#include "common/connstrings.h"
-
-
-/* The connection URI must start with either of the following designators: */
-static const char uri_designator[] = "postgresql://";
-static const char short_uri_designator[] = "postgres://";
-
-
-/*
- * Checks if connection string starts with either of the valid URI prefix
- * designators.
- *
- * Returns the URI prefix length, 0 if the string doesn't contain a URI prefix.
- */
-int
-libpq_connstring_uri_prefix_length(const char *connstr)
-{
-	if (strncmp(connstr, uri_designator,
-				sizeof(uri_designator) - 1) == 0)
-		return sizeof(uri_designator) - 1;
-
-	if (strncmp(connstr, short_uri_designator,
-				sizeof(short_uri_designator) - 1) == 0)
-		return sizeof(short_uri_designator) - 1;
-
-	return 0;
-}
-
-/*
- * Recognized connection string either starts with a valid URI prefix or
- * contains a "=" in it.
- *
- * Must be consistent with parse_connection_string: anything for which this
- * returns true should at least look like it's parseable by that routine.
- */
-bool
-libpq_connstring_is_recognized(const char *connstr)
-{
-	return libpq_connstring_uri_prefix_length(connstr) != 0 ||
-		strchr(connstr, '=') != NULL;
-}

src/include/common/connstrings.h

@@ -1,16 +0,0 @@
-/*
- *	connstrings.h
- *		connecting string processing prototypes
- *
- *	Copyright (c) 2012-2015, PostgreSQL Global Development Group
- *
- *	src/include/common/connstrings.h
- */
-#ifndef CONNSTRINGS_H
-#define CONNSTRINGS_H
-
-
-extern int	libpq_connstring_uri_prefix_length(const char *connstr);
-extern bool libpq_connstring_is_recognized(const char *connstr);
-
-#endif   /* CONNSTRINGS_H */

src/interfaces/libpq/fe-connect.c

@@ -22,7 +22,6 @@
 #include <time.h>
 #include <unistd.h>
 
-#include "common/connstrings.h"
 #include "libpq-fe.h"
 #include "libpq-int.h"
 #include "fe-auth.h"
@@ -340,6 +339,8 @@ static void closePGconn(PGconn *conn);
 static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage);
 static PQconninfoOption *parse_connection_string(const char *conninfo,
 						PQExpBuffer errorMessage, bool use_defaults);
+static int	uri_prefix_length(const char *connstr);
+static bool recognized_connection_string(const char *connstr);
 static PQconninfoOption *conninfo_parse(const char *conninfo,
 			   PQExpBuffer errorMessage, bool use_defaults);
 static PQconninfoOption *conninfo_array_parse(const char *const * keywords,
@@ -970,7 +971,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
 	 * If the dbName parameter contains what looks like a connection string,
 	 * parse it into conn struct using connectOptions1.
 	 */
-	if (dbName && libpq_connstring_is_recognized(dbName))
+	if (dbName && recognized_connection_string(dbName))
 	{
 		if (!connectOptions1(conn, dbName))
 			return conn;
@@ -4184,13 +4185,46 @@ parse_connection_string(const char *connstr, PQExpBuffer errorMessage,
 						bool use_defaults)
 {
 	/* Parse as URI if connection string matches URI prefix */
-	if (libpq_connstring_uri_prefix_length(connstr) != 0)
+	if (uri_prefix_length(connstr) != 0)
 		return conninfo_uri_parse(connstr, errorMessage, use_defaults);
 
 	/* Parse as default otherwise */
 	return conninfo_parse(connstr, errorMessage, use_defaults);
 }
 
+/*
+ * Checks if connection string starts with either of the valid URI prefix
+ * designators.
+ *
+ * Returns the URI prefix length, 0 if the string doesn't contain a URI prefix.
+ */
+static int
+uri_prefix_length(const char *connstr)
+{
+	if (strncmp(connstr, uri_designator,
+				sizeof(uri_designator) - 1) == 0)
+		return sizeof(uri_designator) - 1;
+
+	if (strncmp(connstr, short_uri_designator,
+				sizeof(short_uri_designator) - 1) == 0)
+		return sizeof(short_uri_designator) - 1;
+
+	return 0;
+}
+
+/*
+ * Recognized connection string either starts with a valid URI prefix or
+ * contains a "=" in it.
+ *
+ * Must be consistent with parse_connection_string: anything for which this
+ * returns true should at least look like it's parseable by that routine.
+ */
+static bool
+recognized_connection_string(const char *connstr)
+{
+	return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL;
+}
+
 /*
  * Subroutine for parse_connection_string
  *
@@ -4366,7 +4400,7 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
  *
  * If expand_dbname is non-zero, and the value passed for the first occurrence
  * of "dbname" keyword is a connection string (as indicated by
- * libpq_connstring_is_recognized) then parse and process it, overriding any
+ * recognized_connection_string) then parse and process it, overriding any
  * previously processed conflicting keywords. Subsequent keywords will take
  * precedence, however.
  */
@@ -4397,7 +4431,7 @@ conninfo_array_parse(const char *const * keywords, const char *const * values,
 			 * defaults here -- those get picked up later. We only want to
 			 * override for those parameters actually passed.
 			 */
-			if (libpq_connstring_is_recognized(pvalue))
+			if (recognized_connection_string(pvalue))
 			{
 				dbname_options = parse_connection_string(pvalue, errorMessage, false);
 				if (dbname_options == NULL)
@@ -4688,7 +4722,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
 	start = buf;
 
 	/* Skip the URI prefix */
-	prefix_len = libpq_connstring_uri_prefix_length(uri);
+	prefix_len = uri_prefix_length(uri);
 	if (prefix_len == 0)
 	{
 		/* Should never happen */