Thread: Missing compiled default for channel_binding param from PQconndefaults

$ python
>>> import psycopg3
>>> print([info for info in psycopg3.pq.Conninfo.get_defaults() if info.keyword == b"channel_binding"])
[ConninfoOption(keyword=b'channel_binding',
envvar=b'PGCHANNELBINDING', compiled=None, val=None,
label=b'Channel-Binding', dispchar=b'', dispsize=8)]

compiled is expected to be "prefer" or "disable" as per docs.

Please find a patch attached.

Cheers

-- Daniele

Daniele Varrazzo <daniele.varrazzo@gmail.com> writes:
> compiled is expected to be "prefer" or "disable" as per docs.
> Please find a patch attached.

Yeah, that does look inconsistent, and I don't object to fixing
it.  But it's already the case that not all connection parameters
have defaults in that struct.  Isn't there a psycopg3 bug here
too, that it's not coping with a null default sanely?

I'm a bit suspicious that psycopg3 is expecting that NULL and
empty-string are equivalent for all parameters.  That is a convention
we upheld for a long time, but recent parameters have gotten away
from that --- not only channel_binding, but others such as sslmode.
If that's the underlying cause here then we need to think about
whether we want to restore that expectation.

            regards, tom lane

On Mon, 28 Dec 2020 at 01:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Daniele Varrazzo <daniele.varrazzo@gmail.com> writes:
> > compiled is expected to be "prefer" or "disable" as per docs.
> > Please find a patch attached.
>
> Yeah, that does look inconsistent, and I don't object to fixing
> it.  But it's already the case that not all connection parameters
> have defaults in that struct.

I am testing with a function to report connection info to the user, at
a higher level than this structure and with less noise, but without
omitting meaningful info. As far as I can see, this one and "passfile"
are the only parameters for which PQconndefaults reports something
different from PQconninfo which the user hasn't set explicitly (in the
connection string or with an env var). "passfile" depends on the user
home so it's understandable.


> Isn't there a psycopg3 bug here
> too, that it's not coping with a null default sanely?
>
> I'm a bit suspicious that psycopg3 is expecting that NULL and
> empty-string are equivalent for all parameters.  That is a convention
> we upheld for a long time, but recent parameters have gotten away
> from that --- not only channel_binding, but others such as sslmode.
> If that's the underlying cause here then we need to think about
> whether we want to restore that expectation.

As far as I can see, psycopg3 reports the distinction between nulls
and empty strings ok: see the "compiled" values in this sample. Or
maybe I didn't understand your observation?

    >>> [i for i in psycopg3.pq.Conninfo.get_defaults() if i.keyword
in (b"options", b"application_name")]
    [ConninfoOption(keyword=b'options', envvar=b'PGOPTIONS',
compiled=b'', val=b'', label=b'Backend-Options', dispchar=b'',
dispsize=40),
    ConninfoOption(keyword=b'application_name', envvar=b'PGAPPNAME',
compiled=None, val=b'piro@baloo', label=b'Application-Name',
dispchar=b'', dispsize=64)]

Psycopg itself doesn't do anything actively with these defaults, e.g.
it doesn't use them to build a connection string. When a user creates
a connection string, empty strings are respected as valid parameter
values and not discarded or conflated otherwise with None. If there is
more care to have somewhere else I'm happy to know and adjust
accordingly. It would be good to know an example to keep in the
regression tests.


-- Daniele

Daniele Varrazzo <daniele.varrazzo@gmail.com> writes:
> On Mon, 28 Dec 2020 at 01:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Isn't there a psycopg3 bug here
>> too, that it's not coping with a null default sanely?

> As far as I can see, psycopg3 reports the distinction between nulls
> and empty strings ok: see the "compiled" values in this sample. Or
> maybe I didn't understand your observation?

If the null parameter gets passed through cleanly, why don't we end
up with connectOptions2 inserting the correct value (line 1257 as
of HEAD)?  It's possible that there's more than one thing going
wrong here, but I don't really understand why the existing code is
leading to a failure.

            regards, tom lane

I wrote:
> If the null parameter gets passed through cleanly, why don't we end
> up with connectOptions2 inserting the correct value (line 1257 as
> of HEAD)?

Oh, wait, I misunderstood your original complaint.  It's not that
connections are failing, it's just that you would like PQconndefaults
to make the default value visible if there is one, right?

            regards, tom lane

On Mon, 28 Dec 2020 at 03:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > If the null parameter gets passed through cleanly, why don't we end
> > up with connectOptions2 inserting the correct value (line 1257 as
> > of HEAD)?
>
> Oh, wait, I misunderstood your original complaint.  It's not that
> connections are failing, it's just that you would like PQconndefaults
> to make the default value visible if there is one, right?

Yes, I was about to reply exactly that :) I've only found a
discrepancy between PQconninfo(pgconn) and PQconndefaults(), not a
connection problem

Cheers

-- Daniele

Daniele Varrazzo <daniele.varrazzo@gmail.com> writes:
> On Mon, 28 Dec 2020 at 03:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Oh, wait, I misunderstood your original complaint.  It's not that
>> connections are failing, it's just that you would like PQconndefaults
>> to make the default value visible if there is one, right?

> Yes, I was about to reply exactly that :) I've only found a
> discrepancy between PQconninfo(pgconn) and PQconndefaults(), not a
> connection problem

Right.  Pushed.

            regards, tom lane