On Mon, 9 Oct 2023 at 22:25, Jeff Davis <pg...@j-davis.com> wrote: > We absolutely would > need to update the documentation, and clients (like psql) really should > be updated.
+1 > > I think one > > could conclude on these facts either that (a) client_encoding is fine > > and the problems with controlling behavior using that kind of > > mechanism are mostly theoretical or > > I'm not clear on the exact rules for a protocol version bump and why a > GUC helps us avoid one. If we have a binary_formats GUC, the client > would need to know the server version and check that it's >=17 before > sending the "SET binary_formats='...'" commmand, right? I agree that we'd probably still want to do a protocol minor version bump. FYI there is another thread trying to introduce protocol change which needs a minor version bump. Patch number 0003 in that patchset is meant to actually make libpq handle minor version increases correctly. If we need a version bump than that would be useful[1]. > What's the > difference between that and making it an explicit protocol message that > only >=17 understand? Honestly I think the main difference is the need to introduce this explicit protocol message. If we do, I think it might be best to have this be a way of setting a GUC at the Protocol level, and expand the GucContext enum to have a way to disallow setting it from SQL (e.g. PGC_PROTOCOL), while still allowing PgBouncer (or other poolers) to change the GUC as part of the connection handoff, in a way that's similar to what's being done for client_encoding now. We might even want to make client_encoding PGC_PROTOCOL too (eventually). Actually, for connection poolers there's other reasons to want to set GUC values at the protocol level instead of SQL. Because the value of the ParameterStatus response is sadly not a valid SQL string... That's why in PgBouncer we have to re-quote the value [2], which is a problem for any GUC_LIST_QUOTE type, which search_path is. This GUC_LIST_QUOTE logic is actually not completely correct in PgBouncer and only handles "" (empty search_path), because for search_path that's the only reasonable problematic case that people might hit (e.g. truncating to NAMELEN is another problem, but elements in search_path should already be at most NAMELEN). But still it would be good not to have to worry about that. And being able to send the value in ParameterStatus back verbatim to the server would be quite helpful for PgBouncer. [1]: https://www.postgresql.org/message-id/flat/CAFj8pRAX48WH5Y6BbqnZbUSzmtEaQZ22rY_6cYw%3DE9QkoVvL0A%40mail.gmail.com#643c91f84ae33b316c0fed64e19c8e49 [2]: https://github.com/pgbouncer/pgbouncer/blob/60708022d5b934fa53c51849b9f02d87a7881b11/src/varcache.c#L172-L183
