Обсуждение: Reviewing patch "URI connection string support for libpq"

Поиск
Список
Период
Сортировка

Reviewing patch "URI connection string support for libpq"

От
Harold Giménez
Дата:
Hello hackers,

I've been a reader of this list for some time, but have never posted.

I have interest in the URI connection string support patch[1], so I'm in the process of reviewing it. I have a couple of comments and questions:

1. I see no tests in the patch. I'd like to start getting together a set of tests, likely based on the connection string permutations found on Greg Smith's response[2]. However I don't find an obvious place to put them. They could possibly live in the test/examples directory. Another thought is to use dblink in a test, although it may be problematic to depend on a contrib package for a test, to say the least. Any thoughts on how to test this are most welcome.
2. The documentation/manual was not updated as part of this patch, so this is pending.
3. I for one do prefer the `postgres` prefix, as opposed to `postgresql` for the reasons stated on an earlier thread [3]. In my opinion the best way to move forward is to support them both.

The good news is the patch still applies fine on the 9.2 HEAD, and seems to work well.

Regards,

-Harold

Re: Reviewing patch "URI connection string support for libpq"

От
Alex Shulgin
Дата:
Harold Giménez <harold.gimenez@gmail.com> writes:

>    I have interest in the URI connection string support patch[1], so I'm
>    in the process of reviewing it. I have a couple of comments and
>    questions:

Hello, thank you for your interest and the review!

>    1. I see no tests in the patch. I'd like to start getting together a
>    set of tests, likely based on the connection string permutations found
>    on Greg Smith's response[2]. However I don't find an obvious place to
>    put them. They could possibly live in the test/examples directory.
>    Another thought is to use dblink in a test, although it may be
>    problematic to depend on a contrib package for a test, to say the
>    least. Any thoughts on how to test this are most welcome.

I was also curious on how to add any sort of regression testing (likely
using psql,) but didn't get any advice as far as I can recall.

>    2. The documentation/manual was not updated as part of this patch, so
>    this is pending.

I've marked the patch as Work-In-Progress and specifically omitted
documentation changes until we settle on functionality.

>    3. I for one do prefer the `postgres` prefix, as opposed to
>    `postgresql` for the reasons stated on an earlier thread [3]. In my
>    opinion the best way to move forward is to support them both.

The updated v4 version of the patch does cover this:
http://archives.postgresql.org/pgsql-hackers/2012-02/msg01141.php

--
Alex