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

2019-12-29 Thread Robert Haas
On Sun, Dec 29, 2019 at 8:35 AM Dave Cramer wrote: > So where are we on this patch ? AFAICT using _pq is a protocol level option. It is, but it only lets you set the initial value, not change things later. Fixing that is probably going to require introducing a new protocol message with careful

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

2019-12-29 Thread Dave Cramer
On Wed, 6 Nov 2019 at 11:09, Robert Haas wrote: > On Tue, Nov 5, 2019 at 10:02 AM Alvaro Herrera > wrote: > > There's a reason the SQL standard defines SET SESSION AUTHORIZATION but > > no RESET SESSION AUTHORIZATION: once you enter a security context, you > > cannot escape it. ISTM that

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

2019-11-06 Thread Robert Haas
On Tue, Nov 5, 2019 at 10:02 AM Alvaro Herrera wrote: > There's a reason the SQL standard defines SET SESSION AUTHORIZATION but > no RESET SESSION AUTHORIZATION: once you enter a security context, you > cannot escape it. ISTM that essentially we broke feature F321 "User > authorization" by

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

2019-11-06 Thread Robert Haas
On Tue, Nov 5, 2019 at 9:06 AM Peter Eisentraut wrote: > Yeah, the problem I see here is this: Client 1 uses language driver A, > client 2 uses language driver B. Connection pooling is in use, and they > both connect to the same pool. Everyone is happy. > > Now this feature gets introduced.

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

2019-11-05 Thread Alvaro Herrera
On 2019-Oct-08, Craig Ringer wrote: > On Fri, 12 Jul 2019 at 01:27, Robert Haas wrote: > > > We have steadfastly refused to provide protocol-level tools for things > > like "please change my user ID, and don't let anyone change it again > > via SQL," and that's a huge problem for things like

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

2019-11-05 Thread Peter Eisentraut
On 2019-10-12 05:05, Tom Lane wrote: Andres Freund writes: On 2019-10-11 16:30:17 -0400, Robert Haas wrote: But, if it does need to be changed, it seems like a terrible idea to allow it to be done via SQL. Otherwise, the user can break the driver by using SQL to set the list to something that

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

2019-11-05 Thread Dave Cramer
On Sun, 3 Nov 2019 at 19:40, Thomas Munro wrote: > On Wed, Oct 16, 2019 at 6:49 PM Dave Cramer wrote: > > Here's an updated patch that addresses some of Andres' concerns > specifically does not use strtok. > > Hi Dave, > > I think you need to s/strncasecmp/pg_strncasecmp/ to make this build on

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

2019-11-03 Thread Thomas Munro
On Wed, Oct 16, 2019 at 6:49 PM Dave Cramer wrote: > Here's an updated patch that addresses some of Andres' concerns specifically > does not use strtok. Hi Dave, I think you need to s/strncasecmp/pg_strncasecmp/ to make this build on Windows.

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

2019-10-15 Thread Dave Cramer
On Sat, 12 Oct 2019 at 05:05, Tom Lane wrote: > Andres Freund writes: > > On 2019-10-11 16:30:17 -0400, Robert Haas wrote: > >> But, if it does need to be changed, it seems like a terrible idea to > >> allow it to be done via SQL. Otherwise, the user can break the driver > >> by using SQL to

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

2019-10-11 Thread Tom Lane
Andres Freund writes: > On 2019-10-11 16:30:17 -0400, Robert Haas wrote: >> But, if it does need to be changed, it seems like a terrible idea to >> allow it to be done via SQL. Otherwise, the user can break the driver >> by using SQL to set the list to something that the driver's not >>

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

2019-10-11 Thread Andres Freund
Hi, On 2019-10-11 16:30:17 -0400, Robert Haas wrote: > On Fri, Oct 11, 2019 at 8:21 AM Dave Cramer wrote: > > So off the top of my head providing a system function seems like the way to > > go here unless you were contemplating adding something to the protocol ? > > Since the list of

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

2019-10-11 Thread Chapman Flack
On 10/11/19 4:44 PM, Dave Cramer wrote: > On Fri, 11 Oct 2019 at 16:41, Chapman Flack >> On 10/11/19 4:30 PM, Robert Haas wrote: >>> allow it to be done via SQL. Otherwise, the user can break the driver >>> by using SQL to set the list to something that the driver's not >>> expecting, and

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

