Re: POLA violation with \c service=

Hi

2015-02-20 21:55 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:

> Pavel Stehule wrote:
> > 2015-02-20 8:22 GMT+01:00 David Fetter <david(at)fetter(dot)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.
>

I don't think so it is preferable way - me (as developer) doesn't interest
a format of connection string - and if somebody would to check the format,
then he use a simply regexp. It is task for libpq to check and detect used
format correctly. "psql" works on very low level and needs these
functionality almost all for autocomplete - and it is not usual task for
database based applications.

>
> 2. export it in libpgcommon. If we choose this option we should
> probably rename those functions, as in the attached patch.
>

+1

>
> 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?
>

aggree with you

Regards

Pavel

>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>