Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-28 Thread Robert Haas
On Wed, Nov 27, 2013 at 4:44 PM, Bruce Momjian br...@momjian.us wrote: Seems broadly reasonable, but I'd use no other effect throughout. That sounds awkward, e.g.: Issuing commandROLLBACK/ outside of a transaction block emits a warning but has no other effect. I could live with

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-28 Thread Bruce Momjian
On Thu, Nov 28, 2013 at 04:51:14PM -0500, Robert Haas wrote: On Wed, Nov 27, 2013 at 4:44 PM, Bruce Momjian br...@momjian.us wrote: Seems broadly reasonable, but I'd use no other effect throughout. That sounds awkward, e.g.: Issuing commandROLLBACK/ outside of a transaction

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-28 Thread Robert Haas
On Nov 28, 2013, at 5:18 PM, Bruce Momjian br...@momjian.us wrote: On Thu, Nov 28, 2013 at 04:51:14PM -0500, Robert Haas wrote: On Wed, Nov 27, 2013 at 4:44 PM, Bruce Momjian br...@momjian.us wrote: Seems broadly reasonable, but I'd use no other effect throughout. That sounds awkward, e.g.:

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-27 Thread Robert Haas
On Tue, Nov 26, 2013 at 5:02 PM, Bruce Momjian br...@momjian.us wrote: On Tue, Nov 26, 2013 at 01:58:04PM -0300, Alvaro Herrera wrote: Bruce Momjian escribió: On Tue, Nov 26, 2013 at 11:22:39AM -0500, Tom Lane wrote: Uh, I ended up mentioning no effect to highlight it does nothing,

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-27 Thread Bruce Momjian
On Wed, Nov 27, 2013 at 03:59:31PM -0500, Robert Haas wrote: On Tue, Nov 26, 2013 at 5:02 PM, Bruce Momjian br...@momjian.us wrote: On Tue, Nov 26, 2013 at 01:58:04PM -0300, Alvaro Herrera wrote: Bruce Momjian escribió: On Tue, Nov 26, 2013 at 11:22:39AM -0500, Tom Lane wrote: Uh, I

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-27 Thread Bruce Momjian
On Wed, Nov 27, 2013 at 04:44:02PM -0500, Bruce Momjian wrote: I could live with this: Issuing commandROLLBACK/ outside of a transaction block has no effect except emitting a warning. Proposed doc patch attached. -- Bruce Momjian br...@momjian.ushttp://momjian.us

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-26 Thread Bruce Momjian
On Mon, Nov 25, 2013 at 10:12:43PM -0500, Bruce Momjian wrote: 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

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-26 Thread Tom Lane
Bruce Momjian br...@momjian.us writes: On Mon, Nov 25, 2013 at 10:04:19PM -0500, Robert Haas wrote: But the documentation says: - Issuing commandABORT/ when not inside a transaction does - no harm, but it will provoke a warning message. + Issuing commandABORT/ outside of a transaction

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-26 Thread Bruce Momjian
On Tue, Nov 26, 2013 at 11:22:39AM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Mon, Nov 25, 2013 at 10:04:19PM -0500, Robert Haas wrote: But the documentation says: - Issuing commandABORT/ when not inside a transaction does - no harm, but it will provoke a

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-26 Thread Alvaro Herrera
Bruce Momjian escribió: On Tue, Nov 26, 2013 at 11:22:39AM -0500, Tom Lane wrote: 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?

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-26 Thread Bruce Momjian
On Tue, Nov 26, 2013 at 01:58:04PM -0300, Alvaro Herrera wrote: Bruce Momjian escribió: On Tue, Nov 26, 2013 at 11:22:39AM -0500, Tom Lane wrote: Uh, I ended up mentioning no effect to highlight it does nothing, rather than mention a warning. Would people prefer I say warning? Or

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-25 Thread Bruce Momjian
On Fri, Nov 22, 2013 at 01:19:55PM -0500, Bruce Momjian wrote: On Fri, Nov 22, 2013 at 12:17:41PM -0500, Bruce Momjian wrote: Good points. I have modified the attached patch to do as you suggested. Also, I have read through the thread and summarized the positions of the posters:

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-25 Thread Robert Haas
On Mon, Nov 25, 2013 at 7:19 PM, Bruce Momjian br...@momjian.us wrote: On Fri, Nov 22, 2013 at 01:19:55PM -0500, Bruce Momjian wrote: On Fri, Nov 22, 2013 at 12:17:41PM -0500, Bruce Momjian wrote: Good points. I have modified the attached patch to do as you suggested. Also, I have read

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-25 Thread Bruce Momjian
On Mon, Nov 25, 2013 at 10:04:19PM -0500, Robert Haas wrote: On Mon, Nov 25, 2013 at 7:19 PM, Bruce Momjian br...@momjian.us wrote: On Fri, Nov 22, 2013 at 01:19:55PM -0500, Bruce Momjian wrote: On Fri, Nov 22, 2013 at 12:17:41PM -0500, Bruce Momjian wrote: Good points. I have modified

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-22 Thread Alvaro Herrera
Bruce Momjian escribió: OK, here is a patch which changes ABORT from NOTICE to WARNING, and SET from ERROR (which is new in 9.4) to WARNING. I don't like that this patch changes RequireTransactionChain() from actually requiring one, to a function that maybe requires a transaction chain, and

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-22 Thread Bruce Momjian
On Fri, Nov 22, 2013 at 10:24:35AM -0300, Alvaro Herrera wrote: Bruce Momjian escribió: OK, here is a patch which changes ABORT from NOTICE to WARNING, and SET from ERROR (which is new in 9.4) to WARNING. I don't like that this patch changes RequireTransactionChain() from actually

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-22 Thread Bruce Momjian
On Fri, Nov 22, 2013 at 12:17:41PM -0500, Bruce Momjian wrote: Good points. I have modified the attached patch to do as you suggested. Also, I have read through the thread and summarized the positions of the posters: 9.3 WARNING ERROR SET

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-20 Thread Bruce Momjian
On Tue, Nov 19, 2013 at 10:21:47PM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Does anyone know if this C comment justifies why ABORT is a NOTICE and not WARNING? /* * The user issued ABORT when not inside a transaction. Issue a

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-20 Thread Tom Lane
Bruce Momjian br...@momjian.us writes: On Tue, Nov 19, 2013 at 10:21:47PM -0500, Tom Lane wrote: My personal standpoint is that I don't care much whether these messages are NOTICE or WARNING. What I'm not happy about is promoting cases that have been non-error conditions for years into

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-20 Thread Bruce Momjian
On Wed, Nov 20, 2013 at 10:04:22AM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Tue, Nov 19, 2013 at 10:21:47PM -0500, Tom Lane wrote: My personal standpoint is that I don't care much whether these messages are NOTICE or WARNING. What I'm not happy about is promoting

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-20 Thread Bruce Momjian
On Wed, Nov 20, 2013 at 10:16:00AM -0500, Bruce Momjian wrote: The attached patch changes ABORT from NOTICE to WARNING, and documents that all other are errors. This top-level logic idea came from Robert Haas, and it has some level of consistency. This patch utterly fails to address

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-20 Thread Robert Haas
On Wed, Nov 20, 2013 at 4:19 PM, Bruce Momjian br...@momjian.us wrote: On Wed, Nov 20, 2013 at 10:16:00AM -0500, Bruce Momjian wrote: The attached patch changes ABORT from NOTICE to WARNING, and documents that all other are errors. This top-level logic idea came from Robert Haas, and it

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-20 Thread Bruce Momjian
On Wed, Nov 20, 2013 at 04:31:12PM -0500, Robert Haas wrote: On Wed, Nov 20, 2013 at 4:19 PM, Bruce Momjian br...@momjian.us wrote: On Wed, Nov 20, 2013 at 10:16:00AM -0500, Bruce Momjian wrote: The attached patch changes ABORT from NOTICE to WARNING, and documents that all other are

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-20 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote: Well, Tom and I are on opposite sides of this, I suppose.  I prefer ERROR for everything other than the top-level transaction commands, and see no benefit from opting for a wishy-washy warning. +1 If the user issued a local command outside of a

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Bruce Momjian
On Sat, Nov 9, 2013 at 02:15:52PM -0500, Robert Haas wrote: On Fri, Nov 8, 2013 at 5:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: [ I'm so far behind ... ] Bruce Momjian br...@momjian.us writes: Applied. Thank you for all your suggestions. I thought the suggestion had been to issue a

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Andres Freund
On 2013-11-19 13:05:01 -0500, Bruce Momjian wrote: SAVEPOINT test= ROLLBACK TO SAVEPOINT asdf; ERROR: ROLLBACK TO SAVEPOINT can only be used in transaction blocks Notice that they do _not_ check their arguments; they just throw errors. With this patch they issue

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Bruce Momjian
On Tue, Nov 19, 2013 at 07:08:05PM +0100, Andres Freund wrote: On 2013-11-19 13:05:01 -0500, Bruce Momjian wrote: SAVEPOINT test= ROLLBACK TO SAVEPOINT asdf; ERROR: ROLLBACK TO SAVEPOINT can only be used in transaction blocks Notice that they do _not_ check their

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Robert Haas
On Tue, Nov 19, 2013 at 1:05 PM, Bruce Momjian br...@momjian.us wrote: A patch to issue only warnings is attached. In a way this change improves the code by throwing errors only when the commands are invalid, rather than just useless. You could argue that ROLLBACK TO SAVEPOINT should throw

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Andres Freund
On 2013-11-19 13:09:16 -0500, Bruce Momjian wrote: Why change the historical behaviour for savepoints? Because as Tom stated, we already do warnings for other useless transaction commands like BEGIN WORK inside a transaction block: Which imo is a bad, bad historical accident. I've

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Bruce Momjian
On Tue, Nov 19, 2013 at 07:12:32PM +0100, Andres Freund wrote: On 2013-11-19 13:09:16 -0500, Bruce Momjian wrote: Why change the historical behaviour for savepoints? Because as Tom stated, we already do warnings for other useless transaction commands like BEGIN WORK inside a transaction

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Andres Freund
On 2013-11-19 13:14:34 -0500, Bruce Momjian wrote: On Tue, Nov 19, 2013 at 07:12:32PM +0100, Andres Freund wrote: But even if that weren't a concern, the fact that BEGIN does it one way currently doesn't seem very indicative of changing other historical behaviour. Look at this gem,

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Robert Haas
On Tue, Nov 19, 2013 at 1:14 PM, Bruce Momjian br...@momjian.us wrote: On Tue, Nov 19, 2013 at 07:12:32PM +0100, Andres Freund wrote: On 2013-11-19 13:09:16 -0500, Bruce Momjian wrote: Why change the historical behaviour for savepoints? Because as Tom stated, we already do warnings for

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Bruce Momjian
On Tue, Nov 19, 2013 at 01:20:47PM -0500, Robert Haas wrote: I think the pattern is and should be different for toplevel transaction control commands than for other things. If you issue a BEGIN, we want it to end up that you're definitely in a transaction at that point, and if you issue a

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Bruce Momjian
On Tue, Nov 19, 2013 at 01:31:55PM -0500, Bruce Momjian wrote: On Tue, Nov 19, 2013 at 01:20:47PM -0500, Robert Haas wrote: I think the pattern is and should be different for toplevel transaction control commands than for other things. If you issue a BEGIN, we want it to end up that you're

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Bruce Momjian
On Tue, Nov 19, 2013 at 01:37:56PM -0500, Bruce Momjian wrote: On Tue, Nov 19, 2013 at 01:31:55PM -0500, Bruce Momjian wrote: On Tue, Nov 19, 2013 at 01:20:47PM -0500, Robert Haas wrote: I think the pattern is and should be different for toplevel transaction control commands than for

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Dimitri Fontaine
Andres Freund and...@2ndquadrant.com writes: On 2013-11-19 13:09:16 -0500, Bruce Momjian wrote: Because as Tom stated, we already do warnings for other useless transaction commands like BEGIN WORK inside a transaction block: Which imo is a bad, bad historical accident. I've repeatedly seen

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-19 Thread Tom Lane
Bruce Momjian br...@momjian.us writes: Does anyone know if this C comment justifies why ABORT is a NOTICE and not WARNING? /* * The user issued ABORT when not inside a transaction. Issue a * NOTICE and go to abort state. The upcoming call to

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-18 Thread Bruce Momjian
On Sat, Nov 9, 2013 at 02:15:52PM -0500, Robert Haas wrote: On Fri, Nov 8, 2013 at 5:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: [ I'm so far behind ... ] Bruce Momjian br...@momjian.us writes: Applied. Thank you for all your suggestions. I thought the suggestion had been to issue a

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-09 Thread Robert Haas
On Fri, Nov 8, 2013 at 5:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: [ I'm so far behind ... ] Bruce Momjian br...@momjian.us writes: Applied. Thank you for all your suggestions. I thought the suggestion had been to issue a *warning*. How did that become an error? This patch seems likely

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-11-08 Thread Tom Lane
[ I'm so far behind ... ] Bruce Momjian br...@momjian.us writes: Applied. Thank you for all your suggestions. I thought the suggestion had been to issue a *warning*. How did that become an error? This patch seems likely to break applications that may have just been harmlessly sloppy about

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-10-04 Thread Bruce Momjian
On Fri, Oct 4, 2013 at 09:40:38AM +0530, Amit Kapila wrote: On Thu, Oct 3, 2013 at 8:35 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-09-30 22:19:31 -0400, Bruce Momjian wrote: On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote: Shouldn't we do it for Set Constraints

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-10-03 Thread Amit Kapila
On Tue, Oct 1, 2013 at 7:49 AM, Bruce Momjian br...@momjian.us wrote: On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote: Shouldn't we do it for Set Constraints as well? Oh, very good point. I missed that one. Updated patch attached. I am glad you are seeing things I am not.

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-10-03 Thread Bruce Momjian
On Thu, Oct 3, 2013 at 11:50:09AM +0530, Amit Kapila wrote: I looked at this but could not see how to easily pass the value of 'isTopLevel' down to the SELECT. All the other checks have isTopLevel passed down from the utility case statement. Yes, we cannot pass isTopLevel, but as

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-10-03 Thread Andres Freund
On 2013-09-30 22:19:31 -0400, Bruce Momjian wrote: On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote: Shouldn't we do it for Set Constraints as well? Oh, very good point. I missed that one. Updated patch attached. I am glad you are seeing things I am not. :-) 1. The

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-10-03 Thread Amit Kapila
On Thu, Oct 3, 2013 at 8:35 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-09-30 22:19:31 -0400, Bruce Momjian wrote: On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote: Shouldn't we do it for Set Constraints as well? Oh, very good point. I missed that one. Updated

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-10-03 Thread Amit Kapila
On Thu, Oct 3, 2013 at 8:32 PM, Bruce Momjian br...@momjian.us wrote: On Thu, Oct 3, 2013 at 11:50:09AM +0530, Amit Kapila wrote: I looked at this but could not see how to easily pass the value of 'isTopLevel' down to the SELECT. All the other checks have isTopLevel passed down from the

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-09-30 Thread Bruce Momjian
On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote: Shouldn't we do it for Set Constraints as well? Oh, very good point. I missed that one. Updated patch attached. I am glad you are seeing things I am not. :-) 1. The function set_config also needs similar functionality, else

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-09-29 Thread Amit Kapila
On Wed, Sep 25, 2013 at 2:55 AM, Bruce Momjian br...@momjian.us wrote: On Thu, Sep 12, 2013 at 09:38:43AM +0530, Amit Kapila wrote: I have created the attached patch which issues an error when SET TRANSACTION and SET LOCAL are used outside of transactions: test= set transaction

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-09-24 Thread Bruce Momjian
On Thu, Sep 12, 2013 at 09:38:43AM +0530, Amit Kapila wrote: I have created the attached patch which issues an error when SET TRANSACTION and SET LOCAL are used outside of transactions: test= set transaction isolation level serializable; ERROR: SET TRANSACTION can only

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-09-11 Thread Amit Kapila
On Wed, Sep 11, 2013 at 4:19 AM, Bruce Momjian br...@momjian.us wrote: On Sun, Feb 3, 2013 at 07:19:14AM +, Amit kapila wrote: On Saturday, February 02, 2013 9:08 PM Robert Haas wrote: On Fri, Feb 1, 2013 at 12:04 AM, Amit Kapila amit.kap...@huawei.com wrote: I think user should be

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-09-10 Thread Bruce Momjian
On Sun, Feb 3, 2013 at 07:19:14AM +, Amit kapila wrote: On Saturday, February 02, 2013 9:08 PM Robert Haas wrote: On Fri, Feb 1, 2013 at 12:04 AM, Amit Kapila amit.kap...@huawei.com wrote: I think user should be aware of effect before using SET commands, as these are used at various

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-02-02 Thread Robert Haas
On Fri, Feb 1, 2013 at 12:04 AM, Amit Kapila amit.kap...@huawei.com wrote: I think user should be aware of effect before using SET commands, as these are used at various levels (TRANSACTION, SESSION, ...). Ideally, sure. But these kinds of mistakes are easy to make. That's why LOCK and

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-02-02 Thread Amit kapila
On Saturday, February 02, 2013 9:08 PM Robert Haas wrote: On Fri, Feb 1, 2013 at 12:04 AM, Amit Kapila amit.kap...@huawei.com wrote: I think user should be aware of effect before using SET commands, as these are used at various levels (TRANSACTION, SESSION, ...). Ideally, sure. But these

Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block

2013-01-31 Thread Amit Kapila
On Wednesday, January 30, 2013 6:53 AM Morten Hustveit wrote: Hi! Calling SET TRANSACTION ISOLATION LEVEL ... outside a transaction block has no effect. This is unlike LOCK ... and DECLARE foo CURSOR FOR ..., which both raise an error. This is also unlike MySQL, where such a statement