Postgres Pro
Downloads
Home   >   mailing lists

Re: POLA violation with \c service= - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: POLA violation with \c service=
Date
Msg-id CAFj8pRDd9iiQ--o+bz3YoOVBW_FKkNrXOxhGu_tAh-J8sdS7tA@mail.gmail.com
Whole thread Raw
In response to Re: POLA violation with \c service=  (David Fetter <david@fetter.org>)
Responses Re: POLA violation with \c service=  (David Fetter <david@fetter.org>)
List pgsql-hackers
Tree view
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@fetter.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: Small doc patch about pg_service.conf
Next
From: Heikki Linnakangas
Date:
Subject: Re: Re: [COMMITTERS] pgsql: Another attempt at fixing Windows Norwegian locale.