Re: [HACKERS] Assertions in PL/PgSQL
2013/11/27 Peter Eisentraut pete...@gmx.net On Tue, 2013-11-19 at 10:40 -0500, Robert Haas wrote: I think the goal was to get to RAISE ASSERT WHEN ...; then, if assertions are off, you do nothing; if they're on, you error. IF condition THEN RAISE... isn't a suitable surrogate in that case because you incur the overhead of testing the condition regardless. So if I do RAISE ASSERT WHEN condition and assertions are off, then condition wouldn't even be evaluated? But what about RAISE NOTICE WHEN, when log_min_messages is error? What about the side effects of the format string? This is all just getting too weird. I don't see anything wrong with considering a separate ASSERT command with its own semantics, like in many other programming languages. My objection against ASSERT command was one - it was too simply (against to cost of possible collision from introduction new (wide used) keyword. I can live with ASSERT statement - but I expect as minimum a possibility to specify level (failure, tracing, ...) and specify a message related to assert. Assert with only expression is not enough. Regards Pavel
Re: [HACKERS] Assertions in PL/PgSQL
On Tue, 2013-11-19 at 10:40 -0500, Robert Haas wrote: I think the goal was to get to RAISE ASSERT WHEN ...; then, if assertions are off, you do nothing; if they're on, you error. IF condition THEN RAISE... isn't a suitable surrogate in that case because you incur the overhead of testing the condition regardless. So if I do RAISE ASSERT WHEN condition and assertions are off, then condition wouldn't even be evaluated? But what about RAISE NOTICE WHEN, when log_min_messages is error? What about the side effects of the format string? This is all just getting too weird. I don't see anything wrong with considering a separate ASSERT command with its own semantics, like in many other programming languages. -- 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] Assertions in PL/PgSQL
On Sun, Nov 17, 2013 at 5:10 PM, Tom Lane t...@sss.pgh.pa.us wrote: Pavel Stehule pavel.steh...@gmail.com writes: [ rebased patch for RAISE WHEN ] I have to say I do not see the point of this. It does nothing you can't do already with IF condition THEN RAISE And frankly the RAISE statement has got too darn many options already. We don't need yet more cruft on it that we'll have to maintain forevermore. If this were improving standards compliance somehow, I'd be okay with it; but what other implementation has got this? This is a fair point. I think the goal was to get to RAISE ASSERT WHEN ...; then, if assertions are off, you do nothing; if they're on, you error. IF condition THEN RAISE... isn't a suitable surrogate in that case because you incur the overhead of testing the condition regardless. Now that having been said, I'm a bit wary of adding every new frammish someone suggests to PL/pgsql. Many of the things we've added recently are things I anticipate that I'll never use. -- 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] Assertions in PL/PgSQL
2013/11/19 Robert Haas robertmh...@gmail.com On Sun, Nov 17, 2013 at 5:10 PM, Tom Lane t...@sss.pgh.pa.us wrote: Pavel Stehule pavel.steh...@gmail.com writes: [ rebased patch for RAISE WHEN ] I have to say I do not see the point of this. It does nothing you can't do already with IF condition THEN RAISE And frankly the RAISE statement has got too darn many options already. We don't need yet more cruft on it that we'll have to maintain forevermore. If this were improving standards compliance somehow, I'd be okay with it; but what other implementation has got this? This is a fair point. I think the goal was to get to RAISE ASSERT WHEN ...; then, if assertions are off, you do nothing; if they're on, you error. IF condition THEN RAISE... isn't a suitable surrogate in that case because you incur the overhead of testing the condition regardless. Now that having been said, I'm a bit wary of adding every new frammish someone suggests to PL/pgsql. Many of the things we've added recently are things I anticipate that I'll never use. lot of features are popular with some delay. CTE is very popular now, and two years ago only few developers used it. Lot of applications are developed for 9.1 still. Regards Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] Assertions in PL/PgSQL
On Tue, Nov 19, 2013 at 10:59 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Now that having been said, I'm a bit wary of adding every new frammish someone suggests to PL/pgsql. Many of the things we've added recently are things I anticipate that I'll never use. lot of features are popular with some delay. CTE is very popular now, and two years ago only few developers used it. Lot of applications are developed for 9.1 still. I think that's true, but not particularly relevant. CTEs are obviously a major feature; a lot of the stuff we've been adding to PL/pgsql is tinkering around the edges. -- 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] Assertions in PL/PgSQL
2013/11/19 Robert Haas robertmh...@gmail.com On Tue, Nov 19, 2013 at 10:59 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Now that having been said, I'm a bit wary of adding every new frammish someone suggests to PL/pgsql. Many of the things we've added recently are things I anticipate that I'll never use. lot of features are popular with some delay. CTE is very popular now, and two years ago only few developers used it. Lot of applications are developed for 9.1 still. I think that's true, but not particularly relevant. CTEs are obviously a major feature; a lot of the stuff we've been adding to PL/pgsql is tinkering around the edges. I agree so almost all last features are not major features - but I don't think so it is wrong (and structured exception is not minor feature). Almost all work is done. There are only few issues, that should be solved: * deeper checking embedded SQL * more robust work with nested types - assign statement * some support for large and complex projects (support for developer tools like coverage calculation, dependency graphs and assertions) Regards Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] Assertions in PL/PgSQL
Pavel Stehule pavel.steh...@gmail.com writes: [ rebased patch for RAISE WHEN ] I have to say I do not see the point of this. It does nothing you can't do already with IF condition THEN RAISE And frankly the RAISE statement has got too darn many options already. We don't need yet more cruft on it that we'll have to maintain forevermore. If this were improving standards compliance somehow, I'd be okay with it; but what other implementation has got this? 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] Assertions in PL/PgSQL
2013/11/17 Tom Lane t...@sss.pgh.pa.us Pavel Stehule pavel.steh...@gmail.com writes: [ rebased patch for RAISE WHEN ] I have to say I do not see the point of this. It does nothing you can't do already with IF condition THEN RAISE And frankly the RAISE statement has got too darn many options already. We don't need yet more cruft on it that we'll have to maintain forevermore. If this were improving standards compliance somehow, I'd be okay with it; but what other implementation has got this? RAISE statement is not ANSI compliant ever, and it has only thin similarity with Oracle' PL/SQL RAISE statement now - and it is significantly enhanced in relation to original ADA Usually I am not a happy, when PL/pgSQL going far from original ADA, but I think so this use case is very practical current usual pattern is less readable than conditional RAISE It is similar to CONTINUE and EXIST statement. Actually we need a some functionality, that allows simply write assertions (without custom source code uglyfication). RAISE WHEN is good for this purpose. Regards Pavel regards, tom lane
Re: [HACKERS] Assertions in PL/PgSQL
rebased patch
Regards
Pavel
2013/11/14 Peter Eisentraut pete...@gmx.net
On Wed, 2013-10-09 at 18:57 +0200, Pavel Stehule wrote:
here is a patch for RAISE WHEN clause
Your patch needs to be rebased.
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index ca2c2b5..d6845d7 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1753,9 +1753,7 @@ BEGIN
-- Since execution is not finished, we can check whether rows were returned
-- and raise exception if not.
-IF NOT FOUND THEN
-RAISE EXCEPTION 'No flight at %.', $1;
-END IF;
+RAISE EXCEPTION 'No flight at %.', $1 WHEN NOT FOUND;
RETURN;
END
@@ -3376,11 +3374,11 @@ END LOOP optional replaceablelabel/replaceable /optional;
raise errors.
synopsis
-RAISE optional replaceable class=parameterlevel/replaceable /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
-RAISE optional replaceable class=parameterlevel/replaceable /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
-RAISE optional replaceable class=parameterlevel/replaceable /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
-RAISE optional replaceable class=parameterlevel/replaceable /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional;
-RAISE ;
+RAISE optional replaceable class=parameterlevel/replaceable /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional optional WHEN replaceableboolean-expression/replaceable /optional ;
+RAISE optional replaceable class=parameterlevel/replaceable /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional optional WHEN replaceableboolean-expression/replaceable /optional;
+RAISE optional replaceable class=parameterlevel/replaceable /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional optional WHEN replaceableboolean-expression/replaceable /optional;
+RAISE optional replaceable class=parameterlevel/replaceable /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional optional WHEN replaceableboolean-expression/replaceable /optional;
+RAISE optional WHEN replaceableboolean-expression/replaceable /optional ;
/synopsis
The replaceable class=parameterlevel/replaceable option specifies
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index bc31fe9..edb6105 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2874,6 +2874,20 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
char *err_schema = NULL;
ListCell *lc;
+ /* check condition when is entered */
+ if (stmt-cond != NULL)
+ {
+ bool value;
+ bool isnull;
+
+ value = exec_eval_boolean(estate, stmt-cond, isnull);
+ exec_eval_cleanup(estate);
+
+ /* ignore statement, when result of condition is false or NULL */
+ if (isnull || value == false)
+ return PLPGSQL_RC_OK;
+ }
+
/* RAISE with no parameters: re-throw current exception */
if (stmt-condname == NULL stmt-message == NULL
stmt-options == NIL)
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index f112282..a4d7035 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -63,6 +63,7 @@ static void current_token_is_not_variable(int tok);
static PLpgSQL_expr *read_sql_construct(int until,
int until2,
int until3,
+ int until4,
const char *expected,
const char *sqlstart,
bool isexpression,
@@ -105,7 +106,7 @@ static void check_labels(const char *start_label,
int end_location);
static PLpgSQL_expr *read_cursor_args(PLpgSQL_var *cursor,
int until, const char *expected);
-static List *read_raise_options(void);
+static List *read_raise_options(int *tok);
%}
@@ -1386,6 +1387,7 @@ for_control :
Re: [HACKERS] Assertions in PL/PgSQL
On Wed, 2013-10-09 at 18:57 +0200, Pavel Stehule wrote: here is a patch for RAISE WHEN clause Your patch needs to be rebased. -- 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] Assertions in PL/PgSQL
Hello 2013/9/24 Robert Haas robertmh...@gmail.com On Mon, Sep 23, 2013 at 5:48 AM, Amit Khandekar amit.khande...@enterprisedb.com wrote: The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; RAISE [ ASSERT ] WHEN bool_expression; RAISE ; I don't think so it is a good idea. WHEN clause should be independent on exception level. I am ok with generalizing the WHEN clause across all levels. The main proposal was for adding assertion support, so we can keep the WHEN generalization as a nice-to-have stuff and do it only if it comes as a natural extension in the assertion support patch. I think that's right: ISTM that at this point there are two different proposals here. 1. Allowing ASSERT as an argument to RAISE. 2. Allowing RAISE to have a WHEN clause. Those two things are logically separate. We could do either one without doing the other one. here is a patch for RAISE WHEN clause Regards Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company plpgsql-raise-when_v1_20131009.patch Description: Binary data -- 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] Assertions in PL/PgSQL
On Wed, Oct 9, 2013 at 12:57 PM, Pavel Stehule pavel.steh...@gmail.com wrote: here is a patch for RAISE WHEN clause This is in effect a whole new patch by a different author. Please submit it to the next CommitFest; I'm marking the entry for Assertions in PL/PgSQL as Returned with Feedback. -- 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] Assertions in PL/PgSQL
On Mon, Sep 23, 2013 at 5:48 AM, Amit Khandekar amit.khande...@enterprisedb.com wrote: The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; RAISE [ ASSERT ] WHEN bool_expression; RAISE ; I don't think so it is a good idea. WHEN clause should be independent on exception level. I am ok with generalizing the WHEN clause across all levels. The main proposal was for adding assertion support, so we can keep the WHEN generalization as a nice-to-have stuff and do it only if it comes as a natural extension in the assertion support patch. I think that's right: ISTM that at this point there are two different proposals here. 1. Allowing ASSERT as an argument to RAISE. 2. Allowing RAISE to have a WHEN clause. Those two things are logically separate. We could do either one without doing the other one. -- 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] Assertions in PL/PgSQL
On 9/23/13 6:40 AM, Pavel Stehule wrote: After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression It looks like I'm losing this battle, but this syntax isn't too bad. and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] This sounds like a decent enhancement. p.s. clause WHEN can be used for other exception level - so it can be a interesting shortcut for other use cases. This idea is good, I like it. I could prepare a patch for this, unless someone else wants to? Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
2013/9/23 Marko Tiikkaja ma...@joh.to On 9/23/13 6:40 AM, Pavel Stehule wrote: After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression It looks like I'm losing this battle, but this syntax isn't too bad. I don't win too, but result is good :) and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] This sounds like a decent enhancement. p.s. clause WHEN can be used for other exception level - so it can be a interesting shortcut for other use cases. This idea is good, I like it. I could prepare a patch for this, unless someone else wants to? please, do it. Regards Pavel Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
On 9/23/13 10:50 AM, I wrote: On 9/23/13 6:40 AM, Pavel Stehule wrote: After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression It looks like I'm losing this battle, but this syntax isn't too bad. and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] This sounds like a decent enhancement. Oh, it would be nice to have the option here to say assertions can't be caught by exception handlers, but I don't know how that mechanism works so I'm not sure it's possible. I'll have to look into that. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On 23 September 2013 10:10, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/9/22 Jaime Casanova ja...@2ndquadrant.com El 21/09/2013 17:16, Jaime Casanova ja...@2ndquadrant.com escribió: On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote: On 9/20/13 12:09 PM, Amit Khandekar wrote: On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQL code, would be extremely surprised if this new cool option to RAISE didn't work for some reason. If we use ASSERT the situation is different; most people will realize it's a new command and works differently from RAISE. What about just adding a clause WHEN to the RAISE statement and use the level machinery (client_min_messages) to make it appear or not of course, this has the disadvantage that an EXCEPTION level will always happen... or you can make it a new loglevel that mean EXCEPTION if asserts_enabled meaning RAISE ASSERT of course After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; *RAISE [ ASSERT ] WHEN bool_expression;* RAISE ; comments? Regards Pavel p.s. clause WHEN can be used for other exception level - so it can be a interesting shortcut for other use cases. -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
Re: [HACKERS] Assertions in PL/PgSQL
On 2013-09-23 11:00:50 +0200, Marko Tiikkaja wrote: On 9/23/13 10:50 AM, I wrote: On 9/23/13 6:40 AM, Pavel Stehule wrote: After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression It looks like I'm losing this battle, but this syntax isn't too bad. and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] This sounds like a decent enhancement. Oh, it would be nice to have the option here to say assertions can't be caught by exception handlers, but I don't know how that mechanism works so I'm not sure it's possible. I'll have to look into that. RAISE ASSERT ... assert_level = PANIC :P. Greetings, Andres Freund -- Andres Freund http://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] Assertions in PL/PgSQL
2013/9/23 Amit Khandekar amit.khande...@enterprisedb.com On 23 September 2013 10:10, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/9/22 Jaime Casanova ja...@2ndquadrant.com El 21/09/2013 17:16, Jaime Casanova ja...@2ndquadrant.com escribió: On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote: On 9/20/13 12:09 PM, Amit Khandekar wrote: On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQL code, would be extremely surprised if this new cool option to RAISE didn't work for some reason. If we use ASSERT the situation is different; most people will realize it's a new command and works differently from RAISE. What about just adding a clause WHEN to the RAISE statement and use the level machinery (client_min_messages) to make it appear or not of course, this has the disadvantage that an EXCEPTION level will always happen... or you can make it a new loglevel that mean EXCEPTION if asserts_enabled meaning RAISE ASSERT of course After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; *RAISE [ ASSERT ] WHEN bool_expression;* RAISE ; I don't think so it is a good idea. WHEN clause should be independent on exception level. Pavel comments? Regards Pavel p.s. clause WHEN can be used for other exception level - so it can be a interesting shortcut for other use cases. -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
Re: [HACKERS] Assertions in PL/PgSQL
2013/9/23 Marko Tiikkaja ma...@joh.to On 9/23/13 10:50 AM, I wrote: On 9/23/13 6:40 AM, Pavel Stehule wrote: After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression It looks like I'm losing this battle, but this syntax isn't too bad. and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] This sounds like a decent enhancement. Oh, it would be nice to have the option here to say assertions can't be caught by exception handlers, but I don't know how that mechanism works so I'm not sure it's possible. I'll have to look into that. Personally, I don't think so it is too important, although it can be nice improvement. I don't see use cases where assert can be handled - and with conditional RAISE we can raise a custom exceptions simply. Pavel Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
2013/9/23 Andres Freund and...@2ndquadrant.com On 2013-09-23 11:00:50 +0200, Marko Tiikkaja wrote: On 9/23/13 10:50 AM, I wrote: On 9/23/13 6:40 AM, Pavel Stehule wrote: After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression It looks like I'm losing this battle, but this syntax isn't too bad. and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] This sounds like a decent enhancement. Oh, it would be nice to have the option here to say assertions can't be caught by exception handlers, but I don't know how that mechanism works so I'm not sure it's possible. I'll have to look into that. RAISE ASSERT ... assert_level = PANIC :P. :) maybe some little bit less than PANIC Pavel Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Assertions in PL/PgSQL
On 9/23/13 11:01 AM, Amit Khandekar wrote: The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; *RAISE [ ASSERT ] WHEN bool_expression;* RAISE ; I'd expect RAISE .. WHEN ..; to be the same as: IF .. THEN RAISE; END IF; i.e. in conditionally raise the caught exception in an exception handler. So I'd say making the ASSERT keyword optional here would be very confusing. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On 9/23/13 11:12 AM, I wrote: On 9/23/13 11:01 AM, Amit Khandekar wrote: The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; *RAISE [ ASSERT ] WHEN bool_expression;* RAISE ; I'd expect RAISE .. WHEN ..; to be the same as: IF .. THEN RAISE; END IF; Should've probably proofread that one. I meant: RAISE WHEN true; would be equivalent to IF true THEN RAISE; END IF; Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
2013/9/23 Marko Tiikkaja ma...@joh.to On 9/23/13 11:12 AM, I wrote: On 9/23/13 11:01 AM, Amit Khandekar wrote: The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; *RAISE [ ASSERT ] WHEN bool_expression;* RAISE ; I'd expect RAISE .. WHEN ..; to be the same as: IF .. THEN RAISE; END IF; Should've probably proofread that one. I meant: RAISE WHEN true; would be equivalent to IF true THEN RAISE; END IF; we use a RAISE only keyword statement for resignaling, so it can be really confusing Pavel Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
On 23 September 2013 14:33, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/9/23 Amit Khandekar amit.khande...@enterprisedb.com On 23 September 2013 10:10, Pavel Stehule pavel.steh...@gmail.comwrote: 2013/9/22 Jaime Casanova ja...@2ndquadrant.com El 21/09/2013 17:16, Jaime Casanova ja...@2ndquadrant.com escribió: On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote: On 9/20/13 12:09 PM, Amit Khandekar wrote: On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQL code, would be extremely surprised if this new cool option to RAISE didn't work for some reason. If we use ASSERT the situation is different; most people will realize it's a new command and works differently from RAISE. What about just adding a clause WHEN to the RAISE statement and use the level machinery (client_min_messages) to make it appear or not of course, this has the disadvantage that an EXCEPTION level will always happen... or you can make it a new loglevel that mean EXCEPTION if asserts_enabled meaning RAISE ASSERT of course After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] The assert levels sound a bit like a user might be confused by these levels being present at both places: In the RAISE syntax itself, and the assert GUC level. But I like the syntax. How about keeping the ASSERT keyword optional ? When we have WHEN, we anyway mean that we ware asserting that this condition must be true. So something like this : RAISE [ level ] 'format' [, expression [, ... ]] [ USING option = expression [, ... ] ]; RAISE [ level ] condition_name [ USING option = expression [, ... ] ]; RAISE [ level ] SQLSTATE 'sqlstate' [ USING option = expression [, ... ] ]; RAISE [ level ] USING option = expression [, ... ]; *RAISE [ ASSERT ] WHEN bool_expression;* RAISE ; I don't think so it is a good idea. WHEN clause should be independent on exception level. I am ok with generalizing the WHEN clause across all levels. The main proposal was for adding assertion support, so we can keep the WHEN generalization as a nice-to-have stuff and do it only if it comes as a natural extension in the assertion support patch. Pavel comments? Regards Pavel p.s. clause WHEN can be used for other exception level - so it can be a interesting shortcut for other use cases. -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
Re: [HACKERS] Assertions in PL/PgSQL
2013/9/22 Jaime Casanova ja...@2ndquadrant.com El 21/09/2013 17:16, Jaime Casanova ja...@2ndquadrant.com escribió: On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote: On 9/20/13 12:09 PM, Amit Khandekar wrote: On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQL code, would be extremely surprised if this new cool option to RAISE didn't work for some reason. If we use ASSERT the situation is different; most people will realize it's a new command and works differently from RAISE. What about just adding a clause WHEN to the RAISE statement and use the level machinery (client_min_messages) to make it appear or not of course, this has the disadvantage that an EXCEPTION level will always happen... or you can make it a new loglevel that mean EXCEPTION if asserts_enabled meaning RAISE ASSERT of course After days I am thinking so it can be a good solution syntax - enhanced current RAISE RAISE ASSERT WHEN boolean expression RAISE ASSERT 'some message' WHEN expression and we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql options assert_level = [*ignore*, notice, warning, error] comments? Regards Pavel p.s. clause WHEN can be used for other exception level - so it can be a interesting shortcut for other use cases. -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
Re: [HACKERS] Assertions in PL/PgSQL
On 9/19/13 7:08 AM, Pavel Stehule wrote: FWIW, we've written a framework (currently available in the EnovaTools project on pgFoundry) that allows for very, very fine-grain control over asserts. - Every assert has a name (and an optional sub-name) as well as a level - You can globally set the minimum level that will trigger an assert. This is useful for some debugging stuff; have an assert with a negative level and normally it won't fire unless you set the minimum level to be less than zero. - You can disable an assert globally (across all backends) - You can disable an assert only within your session We should eventually allow for disabling an assert only for your transaction; we just haven't gotten around to it yet. The reason for all this flexibility is the concept of it should be very difficult but not impossible for the code to do X. We use it for sanity-checking things. I think so similar frameworks will be exists (we have some similar functionality) in orafce too - and it is not reason why we should not merge some function to core. I am with Marko, so some simple, user friendly statement for assertions should be very nice plpgsql feature. I am different in opinion how to implementat it and about syntax. I prefer a possibility (not necessary currently implemented) to enhance this feature for similar tasks (as buildin or external feature) Probably You and me have a same opinion so only simple and very primitive assert is not enough: I see as useful feature for assertions: a) possibility to specify a message (two parametric assert) b) possibility to specify some threshold c) possibility to specify some level (exception, warning, notice) .. default should be exception c) possibility to specify a handled/unhandled exception I'm not opposed to the improvements you're proposing, but I do want to point out that none of them would allow us to use these asserts, because we definitely need the ability to enable and disable individual asserts. (Understand that what we've developed is actually rather different from the C concept of asserts...) I'm not saying that's necessarily bad, but there is an interesting point here: different environments might have radically different needs for dealing with asserts that fail. What we *could* make use of would be asserts that are extremely fast when the assert passes but then allow us to do whatever we want when an assert fails (including possibly ignoring the fact that the assert failed). Of course, if the community wanted to just accept the API and functionality we've developed I'd be fine with that too... ;P -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] Assertions in PL/PgSQL
On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote: On 9/20/13 12:09 PM, Amit Khandekar wrote: On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQL code, would be extremely surprised if this new cool option to RAISE didn't work for some reason. If we use ASSERT the situation is different; most people will realize it's a new command and works differently from RAISE. What about just adding a clause WHEN to the RAISE statement and use the level machinery (client_min_messages) to make it appear or not of course, this has the disadvantage that an EXCEPTION level will always happen... or you can make it a new loglevel that mean EXCEPTION if asserts_enabled -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- 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] Assertions in PL/PgSQL
El 21/09/2013 17:16, Jaime Casanova ja...@2ndquadrant.com escribió: On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote: On 9/20/13 12:09 PM, Amit Khandekar wrote: On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQL code, would be extremely surprised if this new cool option to RAISE didn't work for some reason. If we use ASSERT the situation is different; most people will realize it's a new command and works differently from RAISE. What about just adding a clause WHEN to the RAISE statement and use the level machinery (client_min_messages) to make it appear or not of course, this has the disadvantage that an EXCEPTION level will always happen... or you can make it a new loglevel that mean EXCEPTION if asserts_enabled meaning RAISE ASSERT of course -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
Re: [HACKERS] Assertions in PL/PgSQL
On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: On 2013-09-15 23:23, Jaime Casanova wrote: If using ASSERT as keyword is not acceptable, not that i agree but in case. What about using RAISE EXCEPTION WHEN (condition) I was going to suggest the same idea: Extend RAISE syntax without introducing new keywords. Something like: RAISE assert_exception WHEN assert_condition ... where assert_exception is a new exception label which maps to a new internal sqlstate. I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/**mailpref/pgsql-hackershttp://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Assertions in PL/PgSQL
On 9/20/13 12:09 PM, Amit Khandekar wrote: On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote: I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is false. Isn't this possible ? Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQL code, would be extremely surprised if this new cool option to RAISE didn't work for some reason. If we use ASSERT the situation is different; most people will realize it's a new command and works differently from RAISE. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On 9/19/13 9:09 PM, Robert Haas wrote: Personally, I'm pretty skeptical about the value of adding dedicated syntax for this. I mean, I'll be the first to admit that PL/pgsql is a clunky and awkward language. But somebody's always proposing something that they think will make it better, and I feel somehow that if we accept all of those proposals at face value, we'll just end up with a mess. So IMHO the bar for adding new syntax to PL/pgsql should be reasonably high. YMMV, of course, and probably does. I see where you're coming from, and agree, to an extent. The issue of how this is spelled is somewhat secondary for me. I think ASSERT is probably as good as anything. But let's not kid ourselves: even reserving this word only in PL/pgsql WILL break things for some users, and there are LOTS of people out there with LOTS of procedural code. Every tiny backward-incompatibility reduces by just a little bit the percentage of those people who can upgrade and increases the delay before they do. This is an area where past experience has made me quite wary. The thing is, what this means is that to add a new feature to the language, you have to make the syntax so damn ugly that no one wants to use it (see row_count, for example) or you will break some poor user's function. And now we got all this cool functionality which nobody wants to use, and the language itself actually gets progressively worse. All this is starting to sound like it's already too late to make PL/PgSQL better, and we should just start afresh. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On Fri, Sep 20, 2013 at 6:24 AM, Marko Tiikkaja ma...@joh.to wrote: The issue of how this is spelled is somewhat secondary for me. I think ASSERT is probably as good as anything. But let's not kid ourselves: even reserving this word only in PL/pgsql WILL break things for some users, and there are LOTS of people out there with LOTS of procedural code. Every tiny backward-incompatibility reduces by just a little bit the percentage of those people who can upgrade and increases the delay before they do. This is an area where past experience has made me quite wary. The thing is, what this means is that to add a new feature to the language, you have to make the syntax so damn ugly that no one wants to use it (see row_count, for example) or you will break some poor user's function. And now we got all this cool functionality which nobody wants to use, and the language itself actually gets progressively worse. All this is starting to sound like it's already too late to make PL/PgSQL better, and we should just start afresh. To some extent I agree that PL/pgsql is hopeless. I think there are some things we can do to improve it, but most of what gets proposed at least in this forum strikes me as tinkering around the edges, and it can't make up for fundamentally bad language design decisions. Part of the problem, of course, is that most programming languages don't get re-released every year. It's not that it would be OK for C to suddenly reserve a bunch of new keywords; it's that they don't try. And when they do release no language versions (like C99) some people (like us) don't adopt them, for fear of being unable to run our code on older systems. Such considerations apply with equal force to PL/pgsql, but it gets a new release every year rather than every decade, so the problems are magnified. The other part of the problem is that the language isn't designed from the beginning to be extensible. In Perl, for example, they chose to mark variables with a leading $, @, or % and functions with a leading . That last marking has largely fallen into desuetude, but the point is that - to the extent that you do have and use such markers - you can add new keywords without breaking anything. Some languages can also distinguish keywords positionally; for example, ABORT doesn't need to be reserved in PostgreSQL's SQL dialect because it can only appear as a command at the beginning of a line, and it can't be a column, type, or function name in that position. Such an approach might even work ASSERT in PL/pgsql, if there's a clean way to disambiguate vs. the assignment syntax. But even if we can make that work, we're going to continue to face this problem with each new language extension. -- 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] Assertions in PL/PgSQL
2013/9/20 Robert Haas robertmh...@gmail.com On Fri, Sep 20, 2013 at 6:24 AM, Marko Tiikkaja ma...@joh.to wrote: The issue of how this is spelled is somewhat secondary for me. I think ASSERT is probably as good as anything. But let's not kid ourselves: even reserving this word only in PL/pgsql WILL break things for some users, and there are LOTS of people out there with LOTS of procedural code. Every tiny backward-incompatibility reduces by just a little bit the percentage of those people who can upgrade and increases the delay before they do. This is an area where past experience has made me quite wary. The thing is, what this means is that to add a new feature to the language, you have to make the syntax so damn ugly that no one wants to use it (see row_count, for example) or you will break some poor user's function. And now we got all this cool functionality which nobody wants to use, and the language itself actually gets progressively worse. All this is starting to sound like it's already too late to make PL/PgSQL better, and we should just start afresh. To some extent I agree that PL/pgsql is hopeless. I think there are some things we can do to improve it, but most of what gets proposed at least in this forum strikes me as tinkering around the edges, and it can't make up for fundamentally bad language design decisions. Part of the problem, of course, is that most programming languages don't get re-released every year. It's not that it would be OK for C to suddenly reserve a bunch of new keywords; it's that they don't try. And when they do release no language versions (like C99) some people (like us) don't adopt them, for fear of being unable to run our code on older systems. Such considerations apply with equal force to PL/pgsql, but it gets a new release every year rather than every decade, so the problems are magnified. The other part of the problem is that the language isn't designed from the beginning to be extensible. In Perl, for example, they chose to mark variables with a leading $, @, or % and functions with a leading . That last marking has largely fallen into desuetude, but the point is that - to the extent that you do have and use such markers - you can add new keywords without breaking anything. Some languages can also distinguish keywords positionally; for example, ABORT doesn't need to be reserved in PostgreSQL's SQL dialect because it can only appear as a command at the beginning of a line, and it can't be a column, type, or function name in that position. Such an approach might even work ASSERT in PL/pgsql, if there's a clean way to disambiguate vs. the assignment syntax. But even if we can make that work, we're going to continue to face this problem with each new language extension. PL/pgSQL had a ADA completeness, uniformity and beauty newer. But it is not too bad, and one new specialized statement doesn't kill us. A proposed functionality is often used and we have not tools (macros) how to implement it simply. we support a conditions for few statement - so enhancing RAISE statement is possible so some form of RAISE EXCEPTION WHEN NOT FOUND AND use_assrts USING message = 'there are no some'; but this form is relative long and less readable (can be difficult find asserts in code and separate it from custom exceptions). I am fully for some variant of ASSERT statement. The benefit is higher than cost. ASSERT keyword is simply, readable - and I can accept it, if we found a syntax for complete functionality (although I prefer a PRAGMA introduction). Regards Pavel -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] Assertions in PL/PgSQL
On 09/20/2013 01:59 PM, Robert Haas wrote: The other part of the problem is that the language isn't designed from the beginning to be extensible. In Perl, for example, they chose to mark variables with a leading $, @, or % and functions with a leading . That last marking has largely fallen into desuetude, but the point is that - to the extent that you do have and use such markers - you can add new keywords without breaking anything. Some languages can also distinguish keywords positionally; for example, ABORT doesn't need to be reserved in PostgreSQL's SQL dialect because it can only appear as a command at the beginning of a line, and it can't be a column, type, or function name in that position. Such an approach might even work ASSERT in PL/pgsql, if there's a clean way to disambiguate vs. the assignment syntax. But even if we can make that work, we're going to continue to face this problem with each new language extension. Perhaps we could use the pragma approach here and add some types of new functionality in omments --#ASSERT . or even --#pragma ASSERT . It is still not guaranteed to be 100% compatible, but at least changing comments should be relatively safe way for fixing your functions And you could have another pragma to disable some pragmas which you could SET in GUC (global, session or per function) for extra ugliness ;) -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- 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] Assertions in PL/PgSQL
Pavel Stehule escribió: PL/pgSQL had a ADA completeness, uniformity and beauty newer. But it is not too bad, and one new specialized statement doesn't kill us. A proposed functionality is often used and we have not tools (macros) how to implement it simply. we support a conditions for few statement - so enhancing RAISE statement is possible Extending RAISE is one option. Another option is to decorate BEGIN and END with an assertion option; and the assertion would be checked when the block is entered (in BEGIN) or finished (in END). BEGIN ASSERT (a = 1) WITH (name = a_is_one) a := a + 1; END; BEGIN ASSERT (a 0) a := a + 1; END ASSERT (a = 2) WITH (name = a_is_two); This would play nice with loops too, where the assertion is checked on every iteration. And you can have empty blocks if you want the assertion to be standalone in the middle of some block. -- Á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] Assertions in PL/PgSQL
2013/9/20 Alvaro Herrera alvhe...@2ndquadrant.com Pavel Stehule escribió: PL/pgSQL had a ADA completeness, uniformity and beauty newer. But it is not too bad, and one new specialized statement doesn't kill us. A proposed functionality is often used and we have not tools (macros) how to implement it simply. we support a conditions for few statement - so enhancing RAISE statement is possible Extending RAISE is one option. Another option is to decorate BEGIN and END with an assertion option; and the assertion would be checked when the block is entered (in BEGIN) or finished (in END). BEGIN ASSERT (a = 1) WITH (name = a_is_one) a := a + 1; END; BEGIN ASSERT (a 0) a := a + 1; END ASSERT (a = 2) WITH (name = a_is_two); This would play nice with loops too, where the assertion is checked on every iteration. And you can have empty blocks if you want the assertion to be standalone in the middle of some block. it can works, but it looks too strange -1 Pavel -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Assertions in PL/PgSQL
2013/9/18 Jim Nasby j...@nasby.net
On 9/14/13 11:55 PM, Pavel Stehule wrote:
2013/9/15 Marko Tiikkaja ma...@joh.to mailto:ma...@joh.to
On 2013-09-15 00:09, Pavel Stehule wrote:
this is a possibility for introduction a new hook and possibility
implement
asserions and similar task in generic form (as extension). it can
be
assertions, tracing, profiling.
You can already do tracing and profiling in an extension. I don't
see what you would put inside the function body for these two, either.
you cannot mark a tracing points explicitly in current (unsupported now)
extensions.
These functions share same pattern:
CREATE OR REPLACE FUNCTION assert(boolean)
RETURNS void AS $$
IF current_setting('plpgsq.**assertions') = 'on' THEN
IF $1 THEN
RAISE EXCEPTION 'Assert fails';
END IF;
END IF;
END;
$$ LANGUAGE plpgsql;
CREATE OR REPLACE FUNCTION trace(text)
RETURNS void AS $$
IF current_setting('plpgsq.trace'**) = 'on' THEN
RAISE WARNING 'trace: %', $1; END IF;
END;
$$ LANGUAGE plpgsql;
Depends on usage, these functions will not be extremely slow against to
builtin solution - can be faster, if we implement it in C, and little bit
faster if we implement it as internal PLpgSQL statement. But if you use a
one not simple queries, then overhead is not significant (probably).
You have to watch some global state variable and then execute (or not)
some functionality.
FWIW, we've written a framework (currently available in the EnovaTools
project on pgFoundry) that allows for very, very fine-grain control over
asserts.
- Every assert has a name (and an optional sub-name) as well as a level
- You can globally set the minimum level that will trigger an assert. This
is useful for some debugging stuff; have an assert with a negative level
and normally it won't fire unless you set the minimum level to be less than
zero.
- You can disable an assert globally (across all backends)
- You can disable an assert only within your session
We should eventually allow for disabling an assert only for your
transaction; we just haven't gotten around to it yet.
The reason for all this flexibility is the concept of it should be very
difficult but not impossible for the code to do X. We use it for
sanity-checking things.
I think so similar frameworks will be exists (we have some similar
functionality) in orafce too - and it is not reason why we should not merge
some function to core. I am with Marko, so some simple, user friendly
statement for assertions should be very nice plpgsql feature. I am
different in opinion how to implementat it and about syntax. I prefer a
possibility (not necessary currently implemented) to enhance this feature
for similar tasks (as buildin or external feature)
Probably You and me have a same opinion so only simple and very primitive
assert is not enough:
I see as useful feature for assertions:
a) possibility to specify a message (two parametric assert)
b) possibility to specify some threshold
c) possibility to specify some level (exception, warning, notice) ..
default should be exception
c) possibility to specify a handled/unhandled exception
Regards
Pavel
--
Jim C. Nasby, Data Architect j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net
Re: [HACKERS] Assertions in PL/PgSQL
On 9/18/13 5:11 PM, Pavel Stehule wrote: In this code a assert fail can be lost in app log. Or can be knowingly handled and ignored - what is wrong, and should not be allowed. When I wrote a little bit complex procedures, I had to use a EXCEPTION WHEN OTHERS clause - because I would not to lost a transaction. It worked, but searching a syntax errors was significantly harder - so on base of this experience I am thinking so some errors can be handled (related to database usage) and others not - like syntax errors in PL/pgSQL or possible assertions (although we can handle syntax error, but I don't think so it is practical). It significantly increase a work that is necessary for error identification. I think that's a fair point. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On 9/19/13 2:08 PM, Pavel Stehule wrote: I think so similar frameworks will be exists (we have some similar Probably You and me have a same opinion so only simple and very primitive assert is not enough: I see as useful feature for assertions: a) possibility to specify a message (two parametric assert) b) possibility to specify some threshold c) possibility to specify some level (exception, warning, notice) .. default should be exception c) possibility to specify a handled/unhandled exception I think these are all neat ideas on how to further improve this feature. I'd like to see at least a) in 9.4, but I haven't yet looked at how it could be implemented. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
2013/9/19 Marko Tiikkaja ma...@joh.to On 9/19/13 2:08 PM, Pavel Stehule wrote: I think so similar frameworks will be exists (we have some similar Probably You and me have a same opinion so only simple and very primitive assert is not enough: I see as useful feature for assertions: a) possibility to specify a message (two parametric assert) b) possibility to specify some threshold c) possibility to specify some level (exception, warning, notice) .. default should be exception c) possibility to specify a handled/unhandled exception I think these are all neat ideas on how to further improve this feature. I'd like to see at least a) in 9.4, but I haven't yet looked at how it could be implemented. Not all must be implemented in 9.4, although it is +/- only exception parametrization - not hard for implementation. But syntax should be prepared for this functionality (or should be extensible as minimum) before. Bison parser is not friendly for additional extending :( - and we can break a future extending simply just only on syntax level with bad design now. It is reason, why I am doing noise here. I remember relatively difficult extending of RAISE statement. Regards Pavel Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
On Sat, Sep 14, 2013 at 6:09 PM, Marko Tiikkaja ma...@joh.to wrote: On 2013-09-14 23:55, Pavel Stehule wrote: but introduction a reserved keword for one very special purpose (without extensibility) is not prudent. How about using an existing keyword then? ASSERTION used to be reserved in the SQL standard but is unreserved in postgres. CHECK might work and is fully reserved. CONSTRAINT? IS? Personally, I'm pretty skeptical about the value of adding dedicated syntax for this. I mean, I'll be the first to admit that PL/pgsql is a clunky and awkward language. But somebody's always proposing something that they think will make it better, and I feel somehow that if we accept all of those proposals at face value, we'll just end up with a mess. So IMHO the bar for adding new syntax to PL/pgsql should be reasonably high. YMMV, of course, and probably does. The issue of how this is spelled is somewhat secondary for me. I think ASSERT is probably as good as anything. But let's not kid ourselves: even reserving this word only in PL/pgsql WILL break things for some users, and there are LOTS of people out there with LOTS of procedural code. Every tiny backward-incompatibility reduces by just a little bit the percentage of those people who can upgrade and increases the delay before they do. This is an area where past experience has made me quite wary. Maybe I'm worrying over nothing; this really is a pretty small change. But once bitten, twice shy. -- 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] Assertions in PL/PgSQL
Hello 2013/9/18 Marko Tiikkaja ma...@joh.to On 2013-09-16 21:24, Pavel Stehule wrote: 2. a failed assert should to raise a exception, that should not be handled by any exception handler - similar to ERRCODE_QUERY_CANCELED - see exception_matches_conditions. I'm not sure what I think about that idea. I see decent arguments for it working either way. Care to unravel yours a bit more? yes so CREATE OR REPLACE FUNCTION foo(a int) RETURNS int BEGIN ASSERT a BETWEEN 1 AND 100; RETURNS a; END; $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION proc() RETURNS int AS $$ BEGIN do some complex logic that exec a foo function EXCEPTION WHEN OTHERS THEN -- log some errors INSERT INTO log VALUES(...) END; $$ LANGUAGE plpgsql; In this code a assert fail can be lost in app log. Or can be knowingly handled and ignored - what is wrong, and should not be allowed. When I wrote a little bit complex procedures, I had to use a EXCEPTION WHEN OTHERS clause - because I would not to lost a transaction. It worked, but searching a syntax errors was significantly harder - so on base of this experience I am thinking so some errors can be handled (related to database usage) and others not - like syntax errors in PL/pgSQL or possible assertions (although we can handle syntax error, but I don't think so it is practical). It significantly increase a work that is necessary for error identification. Regards Pavel Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
On 9/14/13 11:55 PM, Pavel Stehule wrote:
2013/9/15 Marko Tiikkaja ma...@joh.to mailto:ma...@joh.to
On 2013-09-15 00:09, Pavel Stehule wrote:
this is a possibility for introduction a new hook and possibility
implement
asserions and similar task in generic form (as extension). it can be
assertions, tracing, profiling.
You can already do tracing and profiling in an extension. I don't see what
you would put inside the function body for these two, either.
you cannot mark a tracing points explicitly in current (unsupported now)
extensions.
These functions share same pattern:
CREATE OR REPLACE FUNCTION assert(boolean)
RETURNS void AS $$
IF current_setting('plpgsq.assertions') = 'on' THEN
IF $1 THEN
RAISE EXCEPTION 'Assert fails';
END IF;
END IF;
END;
$$ LANGUAGE plpgsql;
CREATE OR REPLACE FUNCTION trace(text)
RETURNS void AS $$
IF current_setting('plpgsq.trace') = 'on' THEN
RAISE WARNING 'trace: %', $1; END IF;
END;
$$ LANGUAGE plpgsql;
Depends on usage, these functions will not be extremely slow against to builtin
solution - can be faster, if we implement it in C, and little bit faster if we
implement it as internal PLpgSQL statement. But if you use a one not simple
queries, then overhead is not significant (probably).
You have to watch some global state variable and then execute (or not) some
functionality.
FWIW, we've written a framework (currently available in the EnovaTools project
on pgFoundry) that allows for very, very fine-grain control over asserts.
- Every assert has a name (and an optional sub-name) as well as a level
- You can globally set the minimum level that will trigger an assert. This is
useful for some debugging stuff; have an assert with a negative level and
normally it won't fire unless you set the minimum level to be less than zero.
- You can disable an assert globally (across all backends)
- You can disable an assert only within your session
We should eventually allow for disabling an assert only for your transaction;
we just haven't gotten around to it yet.
The reason for all this flexibility is the concept of it should be very difficult
but not impossible for the code to do X. We use it for sanity-checking things.
--
Jim C. Nasby, Data Architect j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net
--
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] Assertions in PL/PgSQL
On 2013-09-16 21:24, Pavel Stehule wrote: 2. a failed assert should to raise a exception, that should not be handled by any exception handler - similar to ERRCODE_QUERY_CANCELED - see exception_matches_conditions. I'm not sure what I think about that idea. I see decent arguments for it working either way. Care to unravel yours a bit more? Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On 9/15/13 10:49 AM, Marko Tiikkaja wrote: On 2013-09-15 16:34, Peter Eisentraut wrote: On Sat, 2013-09-14 at 20:47 +0200, Marko Tiikkaja wrote: Attached is a patch for supporting assertions in PL/PgSQL. These are similar to the Assert() backend macro: they can be disabled during compile time, but when enabled, abort execution if the passed expression is not true. Doesn't build: Ugh. Accidentally edited an auto-generated file. Fixed in the attached, thanks! Please fix the tabs in the SGML files. -- 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] Assertions in PL/PgSQL
Hello
a few other comments:
1. you disable a assert in compile time in dependency of enable_assertions
variable. I don't think, so it is good idea. When somebody enables a
assertions, then assertions will not work on all cached functions in
session. You should to do check if assertions are enabled in execution time
(there are no any significant advantage do it in compile time) or you
should to clean cache.
2. a failed assert should to raise a exception, that should not be handled
by any exception handler - similar to ERRCODE_QUERY_CANCELED - see
exception_matches_conditions.
Regards
Pavel
2013/9/14 Marko Tiikkaja ma...@joh.to
Hi,
Attached is a patch for supporting assertions in PL/PgSQL. These are
similar to the Assert() backend macro: they can be disabled during compile
time, but when enabled, abort execution if the passed expression is not
true.
A simple example:
CREATE FUNCTION delete_user(username text) RETURNS VOID AS $$
BEGIN
DELETE FROM users WHERE users.username = delete_user.username;
ASSERT FOUND;
END
$$ LANGUAGE plpgsql;
SELECT delete_user('mia');
ERROR: Assertion on line 4 failed
CONTEXT: PL/pgSQL function delete_user(text) line 4 at ASSERT
Again, I'll add this to the open commitfest, but feedback is greatly
appreciated.
Regards,
Marko Tiikkaja
--
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] Assertions in PL/PgSQL
On Sat, 2013-09-14 at 20:47 +0200, Marko Tiikkaja wrote: Attached is a patch for supporting assertions in PL/PgSQL. These are similar to the Assert() backend macro: they can be disabled during compile time, but when enabled, abort execution if the passed expression is not true. Doesn't build: pl_exec.c: In function ‘exec_stmt_assert’: pl_exec.c:3647:58: error: ‘ERRCODE_ASSERTION_FAILURE’ undeclared (first use in this function) pl_exec.c:3647:58: note: each undeclared identifier is reported only once for each function it appears in -- 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] Assertions in PL/PgSQL
On 2013-09-15 16:34, Peter Eisentraut wrote:
On Sat, 2013-09-14 at 20:47 +0200, Marko Tiikkaja wrote:
Attached is a patch for supporting assertions in PL/PgSQL. These are
similar to the Assert() backend macro: they can be disabled during
compile time, but when enabled, abort execution if the passed expression
is not true.
Doesn't build:
Ugh. Accidentally edited an auto-generated file. Fixed in the
attached, thanks!
Regards,
Marko Tiikkaja
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***
*** 3528,3533 RAISE unique_violation USING MESSAGE = 'Duplicate user ID: '
|| user_id;
--- 3528,3596
/para
/note
+ sect2 id=plpgsql-assert
+titleAssertions/title
+
+para
+ literalAssertions/literal provide a way to check that the
+ internal state of a function is as expected. For example:
+ programlisting
+ CREATE FUNCTION delete_user(username text) RETURNS VOID AS $$
+ BEGIN
+ DELETE FROM users WHERE users.username = delete_user.username;
+ ASSERT FOUND;
+ END
+ $$ LANGUAGE plpgsql;
+
+ SELECT delete_user('mia');
+ ERROR: Assertion on line 4 failed
+ CONTEXT: PL/pgSQL function delete_user(text) line 4 at ASSERT
+ /programlisting
+
+ One could implement the equivalent functionality with a conditional
+ RAISE EXCEPTION statement, but assertions have two major differences:
+ itemizedlist
+ listitem
+para
+ They're a lot faster to write than the equivalent IF .. THEN
RAISE EXCEPTION .. END IF constructs.
+/para
+ /listitem
+ listitem
+para
+ They can be (and are by default) disabled in production
+ environments. A disabled assertion only incurs a negligible
+ compile-time overhead and no execution time overhead, so you
+ can write complex checks for development environments without
+ having to worry about performance.
+/para
+ /listitem
+ /itemizedlist
+/para
+
+para
+ The configuration parameter varnameplpgsql.enable_assertions/
+ controls whether assertions are enabled. Note that the value of
+ this parameter only affects subsequent compilations of
+ applicationPL/pgSQL/ functions, but not statements already
+ compiled in the current session.
+/para
+
+para
+ It is also possible to enable assertions in a single function
+ (possibly overriding the global setting) by using a compile
+ option:
+ programlisting
+ CREATE FUNCTION delete_user(username text) RETURNS VOID AS $$
+ #enable_assertions
+ BEGIN
+ DELETE FROM users WHERE users.username = delete_user.username;
+ ASSERT FOUND;
+ END
+ $$ LANGUAGE plpgsql;
+ /programlisting
+/para
+ /sect2
+
/sect1
sect1 id=plpgsql-trigger
*** a/src/backend/utils/errcodes.txt
--- b/src/backend/utils/errcodes.txt
***
*** 454,459 PEERRCODE_PLPGSQL_ERROR
plp
--- 454,460
P0001EERRCODE_RAISE_EXCEPTION
raise_exception
P0002EERRCODE_NO_DATA_FOUND
no_data_found
P0003EERRCODE_TOO_MANY_ROWS
too_many_rows
+ P0004EERRCODE_ASSERTION_FAILURE
assertion_failure
Section: Class XX - Internal Error
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***
*** 351,356 do_compile(FunctionCallInfo fcinfo,
--- 351,357
function-fn_cxt = func_cxt;
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
+ function-enable_assertions = plpgsql_enable_assertions;
if (is_dml_trigger)
function-fn_is_trigger = PLPGSQL_DML_TRIGGER;
***
*** 847,852 plpgsql_compile_inline(char *proc_source)
--- 848,854
function-fn_cxt = func_cxt;
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
+ function-enable_assertions = plpgsql_enable_assertions;
plpgsql_ns_init();
plpgsql_ns_push(func_name);
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***
*** 133,138 static int exec_stmt_dynexecute(PLpgSQL_execstate *estate,
--- 133,140
PLpgSQL_stmt_dynexecute *stmt);
static int exec_stmt_dynfors(PLpgSQL_execstate *estate,
PLpgSQL_stmt_dynfors *stmt);
+ static int exec_stmt_assert(PLpgSQL_execstate *estate,
+ PLpgSQL_stmt_assert *stmt);
static void plpgsql_estate_setup(PLpgSQL_execstate *estate,
PLpgSQL_function
Re: [HACKERS] Assertions in PL/PgSQL
El 14/09/2013 15:25, Pavel Stehule pavel.steh...@gmail.com escribió: Hello There is a significant issue - new reserved keyword. There is high probability so lot of users has a functions named assert. I like this functionality, but I dislike any compatibility break for feature, that can be implemented as extension without any lost of compatibility or lost of functionality. So can you redesign this without new keyword? Hi, If using ASSERT as keyword is not acceptable, not that i agree but in case. What about using RAISE EXCEPTION WHEN (condition) -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
Re: [HACKERS] Assertions in PL/PgSQL
On 2013-09-15 23:23, Jaime Casanova wrote: If using ASSERT as keyword is not acceptable, not that i agree but in case. What about using RAISE EXCEPTION WHEN (condition) I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
El 15/09/2013 17:13, Marko Tiikkaja ma...@joh.to escribió: On 2013-09-15 23:23, Jaime Casanova wrote: If using ASSERT as keyword is not acceptable, not that i agree but in case. What about using RAISE EXCEPTION WHEN (condition) I think it would be extremely surprising if a command like that got optimized away based on a GUC, so I don't think that would be a good idea. Ah! good point, didn't think on that -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
[HACKERS] Assertions in PL/PgSQL
Hi,
Attached is a patch for supporting assertions in PL/PgSQL. These are
similar to the Assert() backend macro: they can be disabled during
compile time, but when enabled, abort execution if the passed expression
is not true.
A simple example:
CREATE FUNCTION delete_user(username text) RETURNS VOID AS $$
BEGIN
DELETE FROM users WHERE users.username = delete_user.username;
ASSERT FOUND;
END
$$ LANGUAGE plpgsql;
SELECT delete_user('mia');
ERROR: Assertion on line 4 failed
CONTEXT: PL/pgSQL function delete_user(text) line 4 at ASSERT
Again, I'll add this to the open commitfest, but feedback is greatly
appreciated.
Regards,
Marko Tiikkaja
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***
*** 3528,3533 RAISE unique_violation USING MESSAGE = 'Duplicate user ID: '
|| user_id;
--- 3528,3596
/para
/note
+ sect2 id=plpgsql-assert
+titleAssertions/title
+
+para
+ literalAssertions/literal provide a way to check that the
+ internal state of a function is as expected. For example:
+ programlisting
+ CREATE FUNCTION delete_user(username text) RETURNS VOID AS $$
+ BEGIN
+ DELETE FROM users WHERE users.username = delete_user.username;
+ ASSERT FOUND;
+ END
+ $$ LANGUAGE plpgsql;
+
+ SELECT delete_user('mia');
+ ERROR: Assertion on line 4 failed
+ CONTEXT: PL/pgSQL function delete_user(text) line 4 at ASSERT
+ /programlisting
+
+ One could implement the equivalent functionality with a conditional
+ RAISE EXCEPTION statement, but assertions have two major differences:
+ itemizedlist
+ listitem
+para
+ They're a lot faster to write than the equivalent IF .. THEN
RAISE EXCEPTION .. END IF constructs.
+/para
+ /listitem
+ listitem
+para
+ They can be (and are by default) disabled in production
+ environments. A disabled assertion only incurs a negligible
+ compile-time overhead and no execution time overhead, so you
+ can write complex checks for development environments without
+ having to worry about performance.
+/para
+ /listitem
+ /itemizedlist
+/para
+
+para
+ The configuration parameter varnameplpgsql.enable_assertions/
+ controls whether assertions are enabled. Note that the value of
+ this parameter only affects subsequent compilations of
+ applicationPL/pgSQL/ functions, but not statements already
+ compiled in the current session.
+/para
+
+para
+ It is also possible to enable assertions in a single function
+ (possibly overriding the global setting) by using a compile
+ option:
+ programlisting
+ CREATE FUNCTION delete_user(username text) RETURNS VOID AS $$
+ #enable_assertions
+ BEGIN
+ DELETE FROM users WHERE users.username = delete_user.username;
+ ASSERT FOUND;
+ END
+ $$ LANGUAGE plpgsql;
+ /programlisting
+/para
+ /sect2
+
/sect1
sect1 id=plpgsql-trigger
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***
*** 351,356 do_compile(FunctionCallInfo fcinfo,
--- 351,357
function-fn_cxt = func_cxt;
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
+ function-enable_assertions = plpgsql_enable_assertions;
if (is_dml_trigger)
function-fn_is_trigger = PLPGSQL_DML_TRIGGER;
***
*** 847,852 plpgsql_compile_inline(char *proc_source)
--- 848,854
function-fn_cxt = func_cxt;
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
+ function-enable_assertions = plpgsql_enable_assertions;
plpgsql_ns_init();
plpgsql_ns_push(func_name);
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***
*** 133,138 static int exec_stmt_dynexecute(PLpgSQL_execstate *estate,
--- 133,140
PLpgSQL_stmt_dynexecute *stmt);
static int exec_stmt_dynfors(PLpgSQL_execstate *estate,
PLpgSQL_stmt_dynfors *stmt);
+ static int exec_stmt_assert(PLpgSQL_execstate *estate,
+ PLpgSQL_stmt_assert *stmt);
static void plpgsql_estate_setup(PLpgSQL_execstate *estate,
PLpgSQL_function *func,
***
*** 1466,1471 exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
--- 1468,1477
rc = exec_stmt_close(estate, (PLpgSQL_stmt_close *)
stmt);
break;
+ case PLPGSQL_STMT_ASSERT:
+ rc = exec_stmt_assert(estate, (PLpgSQL_stmt_assert *)
stmt);
+ break;
+
Re: [HACKERS] Assertions in PL/PgSQL
On 14/09/2013 20:47, I wrote: These are similar to the Assert() backend macro: they can be disabled during compile time, but when enabled, abort execution if the passed expression is not true. And by compile time here, I mean at the time when the PL/PgSQL function is compiled, not the postgres server binary. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja ma...@joh.to wrote: On 14/09/2013 20:47, I wrote: These are similar to the Assert() backend macro: they can be disabled during compile time, but when enabled, abort execution if the passed expression is not true. Hi, That's very good, thanks. And by compile time here, I mean at the time when the PL/PgSQL function is compiled, not the postgres server binary. and compile time means when the function is created or replaced? or the first time is used? if the second. Why not have a plpgsql.enable_assert variable? A directive you can use per function then i can keep the ASSERT and activate them by replacing the function -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157
Re: [HACKERS] Assertions in PL/PgSQL
On 2013-09-14 21:55, Jaime Casanova wrote: On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja ma...@joh.to wrote: And by compile time here, I mean at the time when the PL/PgSQL function is compiled, not the postgres server binary. and compile time means when the function is created or replaced? or the first time is used? Uhh.. I have to admit, I'm not sure. I think this would be when you CREATE the function for the backend that created the function, and on the first call in other backends. if the second. Why not have a plpgsql.enable_assert variable? A directive you can use per function then i can keep the ASSERT and activate them by replacing the function The patch supports a compiler option to override the global option and enable it per function: create function foof() returns void as $$ #enable_assertions begin -- failure assert 1 2; end $$ language plpgsql; Does this address your wishes? Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
Hello
There is a significant issue - new reserved keyword. There is high
probability so lot of users has a functions named assert.
I like this functionality, but I dislike any compatibility break for
feature, that can be implemented as extension without any lost of
compatibility or lost of functionality.
So can you redesign this without new keyword?
Regards
Pavel
2013/9/14 Marko Tiikkaja ma...@joh.to
Hi,
Attached is a patch for supporting assertions in PL/PgSQL. These are
similar to the Assert() backend macro: they can be disabled during compile
time, but when enabled, abort execution if the passed expression is not
true.
A simple example:
CREATE FUNCTION delete_user(username text) RETURNS VOID AS $$
BEGIN
DELETE FROM users WHERE users.username = delete_user.username;
ASSERT FOUND;
END
$$ LANGUAGE plpgsql;
SELECT delete_user('mia');
ERROR: Assertion on line 4 failed
CONTEXT: PL/pgSQL function delete_user(text) line 4 at ASSERT
Again, I'll add this to the open commitfest, but feedback is greatly
appreciated.
Regards,
Marko Tiikkaja
--
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] Assertions in PL/PgSQL
El 14/09/2013 15:18, Marko Tiikkaja ma...@joh.to escribió: On 2013-09-14 21:55, Jaime Casanova wrote: On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja ma...@joh.to wrote: And by compile time here, I mean at the time when the PL/PgSQL function is compiled, not the postgres server binary. and compile time means when the function is created or replaced? or the first time is used? Uhh.. I have to admit, I'm not sure. I think this would be when you CREATE the function for the backend that created the function, and on the first call in other backends. if the second. Why not have a plpgsql.enable_assert variable? A directive you can use per function then i can keep the ASSERT and activate them by replacing the function The patch supports a compiler option to override the global option and enable it per function: create function foof() returns void as $$ #enable_assertions begin -- failure assert 1 2; end $$ language plpgsql; Does this address your wishes? That's exactly what i wanted. thanks -- Jaime Casanova 2ndQuadrant: Your PostgreSQL partner
Re: [HACKERS] Assertions in PL/PgSQL
On 2013-09-14 22:24, Pavel Stehule wrote: There is a significant issue - new reserved keyword. There is high probability so lot of users has a functions named assert. Someone may prove me wrong here, but to me it looks like this would only prevent ASSERT from being used as the name of a PL/PgSQL variable. That's still a backwards compatibility break, but the case you were worried about should still work: =# create function assert(boolean) returns boolean as $$ begin if $1 is not true then raise exception 'custom assert()'; end if; return true; end $$ language plpgsql; CREATE FUNCTION =# create function f() returns int as $$ $# begin $# assert false; $# perform assert(true); $# if assert(true) then $# perform assert(false); $# end if; $# end $# $$ language plpgsql; CREATE FUNCTION =# select f(); ERROR: custom assert() CONTEXT: SQL statement SELECT assert(false) PL/pgSQL function f() line 6 at PERFORM I like this functionality, but I dislike any compatibility break for feature, that can be implemented as extension without any lost of compatibility or lost of functionality. I don't see how this could be implemented as an extension, even if you write it in C. I think being able to turn assertions off in production with no execution time overhead is what justifies having this in-core. The nicer syntax isn't enough (compared to, say, PERFORM assert(..)). And if we're only breaking code for people who use assert as the variable name, I'd say we go for it. So can you redesign this without new keyword? I don't see an easy way to do that, but I'm not too familiar with PL/PgSQL's parser so maybe I'm just missing something. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On 2013-09-14 22:40, I wrote: Someone may prove me wrong here, but to me it looks like this would only prevent ASSERT from being used as the name of a PL/PgSQL variable. The comment at the beginning of pl_scanner.c seems to suggest same. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
2013/9/14 Marko Tiikkaja ma...@joh.to On 2013-09-14 22:40, I wrote: Someone may prove me wrong here, but to me it looks like this would only prevent ASSERT from being used as the name of a PL/PgSQL variable. The comment at the beginning of pl_scanner.c seems to suggest same. yes, there are no too much possibilities, how to do it. Effective implementation needs a special PLpgSQL statement - but I am 100% happy with proposed syntax. We introduce a new special keyword just for one simple construct. A some languages has a generic PRAGMA keyword. So I would be much more happy with something like PRAGMA Assert(found); It is much more close to ADA, and it allows a reuse of new keyword for any other usage in future (your proposal is too simply, without possibility open new doors in future). And we can define a some standard predefined asserts too - like Assert, AssertNotNull, AssertNotEmpty, ... other issue - A asserts macros has one or two parameters usually. You should to support two params assert (with message). Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
On 2013-09-14 23:05, Pavel Stehule wrote: A some languages has a generic PRAGMA keyword. So I would be much more happy with something like PRAGMA Assert(found); It is much more close to ADA, and it allows a reuse of new keyword for any other usage in future (your proposal is too simply, without possibility open new doors in future). And we can define a some standard predefined asserts too - like Assert, AssertNotNull, AssertNotEmpty, ... I don't see why e.g. PRAGMA AssertNotEmpty(foo); would be better than ASSERT NotEmpty(foo); and the NotNull version is even sillier considering the expression is arbitrary SQL, and we'd have to do all kinds of different versions or people would be disappointed (AssertNull, AssertNotNull, AssertExists, AssertNotExists, etc.). I see what you're trying to do, but I don't think crippling new features just because we might do something similar at some point is a good idea. I'm guessing this is what happened with the row_count syntax, which made the feature an absolute nightmare to use. other issue - A asserts macros has one or two parameters usually. You should to support two params assert (with message). That I think is worth looking into. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
Dne 14. 9. 2013 23:35 Marko Tiikkaja ma...@joh.to napsal(a): On 2013-09-14 23:05, Pavel Stehule wrote: A some languages has a generic PRAGMA keyword. So I would be much more happy with something like PRAGMA Assert(found); It is much more close to ADA, and it allows a reuse of new keyword for any other usage in future (your proposal is too simply, without possibility open new doors in future). And we can define a some standard predefined asserts too - like Assert, AssertNotNull, AssertNotEmpty, ... I don't see why e.g. PRAGMA AssertNotEmpty(foo); would be better than ASSERT NotEmpty(foo); and the NotNull version is even sillier considering the expression is arbitrary SQL, and we'd have to do all kinds of different versions or people would be disappointed (AssertNull, AssertNotNull, AssertExists, AssertNotExists, etc.). I see what you're trying to do, but I don't think crippling new features just because we might do something similar at some point is a good idea. I'm guessing this is what happened with the row_count syntax, which made the feature an absolute nightmare to use. a more than one asserts can be my personal preferrence (it is not important). but introduction a reserved keword for one very special purpose (without extensibility) is not prudent. plpgsql has still lot of relations to pl/sql and ada, and I don't think so we have to introduce a new original syntax everytime. other issue - A asserts macros has one or two parameters usually. You should to support two params assert (with message). That I think is worth looking into. Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
Dne 14. 9. 2013 23:55 Pavel Stehule pavel.steh...@gmail.com napsal(a): Dne 14. 9. 2013 23:35 Marko Tiikkaja ma...@joh.to napsal(a): On 2013-09-14 23:05, Pavel Stehule wrote: A some languages has a generic PRAGMA keyword. So I would be much more happy with something like PRAGMA Assert(found); It is much more close to ADA, and it allows a reuse of new keyword for any other usage in future (your proposal is too simply, without possibility open new doors in future). And we can define a some standard predefined asserts too - like Assert, AssertNotNull, AssertNotEmpty, ... I don't see why e.g. PRAGMA AssertNotEmpty(foo); would be better than ASSERT NotEmpty(foo); and the NotNull version is even sillier considering the expression is arbitrary SQL, and we'd have to do all kinds of different versions or people would be disappointed (AssertNull, AssertNotNull, AssertExists, AssertNotExists, etc.). I see what you're trying to do, but I don't think crippling new features just because we might do something similar at some point is a good idea. I'm guessing this is what happened with the row_count syntax, which made the feature an absolute nightmare to use. a more than one asserts can be my personal preferrence (it is not important). but introduction a reserved keword for one very special purpose (without extensibility) is not prudent. plpgsql has still lot of relations to pl/sql and ada, and I don't think so we have to introduce a new original syntax everytime this is a possibility for introduction a new hook and possibility implement asserions and similar task in generic form (as extension). it can be assertions, tracing, profiling. I like a integrated assertions, but I would not close a door to future enhancing (probably there will not be a possibility intriduce a new keyword for tracing - although I would to implement a difference between development an production usage. so I am against to your proposal - it doesn't allow any extensibility. other issue - A asserts macros has one or two parameters usually. You should to support two params assert (with message). That I think is worth looking into. Regards, Marko Tiikkaja
Re: [HACKERS] Assertions in PL/PgSQL
On 2013-09-14 23:55, Pavel Stehule wrote: but introduction a reserved keword for one very special purpose (without extensibility) is not prudent. How about using an existing keyword then? ASSERTION used to be reserved in the SQL standard but is unreserved in postgres. CHECK might work and is fully reserved. CONSTRAINT? IS? Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
On 2013-09-15 00:09, Pavel Stehule wrote: this is a possibility for introduction a new hook and possibility implement asserions and similar task in generic form (as extension). it can be assertions, tracing, profiling. You can already do tracing and profiling in an extension. I don't see what you would put inside the function body for these two, either. Regards, Marko Tiikkaja -- 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] Assertions in PL/PgSQL
2013/9/15 Marko Tiikkaja ma...@joh.to
On 2013-09-15 00:09, Pavel Stehule wrote:
this is a possibility for introduction a new hook and possibility
implement
asserions and similar task in generic form (as extension). it can be
assertions, tracing, profiling.
You can already do tracing and profiling in an extension. I don't see
what you would put inside the function body for these two, either.
you cannot mark a tracing points explicitly in current (unsupported now)
extensions.
These functions share same pattern:
CREATE OR REPLACE FUNCTION assert(boolean)
RETURNS void AS $$
IF current_setting('plpgsq.assertions') = 'on' THEN
IF $1 THEN
RAISE EXCEPTION 'Assert fails';
END IF;
END IF;
END;
$$ LANGUAGE plpgsql;
CREATE OR REPLACE FUNCTION trace(text)
RETURNS void AS $$
IF current_setting('plpgsq.trace') = 'on' THEN
RAISE WARNING 'trace: %', $1; END IF;
END;
$$ LANGUAGE plpgsql;
Depends on usage, these functions will not be extremely slow against to
builtin solution - can be faster, if we implement it in C, and little bit
faster if we implement it as internal PLpgSQL statement. But if you use a
one not simple queries, then overhead is not significant (probably).
You have to watch some global state variable and then execute (or not) some
functionality.
Regards
Pavel
Regards,
Marko Tiikkaja
