Thread: specifying repeatable read in PGOPTIONS
Hi, I recently had the need to bury the used isolation level in the connection string, but it turns out that doesn't work that well... PGOPTIONS='-c default_transaction_isolation=serializable' \ psql ... -c "SHOW default_transaction_isolation" works well enough, but PGOPTIONS='-c default_transaction_isolation=repeatable read' \ psql ... -c "SHOW default_transaction_isolation" doesn't, because of the whitespace. I couldn't come up with any adequate quoting. I'd like to propose adding aliases with dashes instead of spaces to the isolation_level_options array? I'd even like to backport it, because it makes benchmarking across versions unneccessarily hard. Additionally we might want to think about a bit better quoting support for such options? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> PGOPTIONS='-c default_transaction_isolation=serializable' \
> psql ... -c "SHOW default_transaction_isolation"
> works well enough, but
> PGOPTIONS='-c default_transaction_isolation=repeatable read' \
> psql ... -c "SHOW default_transaction_isolation"
> doesn't, because of the whitespace. I couldn't come up with any adequate
> quoting.
> I'd like to propose adding aliases with dashes instead of spaces to the
> isolation_level_options array? I'd even like to backport it, because it
> makes benchmarking across versions unneccessarily hard.
-1. This is not a general solution to the problem. There are other
GUCs for which people might want spaces in the value.
> Additionally we might want to think about a bit better quoting support
> for such options?
Yeah. See pg_split_opts(), which explicitly acknowledges that it'll fall
down for space-containing options. Not sure what the most appropriate
quoting convention would be there, but I'm sure we can think of something.
regards, tom lane
On 2014-02-04 11:36:22 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > PGOPTIONS='-c default_transaction_isolation=serializable' \ > > psql ... -c "SHOW default_transaction_isolation" > > works well enough, but > > PGOPTIONS='-c default_transaction_isolation=repeatable read' \ > > psql ... -c "SHOW default_transaction_isolation" > > doesn't, because of the whitespace. I couldn't come up with any adequate > > quoting. > > > I'd like to propose adding aliases with dashes instead of spaces to the > > isolation_level_options array? I'd even like to backport it, because it > > makes benchmarking across versions unneccessarily hard. > > -1. This is not a general solution to the problem. There are other > GUCs for which people might want spaces in the value. Sure, I didn't say it was. But I don't see any oother values that are likely being passed via PGOPTIONS that frequently contain spaces. Sure, you can generate a search_path that does so, but that's just asking for problems. Most other GUCs that can contain spaces are PGC_SIGHUP/POSTMASTER. And having to use quoting just makes it awkward to use from shell. Since all the other option values try to take not to force using spaces, I see little reason not to do so here as well. > > Additionally we might want to think about a bit better quoting support > > for such options? > > Yeah. See pg_split_opts(), which explicitly acknowledges that it'll fall > down for space-containing options. Not sure what the most appropriate > quoting convention would be there, but I'm sure we can think of something. No argument against introducing it. What about simply allowing escaping of the next character using \? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-02-04 11:36:22 -0500, Tom Lane wrote:
>> -1. This is not a general solution to the problem. There are other
>> GUCs for which people might want spaces in the value.
> Sure, I didn't say it was. But I don't see any oother values that are
> likely being passed via PGOPTIONS that frequently contain spaces.
application_name --- weren't we just reading about people passing entire
command lines there? (They must be using some other way of setting it
currently, but PGOPTIONS doesn't seem like an implausible source.)
>> Yeah. See pg_split_opts(), which explicitly acknowledges that it'll fall
>> down for space-containing options. Not sure what the most appropriate
>> quoting convention would be there, but I'm sure we can think of something.
> No argument against introducing it. What about simply allowing escaping
> of the next character using \?
The same thought had occurred to me. Since it'll typically already be
inside some levels of quoting, any quoted-string convention seems like
it'd be a pain to use. But a straight backslash-escapes-the-next-char
thing wouldn't be too awful, I think.
regards, tom lane
Hi Tom, On 2014-02-04 12:02:45 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-02-04 11:36:22 -0500, Tom Lane wrote: > >> -1. This is not a general solution to the problem. There are other > >> GUCs for which people might want spaces in the value. > > > Sure, I didn't say it was. But I don't see any oother values that are > > likely being passed via PGOPTIONS that frequently contain spaces. > > application_name --- weren't we just reading about people passing entire > command lines there? (They must be using some other way of setting it > currently, but PGOPTIONS doesn't seem like an implausible source.) You can't easily use PGOPTIONS to set application_name in many cases anyway, libpq's support for it gets in the way since it takes effect later. And I think libpq is much more likely way to set it. Also you can simply circumvent the problem by using a different naming convention, that's not problem with repeatable read. So I still think we should add read_committed, repeatable_read as aliases. > >> Yeah. See pg_split_opts(), which explicitly acknowledges that it'll fall > >> down for space-containing options. Not sure what the most appropriate > >> quoting convention would be there, but I'm sure we can think of something. > > > No argument against introducing it. What about simply allowing escaping > > of the next character using \? > > The same thought had occurred to me. Since it'll typically already be > inside some levels of quoting, any quoted-string convention seems like > it'd be a pain to use. But a straight backslash-escapes-the-next-char > thing wouldn't be too awful, I think. Ok, here's a patch implementing that. There's a slight backward concern in that a \ would earlier have been passed through unmodified, but now would be taken as a escape. I think that's not too much of a problem though. I thought about simply outputting the escape unless it's been used as an escape before a speace, but that seems like a bad idea, barring future uses to me. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Attachment
On Fri, Feb 7, 2014 at 5:06 AM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2014-02-04 12:02:45 -0500, Tom Lane wrote: >> Andres Freund <andres@2ndquadrant.com> writes: >> > On 2014-02-04 11:36:22 -0500, Tom Lane wrote: >> >> -1. This is not a general solution to the problem. There are other >> >> GUCs for which people might want spaces in the value. >> >> > Sure, I didn't say it was. But I don't see any oother values that are >> > likely being passed via PGOPTIONS that frequently contain spaces. >> >> application_name --- weren't we just reading about people passing entire >> command lines there? (They must be using some other way of setting it >> currently, but PGOPTIONS doesn't seem like an implausible source.) > > You can't easily use PGOPTIONS to set application_name in many cases > anyway, libpq's support for it gets in the way since it takes effect > later. And I think libpq is much more likely way to set it. Also you can > simply circumvent the problem by using a different naming convention, > that's not problem with repeatable read. > > So I still think we should add read_committed, repeatable_read as aliases. Like Tom, I'm -1 on this. This is fixing the problem from the wrong end. But introducing an escaping convention seems more than prudent. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2014-02-09 12:00:02 -0500, Robert Haas wrote: > On Fri, Feb 7, 2014 at 5:06 AM, Andres Freund <andres@2ndquadrant.com> wrote: > > So I still think we should add read_committed, repeatable_read as aliases. > > Like Tom, I'm -1 on this. This is fixing the problem from the wrong end. Why? We do have other options with aliases for option values and all other enum option has taken care not to need spaces. > But introducing an escaping convention seems more than prudent. I've attached a patch implementing \ escapes one mail up... But I don't really see that as prohibiting also adding sensible aliases. It's just annoying that it's currently not possible to run to pgbenches testing both without either restarting the user or ALTER ROLE/DATABASE. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Sun, Feb 9, 2014 at 12:10 PM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2014-02-09 12:00:02 -0500, Robert Haas wrote: >> On Fri, Feb 7, 2014 at 5:06 AM, Andres Freund <andres@2ndquadrant.com> wrote: >> > So I still think we should add read_committed, repeatable_read as aliases. >> >> Like Tom, I'm -1 on this. This is fixing the problem from the wrong end. > > Why? We do have other options with aliases for option values and all > other enum option has taken care not to need spaces. I think that's probably mostly a happy coincidence; I'm not committed to a policy of ensuring that all GUCs can be set to whatever value you want without using the space character. Besides, what's so special about enum GUCs? There can certainly be spaces in string-valued GUCs, and you're not going to be able to get around the problem there with one-off kludges. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Feb 9, 2014 at 12:10 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>> Why? We do have other options with aliases for option values and all
>> other enum option has taken care not to need spaces.
> I think that's probably mostly a happy coincidence; I'm not committed
> to a policy of ensuring that all GUCs can be set to whatever value you
> want without using the space character. Besides, what's so special
> about enum GUCs? There can certainly be spaces in string-valued GUCs,
> and you're not going to be able to get around the problem there with
> one-off kludges.
Pathname GUCs can have spaces in them (that's even pretty common, on
certain platforms). Other GUCs contain SQL identifiers, which can
legally have spaces in them too. So really this is a mechanism
deficiency, not something we should work around by instituting a policy
against spaces in GUC values.
regards, tom lane
On 2014-02-09 12:38:18 -0500, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Sun, Feb 9, 2014 at 12:10 PM, Andres Freund <andres@2ndquadrant.com> wrote: > >> Why? We do have other options with aliases for option values and all > >> other enum option has taken care not to need spaces. > > > I think that's probably mostly a happy coincidence; I'm not committed > > to a policy of ensuring that all GUCs can be set to whatever value you > > want without using the space character. Besides, what's so special > > about enum GUCs? There can certainly be spaces in string-valued GUCs, > > and you're not going to be able to get around the problem there with > > one-off kludges. > > Pathname GUCs can have spaces in them (that's even pretty common, on > certain platforms). Other GUCs contain SQL identifiers, which can > legally have spaces in them too. But pretty much all of those GUCs are either PGC_SIGHUP or PGC_POSTMASTER and thus cannot be set via PGOPTIONS anyway. The two exceptions are application_name (which can in many cases not set because libpq overrides it with a SET) and search_path. Anybody setting the latter to schemas containing spaces deserves having to escape values. > So really this is a mechanism > deficiency, not something we should work around by instituting a policy > against spaces in GUC values. Please note, I am absolutely *not* against such a mechanism, that's why I submitted a patch implementing one. But unneccearily requiring escaping still annoys me. We imo should add the escaping mechanism to master and backpatch the aliases (read_committed, repeatable_read). There's already not enough benchmarking during beta/rc, we shouldn't make it unneccesarily hard. And there's sufficient reason to benchmark the difference between isolation modes, with mvcc catalog snapshots and such. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services