Re: let's make the list of reportable GUCs configurable (was Re: Add %r substitution for psql prompts to show recovery status)

On Wed, Jan 10, 2018 at 3:22 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> My point is specifically that that reasoning fails for features that you
> might try to use to determine what the server version is, or that you
> might try to use before finding out what the server version is.

OK, I didn't understand that your objection was that narrow.

> For
> example somebody might get cute and put an attempt to set _pq_.report
> into their connection request packet. It'll work fine as long as they
> don't test against old servers.

Even though I've referred to commit
ae65f6066dc3d19a55f4fdcd3b30003c5ad8dbed twice in this email thread so
far, this comment makes it look as though you haven't read it, or even
the commit message. The startup packet would be the only place you
*could* put it.

> Yes, you can code correctly if you recognize that the hazard exists,
> I'm just worried about people failing to recognize that. We don't
> have any infrastructure for systematic testing of newer client code
> against older server code, so it's something that bears worrying IMO.
>
> So mostly what I'm objecting to is your claim that applying this feature
> to server_version_num will do anything useful. Leaving that aside,
> it might well be a worthwhile idea.

Well, the reason Craig wants to server_version_num to be GUC_REPORT is
that it's simpler to parse than server_version, and therefore less
error-prone. I discovered today that Craig Sabino Mullane had it as
GUC_REPORT in the 2006 patch that originally added it. That was
commit 04912899e792094ed00766b99b6c604cadf9edf7, which articulated the
parsing-is-simpler justification explicitly. You forced the removal
of GUC_REPORT back then, too (commit
5086dfceba79ecd5d1eb28b8f4ed5221838ff3a6).

But if we add this feature and somebody wants to use it for
server_version_num, it's really pretty simple. In the startup packet,
you say _pq_.report=server_version_num. Then, you call
PQparameterStatus(conn, "server_version_num"). If you don't get a
value, you try calling PQparameterStatus(conn, "server_version") and
extracting the second word. If that still doesn't work then you give
up. That doesn't seem either useless or difficult to implement
correctly from here.

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