Horton

Thread

Commits

  1. Expose the default for channel_binding in PQconndefaults().

  1. Missing compiled default for channel_binding param from PQconndefaults

    Daniele Varrazzo <daniele.varrazzo@gmail.com> — 2020-12-28T01:27:08Z

    $ 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

  2. Re: Missing compiled default for channel_binding param from PQconndefaults

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-12-28T01:48:45Z

    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

  3. Re: Missing compiled default for channel_binding param from PQconndefaults

    Daniele Varrazzo <daniele.varrazzo@gmail.com> — 2020-12-28T02:37:03Z

    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

  4. Re: Missing compiled default for channel_binding param from PQconndefaults

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-12-28T02:44:52Z

    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

  5. Re: Missing compiled default for channel_binding param from PQconndefaults

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-12-28T03:00:50Z

    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

  6. Re: Missing compiled default for channel_binding param from PQconndefaults

    Daniele Varrazzo <daniele.varrazzo@gmail.com> — 2020-12-28T03:04:46Z

    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

  7. Re: Missing compiled default for channel_binding param from PQconndefaults

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-12-28T17:14:46Z

    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