Re: PQconninfoParse()

Hey,

I suggest to clarify the documentation of PQconninfoParse().

First,
"Returns parsed connection options from the provided connection string."
Its false. The returned array contains all options, rather than which only
parsed from the provided connection string. This fact must be specified in
the documentation becase the name of the function mismatches its behaviour.

Second,
"Note that only options explicitly specified in the string will have values
set
in the result array; no defaults are inserted."
Its true. But instead of "no defaults are inserted" I suggest "values of
other
options will have NULL values."

--
// Dmitriy.

On Tue, Sep 27, 2011 at 9:13 AM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com> wrote:
> First,
> "Returns parsed connection options from the provided connection string."
> Its false. The returned array contains all options, rather than which only
> parsed from the provided connection string. This fact must be specified in
> the documentation becase the name of the function mismatches its behaviour.

What do you mean by "all" options? Where else are they coming from
besides the connection string?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hey Robert,

2011/10/10 Robert Haas <robertmhaas(at)gmail(dot)com>

> On Tue, Sep 27, 2011 at 9:13 AM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com>
> wrote:
> > First,
> > "Returns parsed connection options from the provided connection string."
> > Its false. The returned array contains all options, rather than which
> only
> > parsed from the provided connection string. This fact must be specified
> in
> > the documentation becase the name of the function mismatches its
> behaviour.
>
> What do you mean by "all" options? Where else are they coming from
> besides the connection string?
>
I mean options from
static const PQconninfoOption PQconninfoOptions[]
from interfaces/libpq/fe-connect.c

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
// Dmitriy.

On Mon, Oct 10, 2011 at 1:31 PM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com> wrote:
> Hey Robert,
>
> 2011/10/10 Robert Haas <robertmhaas(at)gmail(dot)com>
>>
>> On Tue, Sep 27, 2011 at 9:13 AM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com>
>> wrote:
>> > First,
>> > "Returns parsed connection options from the provided connection string."
>> > Its false. The returned array contains all options, rather than which
>> > only
>> > parsed from the provided connection string. This fact must be specified
>> > in
>> > the documentation becase the name of the function mismatches its
>> > behaviour.
>>
>> What do you mean by "all" options?  Where else are they coming from
>> besides the connection string?
>
> I mean options from
> static const PQconninfoOption PQconninfoOptions[]
> from interfaces/libpq/fe-connect.c

Maybe we should change this:

Note that only options explicitly specified in the string will have
values set in the result array; no defaults are inserted.

To say something like this:

All legal options will be present in the result array, but only those
explicitly specified in the string will have a corresponding value; no
defaults are inserted.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

2011/10/15 Robert Haas <robertmhaas(at)gmail(dot)com>

> On Mon, Oct 10, 2011 at 1:31 PM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com>
> wrote:
> > Hey Robert,
> >
> > 2011/10/10 Robert Haas <robertmhaas(at)gmail(dot)com>
> >>
> >> On Tue, Sep 27, 2011 at 9:13 AM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com>
> >> wrote:
> >> > First,
> >> > "Returns parsed connection options from the provided connection
> string."
> >> > Its false. The returned array contains all options, rather than which
> >> > only
> >> > parsed from the provided connection string. This fact must be
> specified
> >> > in
> >> > the documentation becase the name of the function mismatches its
> >> > behaviour.
> >>
> >> What do you mean by "all" options? Where else are they coming from
> >> besides the connection string?
> >
> > I mean options from
> > static const PQconninfoOption PQconninfoOptions[]
> > from interfaces/libpq/fe-connect.c
>
> Maybe we should change this:
>
> Note that only options explicitly specified in the string will have
> values set in the result array; no defaults are inserted.
>
> To say something like this:
>
> All legal options will be present in the result array, but only those
> explicitly specified in the string will have a corresponding value; no
> defaults are inserted.
>
Exactly! Perfect!

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
// Dmitriy.

2011/10/15 Dmitriy Igrishin <dmitigr(at)gmail(dot)com>

