Re: POLA violation with \c service=

Pavel Stehule wrote:
> 2015-02-20 8:22 GMT+01:00 David Fetter <david@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