Re: POLA violation with \c service=

Hi all

I am sending a review of this patch:

* What it does? - Allow to connect to other db by \connect uri connection
format

postgres=# \c postgresql://localhost?service=old
psql (9.5devel, server 9.2.9)
You are now connected to database "postgres" as user "pavel".

* Would we this feature? - yes, it eliminate inconsistency between cmd line
connect and \connect. It is good idea without any objections.

* This patch is cleanly applicable, later compilation without any issues

* All regress tests passed

* A psql documentation is updated -- this feature (and format) is not
widely known, so maybe some more examples are welcome

* When I tested this feature, it worked as expected

* Code respects PostgreSQL coding rules. I prefer a little bit different
test if keep password. Current code is little bit harder to understand. But
I can live with David's code well too.

if
(!user)

user = PQuser(o_conn);

if
(!host)

host =
PQhost(o_conn);

if
(!port)

port =
PQport(o_conn);

if
(dbname)

has_connection_string =
recognized_connection_string(dbname);

/* we should not to keep password if some connection property is changed
*/

keep_password = strcmp(user, PQuser(o_conn)) == 0 && strcmp(host,
PQhost(o_conn)) == 0
&& strcmp(port, PQport(o_conn)) == 0 &&
!has_connection_string;

I have not any other comments.

Possible questions:
1. more examples in doc
2. small change how to check keep_password

Regards

Pavel

2015-01-13 15:00 GMT+01:00 David Fetter <david(at)fetter(dot)org>:

> On Sat, Jan 10, 2015 at 04:41:16PM -0800, David Fetter wrote:
> > On Sat, Jan 10, 2015 at 09:30:57AM +0100, Erik Rijkers wrote:
> > > On Fri, January 9, 2015 20:15, David Fetter wrote:
> > > > [psql_fix_uri_service_003.patch]
> > >
> > > Applies on master; the feature (switching services) works well but a
> \c without any parameters produces a segfault:
> > >
> > > (centos 6.6, 4.9.2, 64-bit)
> > >
> > >
> > > $ echo -en "$PGSERVICEFILE\n$PGSERVICE\n$PGPORT\n"
> > > /home/aardvark/.pg_service
> > > service_pola
> > > 6968
> > >
> > > $ psql
> > > Timing is on.
> > > psql (9.5devel_service_pola_20150109_2340_ac7009abd228)
> > > Type "help" for help.
> > >
> > > testdb=# \c service=HEAD
> > > You are now connected to database "testdb" as user "aardvark" via
> socket in "/tmp" at port "6545".
> > > testdb=# \c service=service_pola
> > > You are now connected to database "testdb" as user "aardvark" via
> socket in "/tmp" at port "6968".
> > > testdb=# \c
> > > Segmentation fault (core dumped)
> >
> > Fixed by running that function only if the argument exists.
> >
> > More C cleanups, too.
>
> Added to the upcoming commitfest.
>
> Cheers,
> David.
> --
> David Fetter <david(at)fetter(dot)org> http://fetter.org/
> Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
> Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>