Pavel Stehule wrote:
> 2015-02-20 8:22 GMT+01:00 David Fetter <da...@fetter.org>:
>
> > On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote:
> > > Hi
> > >
> > > I am happy with doc changes now.
> > >
> > > When I test last patch, I found sigfault bug, because host =
> > > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007
> > >
> > > If you are agree with fix, I'll mark this patch as ready for commit.
> >
> > Thanks for fixing the bug. Let's go with this.
> >
>
> marked as "ready for commit"
Gave this patch a look. In general it looks pretty good, but there is
one troublesome point: it duplicates two functions from libpq into psql,
including the URI designators. This doesn't look very nice. I thought
about just creating a new src/common (say connstring.c) to host those
two functions and the URI designators, but then on closer look I noticed
that libpq's facilities for URI parsing become severed: two very small
functions become part of libpgcommon, while the more complex parts
remain in libpq.
On the other hand, if we see that psql needs this functionality, isn't
this a clue that other client programs might find it useful too?
(Honestly I'm not completely sure about this point -- other opinions?)
I see three[four] ways forward from here:
1. export this functionality in libpq as one or two new functions. This
would need proper docs, exports.txt, etc.
2. export it in libpgcommon. If we choose this option we should
probably rename those functions, as in the attached patch.
3. accept the patch as is, i.e. duplicate the libq-internal functions in
psql.
[4. reject the whole thing]
I lean towards (2) myself, but what do others think?
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..173b6c3 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,21 @@ 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 [ { [ <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 } ] </literal></term>
+ <term><literal>\connect [ { [ <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 } ] </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>
@@ -822,6 +825,22 @@ testdb=>
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
</para>
+
+ <para>
+ Positional syntax:
+<programlisting>
+=> \c mydb myuser host.dom 6432
+</programlisting>
+ </para>
+
+ <para>
+ The conninfo form detailed in <xref linkend="LIBPQ-CONNSTRING"> takes two forms: keyword-value pairs, and URIs:
+ </para>
+<programlisting>
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+</programlisting>
</listitem>
</varlistentry>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4201956..64210bf 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -31,6 +31,7 @@
#include <sys/stat.h> /* for stat() */
#endif
+#include "common/connstrings.h"
#include "portability/instr_time.h"
#include "libpq-fe.h"
@@ -1608,6 +1609,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))
{
@@ -1621,15 +1624,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 = libpq_connstring_is_recognized(dbname);
+
+ keep_password =
+ ((strcmp(user, PQuser(o_conn)) == 0) &&
+ (!host || 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
@@ -1644,9 +1663,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)
@@ -1654,23 +1677,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));
-
- 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;
+ int paramnum = 0;
+
+ 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/tab-complete.c b/src/bin/psql/tab-complete.c
index 5ddd654..7f0be95 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -52,6 +52,7 @@
#include "libpq-fe.h"
#include "pqexpbuffer.h"
#include "common.h"
+#include "common/connstrings.h"
#include "settings.h"
#include "stringutils.h"
@@ -69,6 +70,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
@@ -3714,10 +3719,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 (libpq_connstring_is_recognized(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 (libpq_connstring_is_recognized(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);
diff --git a/src/common/Makefile b/src/common/Makefile
index 372a21b..d1338c1 100644
--- a/src/common/Makefile
+++ b/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
+OBJS_FRONTEND = $(OBJS_COMMON) fe_memutils.o connstrings.o
OBJS_SRV = $(OBJS_COMMON:%.o=%_srv.o)
diff --git a/src/common/connstrings.c b/src/common/connstrings.c
new file mode 100644
index 0000000..aa5fdb0
--- /dev/null
+++ b/src/common/connstrings.c
@@ -0,0 +1,51 @@
+/*
+ * connstrings.c
+ * connecting string related 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 the given 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 libpq's 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;
+}
diff --git a/src/include/common/connstrings.h b/src/include/common/connstrings.h
new file mode 100644
index 0000000..1fe624e
--- /dev/null
+++ b/src/include/common/connstrings.h
@@ -0,0 +1,16 @@
+/*
+ * connstrings.h
+ * connecting string related 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 */
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 25961b1..e5407dd 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -22,6 +22,7 @@
#include <time.h>
#include <unistd.h>
+#include "common/connstrings.h"
#include "libpq-fe.h"
#include "libpq-int.h"
#include "fe-auth.h"
@@ -339,8 +340,6 @@ 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,
@@ -971,7 +970,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 && recognized_connection_string(dbName))
+ if (dbName && libpq_connstring_is_recognized(dbName))
{
if (!connectOptions1(conn, dbName))
return conn;
@@ -4179,13 +4178,15 @@ conninfo_init(PQExpBuffer errorMessage)
*
* If use_defaults is TRUE, default values are filled in (from a service file,
* environment variables, etc).
+ *
+ * Make sure to keep libpq_connstring_is_recognized in sync with this.
*/
static PQconninfoOption *
parse_connection_string(const char *connstr, PQExpBuffer errorMessage,
bool use_defaults)
{
/* Parse as URI if connection string matches URI prefix */
- if (uri_prefix_length(connstr) != 0)
+ if (libpq_connstring_uri_prefix_length(connstr) != 0)
return conninfo_uri_parse(connstr, errorMessage, use_defaults);
/* Parse as default otherwise */
@@ -4193,39 +4194,6 @@ parse_connection_string(const char *connstr, PQExpBuffer errorMessage,
}
/*
- * 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
*
* Deal with a string containing key=value pairs.
@@ -4400,7 +4368,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
- * recognized_connection_string) then parse and process it, overriding any
+ * libpq_connstring_is_recognized) then parse and process it, overriding any
* previously processed conflicting keywords. Subsequent keywords will take
* precedence, however.
*/
@@ -4431,7 +4399,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 (recognized_connection_string(pvalue))
+ if (libpq_connstring_is_recognized(pvalue))
{
dbname_options = parse_connection_string(pvalue, errorMessage, false);
if (dbname_options == NULL)
@@ -4722,7 +4690,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
start = buf;
/* Skip the URI prefix */
- prefix_len = uri_prefix_length(uri);
+ prefix_len = libpq_connstring_uri_prefix_length(uri);
if (prefix_len == 0)
{
/* Should never happen */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