>
>
> 2011/10/15 Robert Haas <robertmhaas(at)gmail(dot)com>
>
>> On Mon, Oct 10, 2011 at 1:31 PM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com>
>> wrote:
>> > Hey Robert,
>> >
>> > 2011/10/10 Robert Haas <robertmhaas(at)gmail(dot)com>
>> >>
>> >> On Tue, Sep 27, 2011 at 9:13 AM, Dmitriy Igrishin <dmitigr(at)gmail(dot)com>
>> >> wrote:
>> >> > First,
>> >> > "Returns parsed connection options from the provided connection
>> string."
>> >> > Its false. The returned array contains all options, rather than which
>> >> > only
>> >> > parsed from the provided connection string. This fact must be
>> specified
>> >> > in
>> >> > the documentation becase the name of the function mismatches its
>> >> > behaviour.
>> >>
>> >> What do you mean by "all" options? Where else are they coming from
>> >> besides the connection string?
>> >
>> > I mean options from
>> > static const PQconninfoOption PQconninfoOptions[]
>> > from interfaces/libpq/fe-connect.c
>>
>> Maybe we should change this:
>>
>> Note that only options explicitly specified in the string will have
>> values set in the result array; no defaults are inserted.
>>
>> To say something like this:
>>
>> All legal options will be present in the result array, but only those
>> explicitly specified in the string will have a corresponding value; no
>> defaults are inserted.
>>
> Exactly! Perfect!
>
Also, I would say something about deprecated options
such as "authtype" which are still presents in the returned
array.

>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
>
> --
> // Dmitriy.
>
>
>

--
// Dmitriy.

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Maybe we should change this:

> Note that only options explicitly specified in the string will have
> values set in the result array; no defaults are inserted.

> To say something like this:

> All legal options will be present in the result array, but only those
> explicitly specified in the string will have a corresponding value; no
> defaults are inserted.

Uh, is that actually a true statement? I thought the result *did*
include default values. That's more or less the point of returning them
all, after all.

regards, tom lane

2011/10/15 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > Maybe we should change this:
>
> > Note that only options explicitly specified in the string will have
> > values set in the result array; no defaults are inserted.
>
> > To say something like this:
>
> > All legal options will be present in the result array, but only those
> > explicitly specified in the string will have a corresponding value; no
> > defaults are inserted.
>
> Uh, is that actually a true statement? I thought the result *did*
> include default values. That's more or less the point of returning them
> all, after all.
>
Oops, indeed, if I understand your correctly thats what
I meant in my first post - "values of other options will have
NULL values" instead of "no defaults are inserted".

>
> regards, tom lane
>

--
// Dmitriy.

On Sat, Oct 15, 2011 at 11:24 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Maybe we should change this:
>
>> Note that only options explicitly specified in the string will have
>> values set in the result array; no defaults are inserted.
>
>> To say something like this:
>
>> All legal options will be present in the result array, but only those
>> explicitly specified in the string will have a corresponding value; no
>> defaults are inserted.
>
> Uh, is that actually a true statement?  I thought the result *did*
> include default values.  That's more or less the point of returning them
> all, after all.

Well, then I'm confused, because you and Dmitriy seem to be saying
opposite things.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sat, Oct 15, 2011 at 11:24 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Uh, is that actually a true statement? I thought the result *did*
>> include default values. That's more or less the point of returning them
>> all, after all.

> Well, then I'm confused, because you and Dmitriy seem to be saying
> opposite things.

[ after experimenting with the code ... ] Oh, I had been thinking that
PQconndefaults gives the same result as PQconninfoParse with an
empty-string argument, but that's not the case. Indeed, the former
fills in default values as current values, but the latter does not.

The proposed wording change seems reasonable, except that "have a
corresponding value" seems a bit vague. Maybe better "have a non-null
val field".

regards, tom lane

On Tue, Oct 18, 2011 at 8:27 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sat, Oct 15, 2011 at 11:24 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Uh, is that actually a true statement?  I thought the result *did*
>>> include default values.  That's more or less the point of returning them
>>> all, after all.
>
>> Well, then I'm confused, because you and Dmitriy seem to be saying
>> opposite things.
>
> [ after experimenting with the code ... ]  Oh, I had been thinking that
> PQconndefaults gives the same result as PQconninfoParse with an
> empty-string argument, but that's not the case.  Indeed, the former
> fills in default values as current values, but the latter does not.
>
> The proposed wording change seems reasonable, except that "have a
> corresponding value" seems a bit vague.  Maybe better "have a non-null
> val field".

I've committed something along these lines.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company