Re: POLA violation with \c service=

On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> 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;

Changed.  This is cleaner.

> I have not any other comments.
>
> Possible questions:
>   1. more examples in doc

I'm not sure how best to illustrate those.  Are you thinking of one
example each for the URI and conninfo cases?

>   2. small change how to check keep_password

Done.

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