Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Fri, Nov 29, 2013 at 01:19:54PM -0500, Bruce Momjian wrote: > On Fri, Nov 29, 2013 at 01:05:20PM -0500, Bruce Momjian wrote: > > On Fri, Nov 29, 2013 at 12:27:49AM -0500, Robert Haas wrote: > > > On Thu, Nov 28, 2013 at 11:04 PM, Alvaro Herrera > > > wrote: > > > > David Johnston wrote: > > > > > > > >> In all of these cases we are assuming that the user understands that > > > >> emitting a warning means that something is being logged to disk and > > > >> thus is > > > >> causing a resource drain. > > > >> > > > >> I like explicitly saying that issuing these commands is pointless/"has > > > >> no > > > >> effect"; being indirect and saying that the only thing they do is emit > > > >> a > > > >> warning omits any explicit explicit explanation of why. And while I > > > >> agree > > > >> that logging the warning is an effect; but it is not the primary/direct > > > >> effect that the user cares about. > > > > > > > > Honestly I still prefer what I proposed initially, which AFAICS has all > > > > the properties you deem desirable in the wording: > > > > > > > > "issuing ROLLBACK outside a transaction emits a warning and otherwise > > > > has no effect". > > > > > > Yeah, I still like "otherwise has no effect" or "has no other effect" > > > best. But I can live with Bruce's latest proposal, too. > > > > OK, great, I have gone with Alvaro's wording; patch attached. > > Duh, missing patch. Attached now. Patch applied. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Fri, Nov 29, 2013 at 01:05:20PM -0500, Bruce Momjian wrote:
> On Fri, Nov 29, 2013 at 12:27:49AM -0500, Robert Haas wrote:
> > On Thu, Nov 28, 2013 at 11:04 PM, Alvaro Herrera
> > wrote:
> > > David Johnston wrote:
> > >
> > >> In all of these cases we are assuming that the user understands that
> > >> emitting a warning means that something is being logged to disk and thus
> > >> is
> > >> causing a resource drain.
> > >>
> > >> I like explicitly saying that issuing these commands is pointless/"has no
> > >> effect"; being indirect and saying that the only thing they do is emit a
> > >> warning omits any explicit explicit explanation of why. And while I
> > >> agree
> > >> that logging the warning is an effect; but it is not the primary/direct
> > >> effect that the user cares about.
> > >
> > > Honestly I still prefer what I proposed initially, which AFAICS has all
> > > the properties you deem desirable in the wording:
> > >
> > > "issuing ROLLBACK outside a transaction emits a warning and otherwise has
> > > no effect".
> >
> > Yeah, I still like "otherwise has no effect" or "has no other effect"
> > best. But I can live with Bruce's latest proposal, too.
>
> OK, great, I have gone with Alvaro's wording; patch attached.
Duh, missing patch. Attached now.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/doc/src/sgml/ref/abort.sgml b/doc/src/sgml/ref/abort.sgml
new file mode 100644
index f3a2fa8..e9138d5
*** a/doc/src/sgml/ref/abort.sgml
--- b/doc/src/sgml/ref/abort.sgml
*** ABORT [ WORK | TRANSACTION ]
*** 63,69
!Issuing ABORT outside of a transaction block has no effect.
--- 63,70
!Issuing ABORT outside of a transaction block
!emits a warning and otherwise has no effect.
diff --git a/doc/src/sgml/ref/rollback.sgml b/doc/src/sgml/ref/rollback.sgml
new file mode 100644
index 4f79621..9a1529f
*** a/doc/src/sgml/ref/rollback.sgml
--- b/doc/src/sgml/ref/rollback.sgml
*** ROLLBACK [ WORK | TRANSACTION ]
*** 60,66
Issuing ROLLBACK outside of a transaction
!block has no effect.
--- 60,66
Issuing ROLLBACK outside of a transaction
!block emits a warning and otherwise has no effect.
diff --git a/doc/src/sgml/ref/set.sgml b/doc/src/sgml/ref/set.sgml
new file mode 100644
index 5a84f69..aaad61e
*** a/doc/src/sgml/ref/set.sgml
--- b/doc/src/sgml/ref/set.sgml
*** SET [ SESSION | LOCAL ] TIME ZONE {
Specifies that the command takes effect for only the current
transaction. After COMMIT or ROLLBACK,
! the session-level setting takes effect again. This has no effect
! outside of a transaction block.
--- 110,118
Specifies that the command takes effect for only the current
transaction. After COMMIT or ROLLBACK,
! the session-level setting takes effect again. Issuing this
! outside of a transaction block emits a warning and otherwise has
! no effect.
diff --git a/doc/src/sgml/ref/set_constraints.sgml b/doc/src/sgml/ref/set_constraints.sgml
new file mode 100644
index a33190c..60cabed
*** a/doc/src/sgml/ref/set_constraints.sgml
--- b/doc/src/sgml/ref/set_constraints.sgml
*** SET CONSTRAINTS { ALL |
This command only alters the behavior of constraints within the
!current transaction. This has no effect outside of a transaction block.
--- 99,106
This command only alters the behavior of constraints within the
!current transaction. Issuing this outside of a transaction block
!emits a warning and otherwise has no effect.
diff --git a/doc/src/sgml/ref/set_transaction.sgml b/doc/src/sgml/ref/set_transaction.sgml
new file mode 100644
index e90ff4a..029b75a
*** a/doc/src/sgml/ref/set_transaction.sgml
--- b/doc/src/sgml/ref/set_transaction.sgml
*** SET SESSION CHARACTERISTICS AS TRANSACTI
*** 185,191
If SET TRANSACTION is executed without a prior
START TRANSACTION or BEGIN,
!it will have no effect.
--- 185,191
If SET TRANSACTION is executed without a prior
START TRANSACTION or BEGIN,
!it emits a warning and otherwise has no effect.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Fri, Nov 29, 2013 at 12:27:49AM -0500, Robert Haas wrote: > On Thu, Nov 28, 2013 at 11:04 PM, Alvaro Herrera > wrote: > > David Johnston wrote: > > > >> In all of these cases we are assuming that the user understands that > >> emitting a warning means that something is being logged to disk and thus is > >> causing a resource drain. > >> > >> I like explicitly saying that issuing these commands is pointless/"has no > >> effect"; being indirect and saying that the only thing they do is emit a > >> warning omits any explicit explicit explanation of why. And while I agree > >> that logging the warning is an effect; but it is not the primary/direct > >> effect that the user cares about. > > > > Honestly I still prefer what I proposed initially, which AFAICS has all > > the properties you deem desirable in the wording: > > > > "issuing ROLLBACK outside a transaction emits a warning and otherwise has > > no effect". > > Yeah, I still like "otherwise has no effect" or "has no other effect" > best. But I can live with Bruce's latest proposal, too. OK, great, I have gone with Alvaro's wording; patch attached. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Fri, Nov 29, 2013 at 12:27:49AM -0500, Robert Haas wrote: > I wish we'd just left this whole thing well enough alone. It wasn't > broken, and didn't need fixing. Well, this started with a complaint that one SET command outside of a transaction had no effect, and expanded to other SET commands and the ABORT/notice inconsistency. I realize the doc discussion is probably excessive, but we do regularly get credit for our docs: https://www.sparkfun.com/news/1316 The PostgreSQL manual is a thing of quiet beauty. I hope "quiet beauty" matches our discussion goal here. :-) -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Thu, Nov 28, 2013 at 11:04 PM, Alvaro Herrera wrote: > David Johnston wrote: > >> In all of these cases we are assuming that the user understands that >> emitting a warning means that something is being logged to disk and thus is >> causing a resource drain. >> >> I like explicitly saying that issuing these commands is pointless/"has no >> effect"; being indirect and saying that the only thing they do is emit a >> warning omits any explicit explicit explanation of why. And while I agree >> that logging the warning is an effect; but it is not the primary/direct >> effect that the user cares about. > > Honestly I still prefer what I proposed initially, which AFAICS has all > the properties you deem desirable in the wording: > > "issuing ROLLBACK outside a transaction emits a warning and otherwise has no > effect". Yeah, I still like "otherwise has no effect" or "has no other effect" best. But I can live with Bruce's latest proposal, too. I wish we'd just left this whole thing well enough alone. It wasn't broken, and didn't need fixing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
David Johnston wrote: > In all of these cases we are assuming that the user understands that > emitting a warning means that something is being logged to disk and thus is > causing a resource drain. > > I like explicitly saying that issuing these commands is pointless/"has no > effect"; being indirect and saying that the only thing they do is emit a > warning omits any explicit explicit explanation of why. And while I agree > that logging the warning is an effect; but it is not the primary/direct > effect that the user cares about. Honestly I still prefer what I proposed initially, which AFAICS has all the properties you deem desirable in the wording: "issuing ROLLBACK outside a transaction emits a warning and otherwise has no effect". -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Robert Haas wrote >> >> Issuing > > ROLLBACK > > outside of a transaction >> block has the sole effect of emitting a warning. > > Sure, that sounds OK. > > ...Robert +1 for: Issuing ROLLBACK outside of a transaction block has no effect except emitting a warning. In all of these cases we are assuming that the user understands that emitting a warning means that something is being logged to disk and thus is causing a resource drain. I like explicitly saying that issuing these commands is pointless/"has no effect"; being indirect and saying that the only thing they do is emit a warning omits any explicit explicit explanation of why. And while I agree that logging the warning is an effect; but it is not the primary/direct effect that the user cares about. I would maybe change the above to: *Issuing ROLLBACK outside of a transaction block has no effect: thus it emits a warning [to both user and log file].* I do like "thus" instead of "except" due to the explicit causality link that is establishes. We emit a warning because what you just did is pointless. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Suggestion-Issue-warning-when-calling-SET-TRANSACTION-outside-transaction-block-tp5743139p5780825.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Tue, Nov 26, 2013 at 08:54:13AM -0800, David Johnston wrote: > How about: > > "Issuing outside of a transaction has no effect and will provoke a > warning." > > I dislike "does no harm" because it can if someone thinks the current state > is different than reality. > > It is good to indicate that a warning is emitted if this is done in error; > thus reinforcing the fact people should be looking at their warnings. > > "when not inside" uses a negative modifier while "outside" is more direct > and thus easier to read, IMO. The phrase "transaction block" seems wordy so > I omitted the word "block". Every statement runs in a transaction so we can't omit "block". -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Bruce Momjian wrote >> >> - Issuing > > ABORT > > when not inside a transaction does >> >> - no harm, but it will provoke a warning message. >> >> + Issuing > > ABORT > > outside of a transaction block has no effect. >> >> >> >> Those things are not the same. >> >> > Uh, I ended up mentioning "no effect" to highlight it does nothing, >> > rather than mention a warning. Would people prefer I say "warning"? >> Or >> > should I say "issues a warning because it has no effect" or something? >> > It is easy to change. >> >> I'd revert the change Robert highlights above. ISTM you've changed the >> code to match the documentation; why would you then change the docs? > > Well, I did it to make it consistent. The question is what to write for > _all_ of the new warnings, including SET. Do we say "warning", do we > say "it has no effect", or do we say both? The ABORT is a just one case > of that. How about: "Issuing outside of a transaction has no effect and will provoke a warning." I dislike "does no harm" because it can if someone thinks the current state is different than reality. It is good to indicate that a warning is emitted if this is done in error; thus reinforcing the fact people should be looking at their warnings. "when not inside" uses a negative modifier while "outside" is more direct and thus easier to read, IMO. The phrase "transaction block" seems wordy so I omitted the word "block". David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Suggestion-Issue-warning-when-calling-SET-TRANSACTION-outside-transaction-block-tp5743139p5780378.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Tom Lane-2 wrote > David Johnston < > polobo@ > > writes: >> Robert Haas wrote >>> I don't think it's worth breaking backward compatibility. I'm not >>> entirely sure what I would have decided here in a vacuum, but at this >>> point existing precedent seems determinative. > >> Well, at this point we have already broken backward compatibility by >> releasing this. With Tom's thread necromancy I missed the fact this got >> released in 9.3 > > Uh, what? The commit I'm objecting to is certainly not in 9.3. > It's this one: > > Author: Bruce Momjian < > bruce@ > > > Branch: master [a54141aeb] 2013-10-04 13:50:28 -0400 > > Issue error on SET outside transaction block in some cases > > Issue error for SET LOCAL/CONSTRAINTS/TRANSACTION outside a > transaction > block, as they have no effect. > > Per suggestion from Morten Hustveit > > I agree that it's too late to reconsider the behavior of pre-existing > cases such as LOCK TABLE, but that doesn't mean I can't complain about > this one. My bad, I was relaying an assertion without checking it myself. I believe my source meant 9.4/head and simply mis-typed 9.3 which I then copied. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Suggestion-Issue-warning-when-calling-SET-TRANSACTION-outside-transaction-block-tp5743139p5779205.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Tue, Nov 19, 2013 at 12:24:50PM -0500, Tom Lane wrote: > David Johnston writes: > > Robert Haas wrote > >> I don't think it's worth breaking backward compatibility. I'm not > >> entirely sure what I would have decided here in a vacuum, but at this > >> point existing precedent seems determinative. > > > Well, at this point we have already broken backward compatibility by > > releasing this. With Tom's thread necromancy I missed the fact this got > > released in 9.3 > > Uh, what? The commit I'm objecting to is certainly not in 9.3. > It's this one: Right. > Author: Bruce Momjian > Branch: master [a54141aeb] 2013-10-04 13:50:28 -0400 > > Issue error on SET outside transaction block in some cases > > Issue error for SET LOCAL/CONSTRAINTS/TRANSACTION outside a transaction > block, as they have no effect. > > Per suggestion from Morten Hustveit > > I agree that it's too late to reconsider the behavior of pre-existing > cases such as LOCK TABLE, but that doesn't mean I can't complain about > this one. OK, so I just posted a summary of what we have now, and a patch that switches them all to warning. Are you saying we should just switch the new ones to warnings? Seeing as these commands have always been useless, I don't see the big argument for backward compatibility myself. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
David Johnston writes: > Robert Haas wrote >> I don't think it's worth breaking backward compatibility. I'm not >> entirely sure what I would have decided here in a vacuum, but at this >> point existing precedent seems determinative. > Well, at this point we have already broken backward compatibility by > releasing this. With Tom's thread necromancy I missed the fact this got > released in 9.3 Uh, what? The commit I'm objecting to is certainly not in 9.3. It's this one: Author: Bruce Momjian Branch: master [a54141aeb] 2013-10-04 13:50:28 -0400 Issue error on SET outside transaction block in some cases Issue error for SET LOCAL/CONSTRAINTS/TRANSACTION outside a transaction block, as they have no effect. Per suggestion from Morten Hustveit I agree that it's too late to reconsider the behavior of pre-existing cases such as LOCK TABLE, but that doesn't mean I can't complain about this one. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Tue, Nov 19, 2013 at 11:53 AM, David Johnston wrote: > Well, at this point we have already broken backward compatibility by > releasing this. With Tom's thread necromancy I missed the fact this got > released in 9.3 Eh, really? I don't see it in 9.3. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Robert Haas wrote > On Mon, Nov 18, 2013 at 9:07 PM, Bruce Momjian < > bruce@ > > wrote: >> Well, ERROR is what LOCK returns, so if we change SET TRANSACTION to be >> WARNING, we should change LOCK too, so on backward-compatibility >> grounds, ERROR makes more sense. >> >> Personally, I am fine with changing them all to WARNING. > > I don't think it's worth breaking backward compatibility. I'm not > entirely sure what I would have decided here in a vacuum, but at this > point existing precedent seems determinative. Well, at this point we have already broken backward compatibility by releasing this. With Tom's thread necromancy I missed the fact this got released in 9.3 Now, given normal upgrade realities the people likely to have this bite them probably are a ways out from upgrading so I wouldn't expect to have seen many complaints yet - but at the same time I do not recall seeing any complaints yet (limited to -bugs and -general) The referenced patch: is released is documented is consistent with precedent established by similar codepaths causes an obvious error in what is considered broken code can be trivially corrected by a user willing and able to update their application I'd say leave this as-is and only re-evaluate the decision if complaints are brought forth. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Suggestion-Issue-warning-when-calling-SET-TRANSACTION-outside-transaction-block-tp5743139p5779170.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Mon, Nov 18, 2013 at 9:07 PM, Bruce Momjian wrote: > Well, ERROR is what LOCK returns, so if we change SET TRANSACTION to be > WARNING, we should change LOCK too, so on backward-compatibility > grounds, ERROR makes more sense. > > Personally, I am fine with changing them all to WARNING. I don't think it's worth breaking backward compatibility. I'm not entirely sure what I would have decided here in a vacuum, but at this point existing precedent seems determinative. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Bruce Momjian wrote > On Mon, Nov 18, 2013 at 06:30:32PM -0800, David Johnston wrote: >> > Personally, I am fine with changing them all to WARNING. >> >> Error makes more sense if the goal is internal consistency. That goal >> should be subservient to backward compatibility. Changing LOCK to >> warning >> is less problematic since the likelihood of current code functioning in >> such >> a way that after upgrade it would begin working differently in the >> absence >> of an error does not seem probable. Basically someone would have be >> trapping on the error and conditionally branching their logic. >> >> That said, if this was a day 0 decision I'd likely raise an error. >> Weakening LOCK doesn't make sense since it is day 0 behavior. Document >> the >> warning for SET as being weaker than ideal because of backward >> compatibility >> and call it a day (i.e. leave LOCK at error). The documentation, not the >> code, then enforces the feeling that such usage is considered wrong >> without >> possibly breaking wrong but working code. > > We normally don't approach warts with documentation --- we usually just > fix them and document them in the release notes. If we did, our docs > would be a whole lot uglier. That is a fair point - though it may be that this instance needs to be one of those "usually" exceptions. For any sane use-case turning this into an error shouldn't cause any grief; and those cases where there is grief should be evaluated and changed anyway. I could honestly live with either change to "SET TRANSACTION" but regardless would leave "LOCK" as-is. The backward compatibility concern, while valid, does indeed seem weak and worth breaking in order to maintain a consistent ABI going forward. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Suggestion-Issue-warning-when-calling-SET-TRANSACTION-outside-transaction-block-tp5743139p5779028.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Mon, Nov 18, 2013 at 06:30:32PM -0800, David Johnston wrote: > > Personally, I am fine with changing them all to WARNING. > > Error makes more sense if the goal is internal consistency. That goal > should be subservient to backward compatibility. Changing LOCK to warning > is less problematic since the likelihood of current code functioning in such > a way that after upgrade it would begin working differently in the absence > of an error does not seem probable. Basically someone would have be > trapping on the error and conditionally branching their logic. > > That said, if this was a day 0 decision I'd likely raise an error. > Weakening LOCK doesn't make sense since it is day 0 behavior. Document the > warning for SET as being weaker than ideal because of backward compatibility > and call it a day (i.e. leave LOCK at error). The documentation, not the > code, then enforces the feeling that such usage is considered wrong without > possibly breaking wrong but working code. We normally don't approach warts with documentation --- we usually just fix them and document them in the release notes. If we did, our docs would be a whole lot uglier. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Bruce Momjian wrote > On Mon, Nov 18, 2013 at 05:05:45PM -0800, David Johnston wrote: >> Bruce Momjian wrote >> > Considering we are doing this outside of a transaction, and WARNING or >> > ERROR is pretty much the same, from a behavioral perspective. >> > >> > Should we change this and LOCK to be a warning? >> >> >From the calling application's perspective an error and a warning are >> definitely behaviorally different. >> >> For this I'd vote for a warning (haven't pondered the LOCK scenario) as >> using SET out of context means the user has a fairly serious >> mis-understanding of the code path they have written (accedentially or >> otherwise). Notice makes sense (speaking generally and without much >> research here) for stuff where the ultimate outcome matches the statement >> but the statement itself didn't actually do anything. Auto-sequence and >> index generation fell into this but even notice was too noisy. In this >> case >> we'd expect that the no-op statement was issued in error and thus should >> be >> changed making a warning the level of incorrect-ness to communicate. A >> notice would be more appropriate if there were valid use-cases for the >> user >> doing this and we just want to make sure they are conscious of the >> unusualness of the situation. >> >> I dislike error for backward compatibility reasons. And saving the user >> from this kind of mistake doesn't warrant breaking what could be properly >> functioning code. Just because PostgreSQL isn't in a transaction does >> not >> mean the client is expecting the current code to work correctly - even if >> by >> accident - as part of a sequence of queries. > > Well, ERROR is what LOCK returns, so if we change SET TRANSACTION to be > WARNING, we should change LOCK too, so on backward-compatibility > grounds, ERROR makes more sense. > > Personally, I am fine with changing them all to WARNING. Error makes more sense if the goal is internal consistency. That goal should be subservient to backward compatibility. Changing LOCK to warning is less problematic since the likelihood of current code functioning in such a way that after upgrade it would begin working differently in the absence of an error does not seem probable. Basically someone would have be trapping on the error and conditionally branching their logic. That said, if this was a day 0 decision I'd likely raise an error. Weakening LOCK doesn't make sense since it is day 0 behavior. Document the warning for SET as being weaker than ideal because of backward compatibility and call it a day (i.e. leave LOCK at error). The documentation, not the code, then enforces the feeling that such usage is considered wrong without possibly breaking wrong but working code. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Suggestion-Issue-warning-when-calling-SET-TRANSACTION-outside-transaction-block-tp5743139p5779006.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Mon, Nov 18, 2013 at 05:05:45PM -0800, David Johnston wrote: > Bruce Momjian wrote > > Considering we are doing this outside of a transaction, and WARNING or > > ERROR is pretty much the same, from a behavioral perspective. > > > > Should we change this and LOCK to be a warning? > > >From the calling application's perspective an error and a warning are > definitely behaviorally different. > > For this I'd vote for a warning (haven't pondered the LOCK scenario) as > using SET out of context means the user has a fairly serious > mis-understanding of the code path they have written (accedentially or > otherwise). Notice makes sense (speaking generally and without much > research here) for stuff where the ultimate outcome matches the statement > but the statement itself didn't actually do anything. Auto-sequence and > index generation fell into this but even notice was too noisy. In this case > we'd expect that the no-op statement was issued in error and thus should be > changed making a warning the level of incorrect-ness to communicate. A > notice would be more appropriate if there were valid use-cases for the user > doing this and we just want to make sure they are conscious of the > unusualness of the situation. > > I dislike error for backward compatibility reasons. And saving the user > from this kind of mistake doesn't warrant breaking what could be properly > functioning code. Just because PostgreSQL isn't in a transaction does not > mean the client is expecting the current code to work correctly - even if by > accident - as part of a sequence of queries. Well, ERROR is what LOCK returns, so if we change SET TRANSACTION to be WARNING, we should change LOCK too, so on backward-compatibility grounds, ERROR makes more sense. Personally, I am fine with changing them all to WARNING. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Bruce Momjian wrote > Considering we are doing this outside of a transaction, and WARNING or > ERROR is pretty much the same, from a behavioral perspective. > > Should we change this and LOCK to be a warning? >From the calling application's perspective an error and a warning are definitely behaviorally different. For this I'd vote for a warning (haven't pondered the LOCK scenario) as using SET out of context means the user has a fairly serious mis-understanding of the code path they have written (accedentially or otherwise). Notice makes sense (speaking generally and without much research here) for stuff where the ultimate outcome matches the statement but the statement itself didn't actually do anything. Auto-sequence and index generation fell into this but even notice was too noisy. In this case we'd expect that the no-op statement was issued in error and thus should be changed making a warning the level of incorrect-ness to communicate. A notice would be more appropriate if there were valid use-cases for the user doing this and we just want to make sure they are conscious of the unusualness of the situation. I dislike error for backward compatibility reasons. And saving the user from this kind of mistake doesn't warrant breaking what could be properly functioning code. Just because PostgreSQL isn't in a transaction does not mean the client is expecting the current code to work correctly - even if by accident - as part of a sequence of queries. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Suggestion-Issue-warning-when-calling-SET-TRANSACTION-outside-transaction-block-tp5743139p5778994.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
