Skip to content

Commit

Permalink
Support run-time parameters in connection strings
Browse files Browse the repository at this point in the history 
Commit 423884a made it superficially look like the
startup message sent by pq included more parameters than just
"user" and "database", and commit 6c7918f
subsequently fell into the trap and attempted to introduce support
for a number of environment variables.

Fix the startup procedure to actually send the parameters to the
server backend, thus adding support for a number of environment
variables as well as connection string parameters.  This behaviour
is slightly different from libpq, which only allows a specific
subset of run-time parameters to be set directly in the connection
string, requiring the rest to be set in the "options" parameter
using a clumsy syntax.  libpq's behaviour was deemed inconsistent,
and therefore was not faithfully reproduced.

This change is not fully backwards compatible as the previous
behaviour was to silently ignore invalid parameters in the
connection string.  These applications will fail with ErrBadConn
when attempting to operate on the sql.DB handle.
  • Loading branch information
@johto
johto committed Oct 13, 2013
1 parent 67cca66 commit 67337af
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 53 deletions.
27 changes: 22 additions & 5 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,26 @@

**Connection String Parameters**

These are a subset of the libpq connection parameters. In addition, a
number of the [environment
variables](http://www.postgresql.org/docs/current/static/libpq-envars.html)
supported by libpq are also supported. Just like libpq, these have
lower precedence than explicitly provided connection parameters.
Similarly to libpq, when establishing a connection using pq you are expected to
supply a [connection string](http://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNSTRING)
containing zero or more parameters. A subset of the connection parameters
supported by libpq are also supported by pq. Additionally, pq also lets you
specify run-time parameters (such as `search_path` or `work_mem`) directly in
the connection string. This is different from libpq, which does not allow
run-time parameters in the connection string, instead requiring you to supply
them in the `options` parameter.

Most [environment variables](http://www.postgresql.org/docs/current/static/libpq-envars.html)
supported by libpq are also supported by pq. If any of the environment
variables not supported by pq are set, pq will panic during connection
establishment. Environment variables have a lower precedence than explicitly
provided connection parameters.

See http://www.postgresql.org/docs/current/static/libpq-connect.html.

For compatibility with libpq, the following special connection parameters are
supported:

* `dbname` - The name of the database to connect to
* `user` - The user to sign in as
* `password` - The user's password
Expand All @@ -49,6 +61,11 @@ Use single quotes for values that contain whitespace:

"user=pqgotest password='with spaces'"

In addition to the parameters listed above, any run-time parameter that can be
set at backend start time can be set in the connection string. For more
information, see
http://www.postgresql.org/docs/current/static/runtime-config.html.

See http://golang.org/pkg/database/sql to learn how to use with `pq` through the `database/sql` package.

## Tests
Expand Down
84 changes: 54 additions & 30 deletions conn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,28 @@ func Open(name string) (_ driver.Conn, err error) {
return nil, err
}

// We can't work with any client_encoding other than UTF-8 currently.
// However, we have historically allowed the user to set it to UTF-8
// explicitly, and there's no reason to break such programs, so allow that.
// Note that the "options" setting could also set client_encoding, but
// parsing its value is not worth it. Instead, we always explicitly send
// client_encoding as a separate run-time parameter, which should override
// anything set in options.
if encoding := o.Get("client_encoding"); encoding != "" {
mustBeUtf8(encoding)
} else {
o.Set("client_encoding", "UTF8")
}
// DateStyle needs a similar treatment.
if datestyle := o.Get("datestyle"); datestyle != "" {
if datestyle != "ISO, MDY" {
panic(fmt.Sprintf("setting datestyle must be absent or %v; got %v",
"ISO, MDY", datestyle))
}
} else {
o.Set("datestyle", "ISO, MDY")
}

// If a user is not provided by any other means, the last
// resort is to use the current operating system provided user
// name.
Expand Down Expand Up @@ -560,10 +582,24 @@ func (cn *conn) ssl(o values) {
func (cn *conn) startup(o values) {
w := cn.writeBuf(0)
w.int32(196608)
w.string("user")
w.string(o.Get("user"))
w.string("database")
w.string(o.Get("dbname"))
// Send the backend the name of the database we want to connect to, and the
// user we want to connect as. Additionally, we send over any run-time
// parameters potentially included in the connection string. If the server
// doesn't recognize any of them, it will reply with an error.
for k, v := range o {
// skip options which can't be run-time parameters
if k == "password" || k == "host" ||
k == "port" || k == "sslmode" {
continue
}
// The protocol requires us to supply the database name as "database"
// instead of "dbname".
if k == "dbname" {
k = "database"
}
w.string(k)
w.string(v)
}
w.string("")
cn.send(w)

Expand Down Expand Up @@ -857,12 +893,6 @@ func parseEnviron(env []string) (out map[string]string) {
unsupported := func() {
panic(fmt.Sprintf("setting %v not supported", parts[0]))
}
mustBe := func(expected string) {
if parts[1] != expected {
panic(fmt.Sprintf("setting %v must be absent or %v; got %v",
parts[0], expected, parts[1]))
}
}

// The order of these is the same as is seen in the
// PostgreSQL 9.1 manual. Unsupported but well-defined
Expand All @@ -874,7 +904,7 @@ func parseEnviron(env []string) (out map[string]string) {
case "PGHOST":
accrue("host")
case "PGHOSTADDR":
accrue("hostaddr")
unsupported()
case "PGPORT":
accrue("port")
case "PGDATABASE":
Expand All @@ -891,29 +921,23 @@ func parseEnviron(env []string) (out map[string]string) {
accrue("application_name")
case "PGSSLMODE":
accrue("sslmode")
case "PGREQUIRESSL":
accrue("requiressl")
case "PGSSLCERT":
accrue("sslcert")
case "PGSSLKEY":
accrue("sslkey")
case "PGSSLROOTCERT":
accrue("sslrootcert")
case "PGSSLCRL":
accrue("sslcrl")
case "PGREQUIRESSL", "PGSSLCERT", "PGSSLKEY", "PGSSLROOTCERT", "PGSSLCRL":
unsupported()
case "PGREQUIREPEER":
accrue("requirepeer")
case "PGKRBSRVNAME":
accrue("krbsrvname")
case "PGGSSLIB":
accrue("gsslib")
unsupported()
case "PGKRBSRVNAME", "PGGSSLIB":
unsupported()
case "PGCONNECT_TIMEOUT":
accrue("connect_timeout")
unsupported()
case "PGCLIENTENCODING":
mustBeUtf8(parts[1])
accrue("client_encoding")
case "PGDATESTYLE":
mustBe("ISO, MDY")
case "PGTZ", "PGGEQO", "PGSYSCONFDIR", "PGLOCALEDIR":
accrue("datestyle")
case "PGTZ":
accrue("timezone")
case "PGGEQO":
accrue("geqo")
case "PGSYSCONFDIR", "PGLOCALEDIR":
unsupported()
}
}
Expand Down
86 changes: 68 additions & 18 deletions conn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,6 @@ func TestReturning(t *testing.T) {

var envParseTests = []struct {
Expected map[string]string
ExpectPanic bool
Env []string
}{
{
Expand All @@ -521,28 +520,13 @@ var envParseTests = []struct {
},
{
Env: []string{"PGDATESTYLE=ISO, MDY"},
Expected: map[string]string{},
},
{
Env: []string{"PGDATESTYLE=ISO, YMD"},
ExpectPanic: true,
Expected: map[string]string{"datestyle": "ISO, MDY"},
},
}

func TestParseEnviron(t *testing.T) {
for i, tt := range envParseTests {
tryParse := func(env []string) (result map[string]string, panicked bool) {
defer func() {
if p := recover(); p != nil {
panicked = true
}
}()
return parseEnviron(env), false
}
results, gotPanic := tryParse(tt.Env)
if gotPanic != tt.ExpectPanic {
t.Errorf("%d: Expected panic: %#v Got: %#v", i, tt.ExpectPanic, gotPanic)
}
results := parseEnviron(tt.Env)
if !reflect.DeepEqual(tt.Expected, results) {
t.Errorf("%d: Expected: %#v Got: %#v", i, tt.Expected, results)
}
Expand Down Expand Up @@ -711,6 +695,72 @@ func TestParseOpts(t *testing.T) {
}
}

func TestRuntimeParameters(t *testing.T) {
type RuntimeTestResult int
const (
ResultBadConn RuntimeTestResult = iota
ResultPanic
ResultSuccess
)

tests := []struct {
conninfo string
param string
expected string
expectedOutcome RuntimeTestResult
}{
// invalid parameter
{"DOESNOTEXIST=foo", "", "", ResultBadConn},
// we can only work with a specific value for these two
{"client_encoding=SQL_ASCII", "", "", ResultPanic},
{"datestyle='ISO, YDM'", "", "", ResultPanic},
// "options" should work exactly as it does in libpq
{"options='-c search_path=pqgotest'", "search_path", "pqgotest", ResultSuccess},
// pq should override client_encoding in this case
{"options='-c client_encoding=SQL_ASCII'", "client_encoding", "UTF8", ResultSuccess},
// allow client_encoding to be set explicitly
{"client_encoding=UTF-8", "client_encoding", "UTF8", ResultSuccess},
// test a runtime parameter not supported by libpq
{"work_mem='139kB'", "work_mem", "139kB", ResultSuccess},
};


for _, test := range tests {
db, err := openTestConnConninfo(test.conninfo)
if err != nil {
t.Fatal(err)
}
defer db.Close()

tryGetParameterValue := func() (value string, outcome RuntimeTestResult) {
defer func() {
if p := recover(); p != nil {
outcome = ResultPanic
}
}()
row := db.QueryRow("SELECT current_setting($1)", test.param)
err = row.Scan(&value)
if err == driver.ErrBadConn {
return "", ResultBadConn
} else if err != nil {
t.Fatalf("unexpected error %v", err)
}
return value, ResultSuccess
}

value, outcome := tryGetParameterValue()
if outcome != test.expectedOutcome {
t.Fatalf("unexpected outcome %v (was expecting %v) for conninfo \"%s\"",
outcome, test.expectedOutcome, test.conninfo)
}
if value != test.expected {
t.Fatalf("bad value for %s: got %s, want %s with conninfo \"%s\"",
test.param, value, test.expected, test.conninfo)
}
}
}


var utf8tests = []struct {
Value string
Valid bool
Expand Down

0 comments on commit 67337af

Please sign in to comment.