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
