Skip to content

Commit

Permalink
Move to maintained jackc/pgx driver (#562)
Browse files Browse the repository at this point in the history 
This patch moves to the maintained `jackc/pgx` driver for connections to PostgreSQL. `lib/pq` is not really maintained any more and has several bugs, which is why `lib/pq` itself recommends using `jackc/pgx` (https://github.com/lib/pq#status).

`jackc/pgx` takes care of all runtime parameters (connect_timeout, sslcert, sslmode, sslkey, ...) for us and does not expose them any more which is why they have been removed from the connection options. This may be seen as a breaking change for some. However, things like `connect_timeout` now actually have an effect as opposed to `lib/pq`.

Also, some defaults have changed which required changing some of the tests.

A CI bug was fixed which referenced the service host (in GitHub Actions) as `${{job.services.postgres.host}}` even though the host is always `127.0.0.1` and no such parameter actually exists in GitHub actions (see https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idservices).

Please note that the `slices` module still uses `lib/pq` to Scan/Value `[]string`.

Closes #559

Signed-off-by: aeneasr <[email protected]>
  • Loading branch information
@aeneasr
aeneasr authored Jun 21, 2020
1 parent 1d26ccb commit ed3097a
Show file tree
Hide file tree
Showing 14 changed files with 178 additions and 270 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ jobs:
- name: Build and run soda
env:
SODA_DIALECT: "postgres"
POSTGRESQL_URL: "postgres://postgres:postgres@${{job.services.postgres.host}}:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
POSTGRESQL_URL: "postgres://postgres:postgres@127.0.0.1:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
run: |
go build -v -tags sqlite -o tsoda ./soda
./tsoda drop -e $SODA_DIALECT -p ./testdata/migrations
Expand All @@ -85,7 +85,7 @@ jobs:
- name: Test
env:
SODA_DIALECT: "postgres"
POSTGRESQL_URL: "postgres://postgres:postgres@${{job.services.postgres.host}}:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
POSTGRESQL_URL: "postgres://postgres:postgres@127.0.0.1:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
run: |
go test -tags sqlite -race ./...
Expand Down
2 changes: 2 additions & 0 deletions SHOULDERS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ Thank you to the following **GIANTS**:

* [github.com/lib/pq](https://godoc.org/github.com/lib/pq)

* [github.com/jackc/pgx](https://godoc.org/github.com/jackc/pgx)

* [github.com/mattn/go-colorable](https://godoc.org/github.com/mattn/go-colorable)

* [github.com/mattn/go-isatty](https://godoc.org/github.com/mattn/go-isatty)
Expand Down
23 changes: 20 additions & 3 deletions dialect_cockroach.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import (
"strings"
"sync"

// Import PostgreSQL driver
_ "github.com/jackc/pgx/v4/stdlib"

"github.com/gobuffalo/fizz"
"github.com/gobuffalo/fizz/translators"
"github.com/gobuffalo/pop/v5/columns"
Expand Down Expand Up @@ -57,7 +60,7 @@ func (p *cockroach) Name() string {
}

func (p *cockroach) DefaultDriver() string {
return namePostgreSQL
return "pgx"
}

func (p *cockroach) Details() *ConnectionDetails {
Expand Down Expand Up @@ -118,7 +121,14 @@ func (p *cockroach) SelectMany(s store, models *Model, query Query) error {
func (p *cockroach) CreateDB() error {
// createdb -h db -p 5432 -U cockroach enterprise_development
deets := p.ConnectionDetails
db, err := sql.Open(deets.Dialect, p.urlWithoutDb())

// Overwrite dialect to match pgx driver for sql.Open
dialect := deets.Dialect
if dialect == "postgres" {
dialect = "pgx"
}

db, err := sql.Open(dialect, p.urlWithoutDb())
if err != nil {
return errors.Wrapf(err, "error creating Cockroach database %s", deets.Database)
}
Expand All @@ -137,7 +147,14 @@ func (p *cockroach) CreateDB() error {

func (p *cockroach) DropDB() error {
deets := p.ConnectionDetails
db, err := sql.Open(deets.Dialect, p.urlWithoutDb())

// Overwrite dialect to match pgx driver for sql.Open
dialect := deets.Dialect
if dialect == "postgres" {
dialect = "pgx"
}

db, err := sql.Open(dialect, p.urlWithoutDb())
if err != nil {
return errors.Wrapf(err, "error dropping Cockroach database %s", deets.Database)
}
Expand Down
194 changes: 40 additions & 154 deletions dialect_postgresql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,20 @@ import (
"fmt"
"io"
"os/exec"
"strings"
"sync"
"unicode"

// Load pgx driver
_ "github.com/jackc/pgx/v4/stdlib"

"github.com/gobuffalo/fizz"
"github.com/gobuffalo/fizz/translators"
"github.com/jackc/pgconn"
"github.com/jmoiron/sqlx"
"github.com/pkg/errors"

"github.com/gobuffalo/pop/v5/columns"
"github.com/gobuffalo/pop/v5/internal/defaults"
"github.com/gobuffalo/pop/v5/logging"
"github.com/jmoiron/sqlx"
pg "github.com/lib/pq"
"github.com/pkg/errors"
)

const namePostgreSQL = "postgres"
Expand All @@ -26,6 +28,7 @@ func init() {
AvailableDialects = append(AvailableDialects, namePostgreSQL)
dialectSynonyms["postgresql"] = namePostgreSQL
dialectSynonyms["pg"] = namePostgreSQL
dialectSynonyms["pgx"] = namePostgreSQL
urlParser[namePostgreSQL] = urlParserPostgreSQL
finalizer[namePostgreSQL] = finalizerPostgreSQL
newConnection[namePostgreSQL] = newPostgreSQL
Expand All @@ -44,7 +47,7 @@ func (p *postgresql) Name() string {
}

func (p *postgresql) DefaultDriver() string {
return namePostgreSQL
return "pgx"
}

func (p *postgresql) Details() *ConnectionDetails {
Expand Down Expand Up @@ -108,7 +111,14 @@ func (p *postgresql) SelectMany(s store, models *Model, query Query) error {
func (p *postgresql) CreateDB() error {
// createdb -h db -p 5432 -U postgres enterprise_development
deets := p.ConnectionDetails
db, err := sql.Open(deets.Dialect, p.urlWithoutDb())

// Overwrite dialect to match pgx driver for sql.Open
dialect := deets.Dialect
if dialect == "postgres" {
dialect = "pgx"
}

db, err := sql.Open(dialect, p.urlWithoutDb())
if err != nil {
return errors.Wrapf(err, "error creating PostgreSQL database %s", deets.Database)
}
Expand All @@ -127,7 +137,14 @@ func (p *postgresql) CreateDB() error {

func (p *postgresql) DropDB() error {
deets := p.ConnectionDetails
db, err := sql.Open(deets.Dialect, p.urlWithoutDb())

// Overwrite dialect to match pgx driver for sql.Open
dialect := deets.Dialect
if dialect == "postgres" {
dialect = "pgx"
}

db, err := sql.Open(dialect, p.urlWithoutDb())
if err != nil {
return errors.Wrapf(err, "error dropping PostgreSQL database %s", deets.Database)
}
Expand Down Expand Up @@ -208,53 +225,36 @@ func newPostgreSQL(deets *ConnectionDetails) (dialect, error) {
return cd, nil
}

// urlParserPostgreSQL parses the options the same way official lib/pg does:
// https://godoc.org/github.com/lib/pq#hdr-Connection_String_Parameters
// urlParserPostgreSQL parses the options the same way jackc/pgconn does:
// https://pkg.go.dev/github.com/jackc/pgconn?tab=doc#ParseConfig
// After parsed, they are set to ConnectionDetails instance
func urlParserPostgreSQL(cd *ConnectionDetails) error {
var err error
name := cd.URL
if strings.HasPrefix(name, "postgres://") || strings.HasPrefix(name, "postgresql://") {
name, err = pg.ParseURL(name)
if err != nil {
return err
}
}

o := make(values)
if err := parseOpts(name, o); err != nil {
conf, err := pgconn.ParseConfig(cd.URL)
if err != nil {
return err
}

if dbname, ok := o["dbname"]; ok {
cd.Database = dbname
}
if host, ok := o["host"]; ok {
cd.Host = host
}
if password, ok := o["password"]; ok {
cd.Password = password
}
if user, ok := o["user"]; ok {
cd.User = user
}
if port, ok := o["port"]; ok {
cd.Port = port
}

options := []string{"sslmode", "fallback_application_name", "connect_timeout", "sslcert", "sslkey", "sslrootcert"}
cd.Database = conf.Database
cd.Host = conf.Host
cd.User = conf.User
cd.Password = conf.Password
cd.Port = fmt.Sprintf("%d", conf.Port)

options := []string{"fallback_application_name"}
for i := range options {
if opt, ok := o[options[i]]; ok {
if opt, ok := conf.RuntimeParams[options[i]]; ok {
cd.Options[options[i]] = opt
}
}

if conf.TLSConfig == nil {
cd.Options["sslmode"] = "disable"
}

return nil
}

func finalizerPostgreSQL(cd *ConnectionDetails) {
cd.Options["sslmode"] = defaults.String(cd.Options["sslmode"], "disable")
cd.Port = defaults.String(cd.Port, portPostgreSQL)
}

Expand All @@ -275,117 +275,3 @@ BEGIN
END LOOP;
END
$func$;`

// Code below is ported from: https://github.com/lib/pq/blob/master/conn.go
type values map[string]string

// scanner implements a tokenizer for libpq-style option strings.
type scanner struct {
s []rune
i int
}

// newScanner returns a new scanner initialized with the option string s.
func newScanner(s string) *scanner {
return &scanner{[]rune(s), 0}
}

// Next returns the next rune.
// It returns 0, false if the end of the text has been reached.
func (s *scanner) Next() (rune, bool) {
if s.i >= len(s.s) {
return 0, false
}
r := s.s[s.i]
s.i++
return r, true
}

// SkipSpaces returns the next non-whitespace rune.
// It returns 0, false if the end of the text has been reached.
func (s *scanner) SkipSpaces() (rune, bool) {
r, ok := s.Next()
for unicode.IsSpace(r) && ok {
r, ok = s.Next()
}
return r, ok
}

// parseOpts parses the options from name and adds them to the values.
//
// The parsing code is based on conninfo_parse from libpq's fe-connect.c
func parseOpts(name string, o values) error {
s := newScanner(name)

for {
var (
keyRunes, valRunes []rune
r rune
ok bool
)

if r, ok = s.SkipSpaces(); !ok {
break
}

// Scan the key
for !unicode.IsSpace(r) && r != '=' {
keyRunes = append(keyRunes, r)
if r, ok = s.Next(); !ok {
break
}
}

// Skip any whitespace if we're not at the = yet
if r != '=' {
r, ok = s.SkipSpaces()
}

// The current character should be =
if r != '=' || !ok {
return fmt.Errorf(`missing "=" after %q in connection info string"`, string(keyRunes))
}

// Skip any whitespace after the =
if r, ok = s.SkipSpaces(); !ok {
// If we reach the end here, the last value is just an empty string as per libpq.
o[string(keyRunes)] = ""
break
}

if r != '\'' {
for !unicode.IsSpace(r) {
if r == '\\' {
if r, ok = s.Next(); !ok {
return fmt.Errorf(`missing character after backslash`)
}
}
valRunes = append(valRunes, r)

if r, ok = s.Next(); !ok {
break
}
}
} else {
quote:
for {
if r, ok = s.Next(); !ok {
return fmt.Errorf(`unterminated quoted string literal in connection string`)
}
switch r {
case '\'':
break quote
case '\\':
r, _ = s.Next()
fallthrough
default:
valRunes = append(valRunes, r)
}
}
}

o[string(keyRunes)] = string(valRunes)
}

return nil
}
Loading

0 comments on commit ed3097a

Please sign in to comment.