Pass URI directly to PQconnectdb

[levkk]

Fixes #387

[larskanis]

Unfortunately all tests fail. Could you please fix this?

[levkk]

Unfortunately all tests fail. Could you please fix this?

This is always how I like to start my PRs...tells people I mean business ;) Let me try and fix this mess I made...

[levkk]

Local tests pass 👍 I excluded the test that allowed the user to override user, password, and other URI-specific parameters when a URI and an options hash is passed in because that would force the library to parse the URI and will throw the bad URI error if multiple database hosts are used. This is a slight behavior change, I would expect this to be somewhat breaking to some apps that use this strange feature.

[cbandy]

As I recall, URI and Hash are accepted together so the caller can set/override parameters like sslmode or connect_timeout without knowing the content or format of the connection string. It doesn't look like this behavior was documented outside of tests.

Should the caller have to think differently about user, dbname, etc?

[cbandy]

I think it's better to have uri either always be URI or always be String. We don't use the structure of URI later on, so converting to String seems appropriate here.

uri = String(args.first)

[cbandy]

It looks like ? does not definitely indicate there is a query.

> URI('postgres://u?n:pw@localhost')              
=> #<URI::Generic postgres://u?n:pw@localhost>

> URI('postgres://un:p?w@localhost')
Traceback (most recent call last):
URI::InvalidURIError (bad URI(is not URI?): "postgres://un:p?w@localhost")

[levkk]

That is interesting...and unfortunate. At the same time, I can't parse the URI that may have two hosts or more because that's definitely not valid. Hmm.

[larskanis]

I overhauled the connection string parser to support URIs with multiple hosts in 5e8c911 . It takes a tradeoff by not modifying the parts of the URI, that were given to the API call, but extends it with additional parameters like hostaddr and fallback_application_name.