On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> Hi all
>
> I am sending a review of this patch:
>
> * What it does? - Allow to connect to other db by \connect uri connection
> format
>
> postgres=# \c postgresql://localhost?service=old
> psql (9.5devel, server 9.2.9)
> You are now connected to database "postgres" as user "pavel".
>
> * Would we this feature? - yes, it eliminate inconsistency between cmd line
> connect and \connect. It is good idea without any objections.
>
> * This patch is cleanly applicable, later compilation without any issues
>
> * All regress tests passed
>
> * A psql documentation is updated -- this feature (and format) is not
> widely known, so maybe some more examples are welcome
>
> * When I tested this feature, it worked as expected
>
> * Code respects PostgreSQL coding rules. I prefer a little bit different
> test if keep password. Current code is little bit harder to understand. But
> I can live with David's code well too.
>
> if
> (!user)
>
> user = PQuser(o_conn);
>
> if
> (!host)
>
> host =
> PQhost(o_conn);
>
>
> if
> (!port)
>
> port =
> PQport(o_conn);
>
>
> if
> (dbname)
>
> has_connection_string =
> recognized_connection_string(dbname);
>
>
> /* we should not to keep password if some connection property is changed
> */
>
>
> keep_password = strcmp(user, PQuser(o_conn)) == 0 && strcmp(host,
> PQhost(o_conn)) == 0
> && strcmp(port, PQport(o_conn)) == 0 &&
> !has_connection_string;
Changed. This is cleaner.
> I have not any other comments.
>
> Possible questions:
> 1. more examples in doc
I'm not sure how best to illustrate those. Are you thinking of one
example each for the URI and conninfo cases?
> 2. small change how to check keep_password
Done.
Cheers,
David.
--
David Fetter <da...@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..cae6bf4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ 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> ] ]</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> ] ] | <replaceable
class="parameter">conninfo</replaceable> string | <replaceable
class="parameter">URI</replaceable> } ] </literal></term>
<listitem>
<para>
Establishes a new connection to a <productname>PostgreSQL</>
- server. If the new connection is successfully made, the
- previous connection is closed. If any of <replaceable
+ server using positional parameters as described below, a
+ <parameter>conninfo</parameter> string, or a <acronym>URI</acronym>.
If the new connection is
+ successfully made, the
+ previous connection is closed. 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
+ previous connection is used. If using positional parameters and there
is no previous
connection, the <application>libpq</application> default for
the parameter's value is used.
</para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..ec86e52 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ bool keep_password = true;
+ bool has_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ keep_password = (
+ (strcmp(user, PQuser(o_conn)) == 0) &&
+ (strcmp(host, PQhost(o_conn)) == 0) &&
+ (strcmp(port, PQport(o_conn)) == 0) &&
+ !has_connection_string);
+
+ /*
+ * Unlike the previous stanzas, changing only the dbname shouldn't
+ * trigger throwing away the password.
+ */
+ 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
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char
*port)
{
password = prompt_for_password(user);
}
- else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+ else if (o_conn && keep_password)
{
- password = pg_strdup(PQpass(o_conn));
+ password = PQpass(o_conn);
+ if (password && *password)
+ password = pg_strdup(password);
+ else
+ password = NULL;
}
while (true)
@@ -1653,23 +1675,28 @@ 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] = "host";
- values[0] = host;
- keywords[1] = "port";
- values[1] = port;
- keywords[2] = "user";
- values[2] = user;
- keywords[3] = "password";
- values[3] = password;
- keywords[4] = "dbname";
- values[4] = dbname;
- keywords[5] = "fallback_application_name";
- values[5] = pset.progname;
- keywords[6] = "client_encoding";
- values[6] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL :
"auto";
- keywords[7] = NULL;
- values[7] = NULL;
+ keywords[0] = "dbname";
+ values[0] = dbname;
+
+ if (!has_connection_string)
+ {
+ keywords[++paramnum] = "host";
+ values[paramnum] = host;
+ keywords[++paramnum] = "port";
+ values[paramnum] = port;
+ keywords[++paramnum] = "user";
+ values[paramnum] = user;
+ }
+ keywords[++paramnum] = "password";
+ values[paramnum] = password;
+ keywords[++paramnum] = "fallback_application_name";
+ values[paramnum] = pset.progname;
+ keywords[++paramnum] = "client_encoding";
+ values[paramnum] = (pset.notty || getenv("PGCLIENTENCODING")) ?
NULL : "auto";
+ keywords[++paramnum] = NULL;
+ values[paramnum] = NULL;
n_conn = PQconnectdbParams(keywords, values, true);
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 275bdcc..e036332 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1771,6 +1771,42 @@ session_username(void)
return PQuser(pset.db);
}
+/*
+ * 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.
+ *
+ * XXX copied from libpq/fe-connect.c
+ */
+static int
+uri_prefix_length(const char *connstr)
+{
+ const char uri_designator[] = "postgresql://";
+ const char short_uri_designator[] = "postgres://";
+ 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.
+ *
+ * XXX copied from libpq/fe-connect.c
+ */
+bool
+recognized_connection_string(const char *connstr)
+{
+ return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL;
+}
+
/* expand_tilde
*
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index eb14d1c..079da43 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -49,4 +49,6 @@ extern const char *session_username(void);
extern void expand_tilde(char **filename);
+extern bool recognized_connection_string(const char *connstr);
+
#endif /* COMMON_H */
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index e39a07c..1c57924 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -69,6 +69,10 @@ extern char *filename_completion_function();
/* word break characters */
#define WORD_BREAKS "\t\n@$><=;|&{() "
+/* XXX Until we can tab complete for URIs and services, each of which
+ * should populate their own lists */
+static const char *const empty_list[] = {NULL};
+
/*
* This struct is used to define "schema queries", which are custom-built
* to obtain possibly-schema-qualified names of database objects. There is
@@ -3706,10 +3710,22 @@ 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);
+ {
+ /* URI/service completion. Nothing for now */
+ if (recognized_connection_string(text))
+ COMPLETE_WITH_LIST(empty_list);
+ else
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+ }
else if (strcmp(prev2_wd, "\\connect") == 0 || strcmp(prev2_wd, "\\c")
== 0)
- COMPLETE_WITH_QUERY(Query_for_list_of_roles);
+ {
+ if (recognized_connection_string(prev_wd))
+ COMPLETE_WITH_LIST(empty_list);
+ else
+ 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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
