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