2019-10-11 Thread Dave Cramer
On Fri, 11 Oct 2019 at 16:41, Chapman Flack wrote: > On 10/11/19 4:30 PM, Robert Haas wrote: > > > But, if it does need to be changed, it seems like a terrible idea to > > allow it to be done via SQL. Otherwise, the user can break the driver > > by using SQL to set the list to something that the

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

2019-10-11 Thread Chapman Flack
On 10/11/19 4:30 PM, Robert Haas wrote: > But, if it does need to be changed, it seems like a terrible idea to > allow it to be done via SQL. Otherwise, the user can break the driver > by using SQL to set the list to something that the driver's not > expecting, and there's nothing the driver can

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

2019-10-11 Thread Robert Haas
On Fri, Oct 11, 2019 at 8:21 AM Dave Cramer wrote: > So off the top of my head providing a system function seems like the way to > go here unless you were contemplating adding something to the protocol ? Since the list of reportable GUCs is for the benefit of the driver, I'm not sure why this

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

2019-10-11 Thread Dave Cramer
On Thu, 10 Oct 2019 at 12:05, Andres Freund wrote: > Hi, > > On 2019-10-09 16:29:07 -0400, Dave Cramer wrote: > > I've added functionality into libpq to be able to set this STARTUP > > parameter as well as changed it to _pq_.report. > > Still need to document this and figure out how to test it.

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

2019-10-10 Thread Craig Ringer
On Thu, 10 Oct 2019 at 23:45, Andres Freund wrote: > > Unless schema qualified you can't rely on that even without search_path > changing. Consider an object in schema b existing, with a search_path of > a,b. Even without the search path changing, somebody could create a type > in a, "masking"

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

2019-10-10 Thread Andres Freund
Hi, On 2019-10-09 16:29:07 -0400, Dave Cramer wrote: > I've added functionality into libpq to be able to set this STARTUP > parameter as well as changed it to _pq_.report. > Still need to document this and figure out how to test it. > From 85de9f48f80a3bfd9e8bdd4f1ba6b177b1ff9749 Mon Sep 17

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

2019-10-10 Thread Andres Freund
Hi, On 2019-10-08 23:57:24 +0800, Craig Ringer wrote: > In other places I've (ab)used GUC_REPORT to push information back to the > client as a workaround for the lack of protocol extensibility / custom > messages. It's a little less ugly than abusing NOTICE messages. I'd prefer > a real way to

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

2019-10-09 Thread Dave Cramer
On Wed, 9 Oct 2019 at 08:15, Dave Cramer wrote: > > > On Tue, 8 Oct 2019 at 11:57, Craig Ringer wrote: > >> On Thu, 11 Jul 2019 at 04:34, Tom Lane wrote: >> >>> Robert Haas writes: >>> > On Wed, Jul 10, 2019 at 9:59 AM Dave Cramer wrote: >>> >> I'm still a bit conflicted about what to do

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

2019-10-09 Thread Dave Cramer
On Tue, 8 Oct 2019 at 11:57, Craig Ringer wrote: > On Thu, 11 Jul 2019 at 04:34, Tom Lane wrote: > >> Robert Haas writes: >> > On Wed, Jul 10, 2019 at 9:59 AM Dave Cramer wrote: >> >> I'm still a bit conflicted about what to do with search_path as I do >> believe this is potentially a

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

2019-10-08 Thread Craig Ringer
On Thu, 11 Jul 2019 at 04:34, Tom Lane wrote: > Robert Haas writes: > > On Wed, Jul 10, 2019 at 9:59 AM Dave Cramer wrote: > >> I'm still a bit conflicted about what to do with search_path as I do > believe this is potentially a security issue. > >> It may be that we always want to report that

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

2019-10-08 Thread Craig Ringer
On Fri, 12 Jul 2019 at 01:27, Robert Haas wrote: > > We have steadfastly refused to provide protocol-level tools for things > like "please change my user ID, and don't let anyone change it again > via SQL," and that's a huge problem for things like connection poolers > which can't parse all the

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

2019-10-02 Thread Robert Haas
On Thu, Jul 11, 2019 at 3:16 PM Dave Cramer wrote: > On Thu, 11 Jul 2019 at 15:07, Robert Haas wrote: >> On Thu, Jul 11, 2019 at 2:29 PM Dave Cramer wrote: >> > So if I understand this correctly if user bob has altered his search path >> > and there is a security-definer function called owned

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

