Re: WIP: URI connection string support for libpq - Mailing list pgsql-hackers

On 12/13/2011 05:45 PM, Alexander Shulgin wrote:
> Before that, why don't also accept "psql://", "pgsql://", "postgre://"
> and anything else? Or wait, aren't we adding to the soup again (or
> rather putting the soup right into libpq?)

There are multiple URI samples within PostgreSQL drivers in the field,
here are two I know of what I believe to be a larger number of samples
that all match in this regard:

http://sequel.rubyforge.org/rdoc/files/doc/opening_databases_rdoc.html
http://www.rmunn.com/sqlalchemy-tutorial/tutorial.html

These two are using "postgres".  One of the hopes in adding URI support
was to make it possible for the libpq spec to look similar to the ones
already floating around, so that they'd all converge.  Using a different
prefix than the most popular ones have already adopted isn't a good way
to start that.  Now, whenever the URI discussion wanders off into
copying the JDBC driver I wonder again why that's relevant.  But making
the implementation look like what people have already deployed surely
is, particularly if there's no downside to doing that.

Initial quick review of your patch:  you suggested this as the general form:

psql -d postgresql://user@pw:host:port/dbname?param1=value1¶m2=value2...

That's presumably supposed to be:

psql -d postgresql://user:pw@host:port/dbname?param1=value1¶m2=value2...

This variation worked here:

$ psql -d postgresql://gsmith@localhost:5432/gsmith

If we had to pick one URI prefix, it should be "postgres".  But given
the general name dysfunction around this project, I can't see how anyone
would complain if we squat on "postgresql" too.  Attached patch modifies
yours to prove we can trivially support both, in hopes of detonating
this argument before it rages on further.  Tested like this:

$ psql -d postgres://gsmith@localhost:5432/gsmith

And that works too now.  I doubt either of us like what I did to the
handoff between conninfo_uri_parse and conninfo_uri_parse_options to
achieve that, but this feature is still young.

After this bit of tinkering with the code, it feels to me like this
really wants a split() function to break out the two sides of a string
across a delimiter, eating it in the process.  Adding the level of
paranoia I'd like around every bit of code I see that does that type of
operation right now would take a while.  Refactoring in terms of split
and perhaps a few similarly higher-level string parsing operations,
targeted for this job, might make it easier to focus on fortifying those
library routines instead.  For example, instead of the gunk I just added
that moves past either type of protocol prefix, I'd like to just say
"split(buf,"://",&left,&right) and then move on with processing the
right side.

I agree with your comment that we need to add some sort of regression
tests for this.  Given how the parsing is done right now, we'd want to
come up with some interesting invalid strings too.  Making sure this
fails gracefully (and not in a buffer overflow way) might even use
something like fuzz testing too.  Around here we've just been building
some Python scripting to do that sort of thing, tests that aren't
practical to do with pg_regress.  Probably be better from the project's
perspective if such things were in Perl instead; so far no one has ever
paid me enough to stomach writing non-trivial things in Perl.  Perhaps
you are more diverse.

--
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us