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 alvhe...@2ndquadrant.com 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 br...@momjian.ushttp://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 br...@momjian.ushttp://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: On Thu, Nov 28, 2013 at 11:04 PM, Alvaro Herrera alvhe...@2ndquadrant.com 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 br...@momjian.ushttp://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
alvhe...@2ndquadrant.com 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 br...@momjian.ushttp://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
/para
para
!Issuing commandABORT/ outside of a transaction block has no effect.
/para
/refsect1
--- 63,70
/para
para
!Issuing commandABORT/ outside of a transaction block
!emits a warning and otherwise has no effect.
/para
/refsect1
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
para
Issuing commandROLLBACK/ outside of a transaction
!block has no effect.
/para
/refsect1
--- 60,66
para
Issuing commandROLLBACK/ outside of a transaction
!block emits a warning and otherwise has no effect.
/para
/refsect1
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 { rep
*** 110,117
para
Specifies that the command takes effect for only the current
transaction. After commandCOMMIT/ or commandROLLBACK/,
! the session-level setting takes effect again. This has no effect
! outside of a transaction block.
/para
/listitem
/varlistentry
--- 110,118
para
Specifies that the command takes effect for only the current
transaction. After commandCOMMIT/ or commandROLLBACK/,
! the session-level setting takes effect again. Issuing this
! outside of a transaction block emits a warning and otherwise has
! no effect.
/para
/listitem
/varlistentry
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 | replaceable cla
*** 99,105
para
This command only alters the behavior of constraints within the
!current transaction. This has no effect outside of a transaction block.
/para
/refsect1
--- 99,106
para
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.
/para
/refsect1
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
para
If commandSET TRANSACTION/command is executed without a prior
commandSTART TRANSACTION/command or commandBEGIN/command,
!it will have no effect.
/para
para
--- 185,191
para
If commandSET TRANSACTION/command is executed without a prior
commandSTART TRANSACTION/command or commandBEGIN/command,
!it emits a warning and otherwise has no effect.
/para
para
--
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 command ROLLBACK / outside of a transaction block has the sole effect of emitting a warning. Sure, that sounds OK. ...Robert +1 for: Issuing commandROLLBACK/ 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 commandROLLBACK/ 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
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
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Thu, Nov 28, 2013 at 11:04 PM, Alvaro Herrera alvhe...@2ndquadrant.com 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
[HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Bruce Momjian wrote - Issuing command ABORT / when not inside a transaction does - no harm, but it will provoke a warning message. + Issuing command 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 command 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
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 command 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 br...@momjian.ushttp://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 Mon, Nov 18, 2013 at 9:07 PM, Bruce Momjian br...@momjian.us 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
Robert Haas wrote On Mon, Nov 18, 2013 at 9:07 PM, Bruce Momjian lt; bruce@ gt; 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 Tue, Nov 19, 2013 at 11:53 AM, David Johnston pol...@yahoo.com 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
Re: [HACKERS] Re: Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
David Johnston pol...@yahoo.com 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 br...@momjian.us 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 12:24:50PM -0500, Tom Lane wrote: David Johnston pol...@yahoo.com 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 br...@momjian.us 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 br...@momjian.ushttp://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
Tom Lane-2 wrote David Johnston lt; polobo@ gt; 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 lt; bruce@ gt; 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
[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
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 br...@momjian.ushttp://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 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 br...@momjian.ushttp://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 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