2019-07-11 Thread Dave Cramer
On Thu, 11 Jul 2019 at 15:07, Robert Haas wrote: > On Thu, Jul 11, 2019 at 2:29 PM Dave Cramer wrote: > > So if I understand this correctly if user bob has altered his search > path and there is a security-definer function called owned by him then > > the search path will be changed for the

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

2019-07-11 Thread Robert Haas
On Thu, Jul 11, 2019 at 2:29 PM Dave Cramer wrote: > So if I understand this correctly if user bob has altered his search path and > there is a security-definer function called owned by him then > the search path will be changed for the duration of the function and reported > for every

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

2019-07-11 Thread Dave Cramer
On Wed, 10 Jul 2019 at 16:22, Robert Haas wrote: > On Wed, Jul 10, 2019 at 9:59 AM Dave Cramer wrote: > > I'm still a bit conflicted about what to do with search_path as I do > believe this is potentially a security issue. > > It may be that we always want to report that and possibly back patch

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

2019-07-11 Thread Robert Haas
On Thu, Jul 11, 2019 at 10:35 AM Tom Lane wrote: > All of the above is based on the assumption that this isn't a plain > old USERSET GUC, which I'm not really seeing the argument for. > OK, there might be *implementation* reasons why we would rather not > deal with on-the-fly changes to the list,

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

2019-07-11 Thread Dave Cramer
On Thu, 11 Jul 2019 at 10:19, Robert Haas wrote: > On Thu, Jul 11, 2019 at 10:11 AM Tom Lane wrote: > > Robert Haas writes: > > > 3. I'm not sure that just ignoring any GUCs we don't find is the right > > > thing. I'm also not sure that it's the wrong thing, but it might be. > > > My question

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

2019-07-11 Thread Tom Lane
Robert Haas writes: > I had the same thought, but I just realized that's probably > unfriendly: at the point when the client is assembling the list of > names to send to the server, it doesn't know the server version. I > think we're probably best off assuming that any names we don't > recognize

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

2019-07-11 Thread Robert Haas
On Thu, Jul 11, 2019 at 10:11 AM Tom Lane wrote: > Robert Haas writes: > > 3. I'm not sure that just ignoring any GUCs we don't find is the right > > thing. I'm also not sure that it's the wrong thing, but it might be. > > My question is: what if there's an extension-owned GUC in play? The > >

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

2019-07-11 Thread Tom Lane
Robert Haas writes: > 3. I'm not sure that just ignoring any GUCs we don't find is the right > thing. I'm also not sure that it's the wrong thing, but it might be. > My question is: what if there's an extension-owned GUC in play? The > library probably isn't even loaded at this stage, unless

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

2019-07-11 Thread Dave Cramer
On Thu, 11 Jul 2019 at 10:06, Robert Haas wrote: > On Thu, Jul 11, 2019 at 8:23 AM Dave Cramer wrote: > > See attached for an initial patch. If this is an acceptable way to go I > will add tests and documentation > > And clean up the code? Doesn't look crazy on a quick glance but I > think I

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

2019-07-11 Thread Dave Cramer
On Wed, 10 Jul 2019 at 16:34, Tom Lane wrote: > Robert Haas writes: > > On Wed, Jul 10, 2019 at 9:59 AM Dave Cramer wrote: > >> I'm still a bit conflicted about what to do with search_path as I do > believe this is potentially a security issue. > >> It may be that we always want to report that

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

2019-07-10 Thread Tom Lane
Robert Haas writes: > On Wed, Jul 10, 2019 at 9:59 AM Dave Cramer wrote: >> I'm still a bit conflicted about what to do with search_path as I do believe >> this is potentially a security issue. >> It may be that we always want to report that and possibly back patch it. > I don't see that as a

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

2019-07-10 Thread Robert Haas
On Wed, Jul 10, 2019 at 9:59 AM Dave Cramer wrote: > I'm still a bit conflicted about what to do with search_path as I do believe > this is potentially a security issue. > It may be that we always want to report that and possibly back patch it. I don't see that as a feasible option unless we

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

2019-07-10 Thread Dave Cramer
On Wed, 10 Jul 2019 at 09:11, Robert Haas wrote: > On Tue, Jul 9, 2019 at 2:32 PM Dave Cramer wrote: > > So did this die from lack of interest? > > > > I have proposed in another thread adding more GUC REPORT variables, but > I see this as a much better way. > > > > I'm willing to code the

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

2019-07-10 Thread Robert Haas
On Tue, Jul 9, 2019 at 2:32 PM Dave Cramer wrote: > So did this die from lack of interest? > > I have proposed in another thread adding more GUC REPORT variables, but I see > this as a much better way. > > I'm willing to code the patch if we can get some buy in here ? It seemed like most people

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

2019-07-09 Thread Dave Cramer
On Mon, 22 Jan 2018 at 07:39, Robert Haas wrote: > On Sun, Jan 21, 2018 at 5:41 PM, Craig Ringer > wrote: > > If we'd done server_version_num in 9.5, for example, less stuff would've > > broken with pg10. > > Yeah, and if Tom hadn't forced it to be reverted from *8.2*, then > every version

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

2018-01-22 Thread Robert Haas
On Sun, Jan 21, 2018 at 5:41 PM, Craig Ringer wrote: > If we'd done server_version_num in 9.5, for example, less stuff would've > broken with pg10. Yeah, and if Tom hadn't forced it to be reverted from *8.2*, then every version anyone still cares about would now have

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

2018-01-21 Thread Craig Ringer
On 11 January 2018 at 09:55, Tom Lane wrote: > Robert Haas writes: > > 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.

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

2018-01-10 Thread Robert Haas
On Wed, Jan 10, 2018 at 3:55 PM, Tom Lane wrote: > Robert Haas writes: >> 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.

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

2018-01-10 Thread Robert Haas
On Wed, Jan 10, 2018 at 3:32 PM, Chapman Flack wrote: > Is there a notion like that in the pq protocol now? If not, and > a protocol bump becomes necessary to meet some need, would it be > worth adding such a notion at the same time, to simplify future > evolution? For the

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

2018-01-10 Thread Robert Haas
On Wed, Jan 10, 2018 at 3:22 PM, Tom Lane 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

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

2018-01-10 Thread Chapman Flack
On 01/10/2018 03:11 PM, Robert Haas wrote: > it will only work on versions that support that option, but that is > true of any new feature. Furthermore, they will easily be able to > tell based on the reported server version whether or not their request > for different behavior was accepted by

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

2018-01-10 Thread Tom Lane
Robert Haas writes: > Your argument here sounds suspiciously like "If we add a new feature > and people use it in a stupid way then it may cause their stuff not to > work". I think you're attacking a straw man ... > Everything that worked before adding an option like

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

2018-01-10 Thread Robert Haas
On Wed, Jan 10, 2018 at 12:36 PM, Tom Lane wrote: > I don't care at all if J. Random Developer's homegrown code only works > with the PG version he's using. The concern I have is that unwanted > server version dependencies will sneak into widely used code, like > psql, or

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

2018-01-10 Thread Chapman Flack
On 01/10/2018 11:08 AM, Robert Haas wrote: > I think that we really need to think about allowing clients to tell > the server which GUCs they'd like reported, instead of having a single > list to which everyone is bound. +1 That already sounded like a good idea back in

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

2018-01-10 Thread Joshua D. Drake
On 01/10/2018 09:22 AM, Tom Lane wrote: ... but I don't think it fixes that, because you couldn't send this new request without making an assumption about the server version being new enough to support it. My entire beef with making server_version_num be GUC_REPORT is that it would encourage

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

2018-01-10 Thread Tom Lane
Robert Haas writes: > I think that we really need to think about allowing clients to tell > the server which GUCs they'd like reported, instead of having a single > list to which everyone is bound. Interesting idea ... > As a side benefit, then Craig and Tom can stop

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

2018-01-10 Thread Simon Riggs
On 10 January 2018 at 16:08, Robert Haas wrote: > I think that we really need to think about allowing clients to tell > the server which GUCs they'd like reported, instead of having a single > list to which everyone is bound. +1 -- Simon Riggs

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

2018-01-10 Thread Robert Haas
On Tue, Jan 9, 2018 at 3:36 PM, Peter Eisentraut wrote: > I agree a backend status message is the right way to do this. > > We could perhaps report transaction_read_only, if we don't want to add a > new one. That's not really the same thing, though. I think