Thread: 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
Attachment
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
<p>On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja <<a href="mailto:marko@joh.to">marko@joh.to</a>> wrote:<br /> >On 14/09/2013 20:47, I wrote:<br /> >><br /> >> These are<br /> >> similar to the Assert() backendmacro: they can be disabled during<br /> >> compile time, but when enabled, abort execution if the passed expression<br/> >> is not true.<br /> ><p>Hi,<p>That's very good, thanks.<p>><br /> > And by "compile time"here, I mean at the time when the PL/PgSQL function is<br /> > compiled, not the postgres server binary.<br /> ><p>and"compile time" means when the function is created or replaced? or the first time is used?<p>if the second. Whynot have a plpgsql.enable_assert variable?<br /> A directive you can use per function <p>then i can keep the ASSERT andactivate them by replacing the function<p>-- <br /> Jaime Casanova <a href="http://www.2ndQuadrant.com">www.2ndQuadrant.com</a><br/> Professional PostgreSQL: Soporte 24x7 y capacitación<br />Phone: +593 4 5107566 Cell: +593 987171157
On 2013-09-14 21:55, Jaime Casanova wrote: > On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja <marko@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
Hello
There is a significant issue - new reserved keyword. There is high probability so lot of users has a functions named "assert". Regards
Pavel
2013/9/14 Marko Tiikkaja <marko@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
<p><br /> El 14/09/2013 15:18, "Marko Tiikkaja" <<a href="mailto:marko@joh.to">marko@joh.to</a>> escribió:<br /> ><br/> > On 2013-09-14 21:55, Jaime Casanova wrote:<br /> >><br /> >> On Sat, Sep 14, 2013 at 1:52 PM,Marko Tiikkaja <<a href="mailto:marko@joh.to">marko@joh.to</a>> wrote:<br /> >>><br /> >>> Andby "compile time" here, I mean at the time when the PL/PgSQL function<br /> >><br /> >> is<br /> >>><br/> >>> compiled, not the postgres server binary.<br /> >>><br /> >><br /> >>and "compile time" means when the function is created or replaced? or the<br /> >> first time is used?<br />><br /> ><br /> > Uhh.. I have to admit, I'm not sure. I think this would be when you CREATE the function forthe backend that created the function, and on the first call in other backends.<br /> ><br /> ><br /> >> ifthe second. Why not have a plpgsql.enable_assert variable?<br /> >> A directive you can use per function<br /> >><br/> >> then i can keep the ASSERT and activate them by replacing the function<br /> ><br /> ><br />> The patch supports a "compiler option" to override the global option and enable it per function:<br /> ><br />> create function foof()<br /> > returns void<br /> > as $$<br /> > #enable_assertions<br /> > begin<br/> > -- failure<br /> > assert 1 > 2;<br /> > end<br /> > $$ language plpgsql;<br /> ><br /> ><br/> > Does this address your wishes?<br /> ><br /> ><p>That's exactly what i wanted. thanks<p>--<br /> JaimeCasanova<br /> 2ndQuadrant: Your PostgreSQL partner
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
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
2013/9/14 Marko Tiikkaja <marko@joh.to>
On 2013-09-14 22:40, I wrote:The comment at the beginning of pl_scanner.c seems to suggest same.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.
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
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_countsyntax, 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
<p dir="ltr"><br /> Dne 14. 9. 2013 23:35 "Marko Tiikkaja" <<a href="mailto:marko@joh.to">marko@joh.to</a>> napsal(a):<br/> ><br /> > On 2013-09-14 23:05, Pavel Stehule wrote:<br /> >><br /> >> A some languageshas a generic PRAGMA keyword. So I would be much more<br /> >> happy with something like<br /> >><br/> >> PRAGMA Assert(found);<br /> >><br /> >> It is much more close to ADA, and it allows areuse of new keyword for any<br /> >> other usage in future (your proposal is too simply, without possibility<br />>> open new doors in future). And we can define a some standard predefined<br /> >> asserts too - like Assert,AssertNotNull, AssertNotEmpty, ...<br /> ><br /> ><br /> > 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 theexpression is arbitrary SQL, and we'd have to do all kinds of different versions or people would be disappointed (AssertNull,AssertNotNull, AssertExists, AssertNotExists, etc.).<br /> ><br /> > I see what you're trying to do, butI don't think crippling new features just because we might do something similar at some point is a good idea. I'm guessingthis is what happened with the row_count syntax, which made the feature an absolute nightmare to use.<br /><p dir="ltr">amore than one asserts can be my personal preferrence (it is not important). <p dir="ltr">but introduction a reservedkeword for one very special purpose (without extensibility) is not prudent.<p dir="ltr">plpgsql has still lot ofrelations to pl/sql and ada, and I don't think so we have to introduce a new original syntax everytime.<br /><p dir="ltr">><br/> >> other issue - A asserts macros has one or two parameters usually. You<br /> >> shouldto support two params assert (with message).<br /> ><br /> ><br /> > That I think is worth looking into.<br/> ><br /> ><br /> > Regards,<br /> > Marko Tiikkaja<br />
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
<p dir="ltr"><br /> Dne 14. 9. 2013 23:55 "Pavel Stehule" <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>>napsal(a):<br /> ><br /> ><br /> > Dne 14.9. 2013 23:35 "Marko Tiikkaja" <<a href="mailto:marko@joh.to">marko@joh.to</a>> napsal(a):<br /> ><br /> >><br /> > > On 2013-09-14 23:05, Pavel Stehule wrote:<br /> > >><br /> > >> A some languageshas a generic PRAGMA keyword. So I would be much more<br /> > >> happy with something like<br /> > >><br/> > >> PRAGMA Assert(found);<br /> > >><br /> > >> It is much more close to ADA, andit allows a reuse of new keyword for any<br /> > >> other usage in future (your proposal is too simply, withoutpossibility<br /> > >> open new doors in future). And we can define a some standard predefined<br /> >>> asserts too - like Assert, AssertNotNull, AssertNotEmpty, ...<br /> > ><br /> > ><br /> > >I don't see why e.g. PRAGMA AssertNotEmpty(foo); would be better than ASSERT NotEmpty(foo); and the NotNull versionis even sillier considering the expression is arbitrary SQL, and we'd have to do all kinds of different versions orpeople would be disappointed (AssertNull, AssertNotNull, AssertExists, AssertNotExists, etc.).<br /> > ><br /> >> I see what you're trying to do, but I don't think crippling new features just because we might do something similarat some point is a good idea. I'm guessing this is what happened with the row_count syntax, which made the featurean absolute nightmare to use.<br /> ><br /> > a more than one asserts can be my personal preferrence (it isnot important).<br /> ><br /> > but introduction a reserved keword for one very special purpose (without extensibility)is not prudent.<br /> ><br /> > plpgsql has still lot of relations to pl/sql and ada, and I don't thinkso we have to introduce a new original syntax everytime<p dir="ltr">this is a possibility for introduction a new hookand possibility implement asserions and similar task in generic form (as extension). it can be assertions, tracing, profiling.<pdir="ltr">I like a integrated assertions, but I would not close a door to future enhancing (probably there willnot be a possibility intriduce a new keyword for tracing - although I would to implement a difference between developmentan production usage.<p dir="ltr">so I am against to your proposal - it doesn't allow any extensibility. <p dir="ltr">><br /> > >> other issue - A asserts macros has one or two parameters usually. You<br /> > >>should to support two params assert (with message).<br /> > ><br /> > ><br /> > > That I thinkis worth looking into.<br /> > ><br /> > ><br /> > > Regards,<br /> > > Marko Tiikkaja<br />
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
2013/9/15 Marko Tiikkaja <marko@joh.to>
On 2013-09-15 00:09, Pavel Stehule wrote: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.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 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;
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;
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
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
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
Attachment
<p><br /> El 14/09/2013 15:25, "Pavel Stehule" <<a href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>>escribió:<br /> ><br /> > Hello<br /> ><br/> > There is a significant issue - new reserved keyword. There is high probability so lot of users has a functionsnamed "assert". <br /> ><br /> > 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.<br /> ><br />> So can you redesign this without new keyword?<br /> ><p>Hi,<p>If using ASSERT as keyword is not acceptable, notthat i agree but in case. What about using RAISE EXCEPTION WHEN (condition)<p>--<br /> Jaime Casanova<br /> 2ndQuadrant:Your PostgreSQL partner
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
<p><br /> El 15/09/2013 17:13, "Marko Tiikkaja" <<a href="mailto:marko@joh.to">marko@joh.to</a>> escribió:<br /> ><br/> > On 2013-09-15 23:23, Jaime Casanova wrote:<br /> >><br /> >> If using ASSERT as keyword is notacceptable, not that i agree but in case.<br /> >> What about using RAISE EXCEPTION WHEN (condition)<br /> ><br/> ><br /> > 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.<br /> ><br /> ><p>Ah! good point, didn't think on that<p>--<br /> JaimeCasanova<br /> 2ndQuadrant: Your PostgreSQL partner
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.
Hello
a few other comments:2013/9/14 Marko Tiikkaja <marko@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
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
Hello
2013/9/18 Marko Tiikkaja <marko@joh.to>
On 2013-09-16 21:24, Pavel Stehule wrote: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?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.
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
Pavel
Regards,
Marko Tiikkaja
On 9/14/13 11:55 PM, Pavel Stehule wrote: > > > > 2013/9/15 Marko Tiikkaja <marko@joh.to <mailto:marko@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 bodyfor 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 implementit in C, and little bit faster if we implement it as internal PLpgSQL statement. But if you use a one not simplequeries, 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-graincontrol 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 assertwith 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 doX". We use it for sanity-checking things. -- Jim C. Nasby, Data Architect jim@nasby.net 512.569.9461 (cell) http://jim.nasby.net
2013/9/18 Jim Nasby <jim@nasby.net>
On 9/14/13 11:55 PM, Pavel Stehule wrote:
2013/9/15 Marko Tiikkaja <marko@joh.to <mailto:marko@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
Pavel
--
Jim C. Nasby, Data Architect jim@nasby.net
512.569.9461 (cell) http://jim.nasby.net
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
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'tyet looked at how it could be implemented. Regards, Marko Tiikkaja
2013/9/19 Marko Tiikkaja <marko@joh.to>
On 9/19/13 2:08 PM, Pavel Stehule wrote:I think so similar frameworks will be exists (we have some similarProbably 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
On Sat, Sep 14, 2013 at 6:09 PM, Marko Tiikkaja <marko@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
On 16 September 2013 03:43, Marko Tiikkaja <marko@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-hackers
On 9/20/13 12:09 PM, Amit Khandekar wrote: > On 16 September 2013 03:43, Marko Tiikkaja <marko@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
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
On Fri, Sep 20, 2013 at 6:24 AM, Marko Tiikkaja <marko@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
2013/9/20 Robert Haas <robertmhaas@gmail.com>
On Fri, Sep 20, 2013 at 6:24 AM, Marko Tiikkaja <marko@joh.to> wrote:To some extent I agree that PL/pgsql is hopeless. I think there are
>> 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.
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
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Ü
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
2013/9/20 Alvaro Herrera <alvherre@2ndquadrant.com>
Pavel Stehule escribió:Extending RAISE is one option. Another option is to decorate BEGIN and
> 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
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 Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
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; havean 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 codeto 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 reasonwhy we should not merge some function to core. I am with Marko, so some simple, user friendly statement for assertionsshould be very nice plpgsql feature. I am different in opinion how to implementat it and about syntax. I prefera possibility (not necessary currently implemented) to enhance this feature for similar tasks (as buildin or externalfeature) > > 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 usethese 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 radicallydifferent 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 whateverwe 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 jim@nasby.net 512.569.9461 (cell) http://jim.nasby.net
On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko@joh.to> wrote: > On 9/20/13 12:09 PM, Amit Khandekar wrote: >> >> On 16 September 2013 03:43, Marko Tiikkaja <marko@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
<p><br /> El 21/09/2013 17:16, "Jaime Casanova" <<a href="mailto:jaime@2ndquadrant.com">jaime@2ndquadrant.com</a>>escribió:<br /> ><br /> > On Fri, Sep 20, 2013 at5:17 AM, Marko Tiikkaja <<a href="mailto:marko@joh.to">marko@joh.to</a>> wrote:<br /> > > On 9/20/13 12:09PM, Amit Khandekar wrote:<br /> > >><br /> > >> On 16 September 2013 03:43, Marko Tiikkaja <<ahref="mailto:marko@joh.to">marko@joh.to</a>> wrote:<br /> > >>><br /> > >>> I think it wouldbe extremely surprising if a command like that got<br /> > >>> optimized away based on a GUC, so I don'tthink that would be a good<br /> > >>> idea.<br /> > >><br /> > >><br /> > >><br/> > >> In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for<br /> > >> ASSERT,and then return NULL if plpgsql_curr_compile->enable_assertions is<br /> > >> false. Isn't this possible?<br /> > ><br /> > ><br /> > > Of course it's possible. But I, as a PostgreSQL user writing PL/PgSQLcode,<br /> > > would be extremely surprised if this new cool option to RAISE didn't work<br /> > > forsome reason. If we use ASSERT the situation is different; most people<br /> > > will realize it's a new commandand works differently from RAISE.<br /> > ><br /> > ><br /> ><br /> > What about just adding a clauseWHEN to the RAISE statement and use<br /> > the level machinery (client_min_messages) to make it appear or not<br/> > of course, this has the disadvantage that an EXCEPTION level will<br /> > always happen... or you can makeit a new loglevel that mean EXCEPTION<br /> > if asserts_enabled<br /> ><p> meaning RAISE ASSERT of course<p>--<br/> Jaime Casanova<br /> 2ndQuadrant: Your PostgreSQL partner
2013/9/22 Jaime Casanova <jaime@2ndquadrant.com>
El 21/09/2013 17:16, "Jaime Casanova" <jaime@2ndquadrant.com> escribió:
>
> On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko@joh.to> wrote:
> > On 9/20/13 12:09 PM, Amit Khandekar wrote:
> >>
> >> On 16 September 2013 03:43, Marko Tiikkaja <marko@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
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
2013/9/23 Marko Tiikkaja <marko@joh.to>
On 9/23/13 6:40 AM, Pavel Stehule wrote:It looks like I'm losing this battle, but this syntax isn't too bad.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
I don't win too, but result is good :)
This sounds like a decent enhancement.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 idea is good, I like it.p.s. clause WHEN can be used for other exception level - so it can be a
interesting shortcut for other use cases.
I could prepare a patch for this, unless someone else wants to?
please, do it.
Regards
Pavel
Pavel
Regards,
Marko Tiikkaja
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
On 23 September 2013 10:10, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2013/9/22 Jaime Casanova <jaime@2ndquadrant.com>
El 21/09/2013 17:16, "Jaime Casanova" <jaime@2ndquadrant.com> escribió:
>
> On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko@joh.to> wrote:
> > On 9/20/13 12:09 PM, Amit Khandekar wrote:
> >>
> >> On 16 September 2013 03:43, Marko Tiikkaja <marko@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 solutionsyntax - enhanced current RAISERAISE ASSERT WHEN boolean expressionRAISE ASSERT 'some message' WHEN expressionand we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql optionsassert_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?RegardsPavelp.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
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
2013/9/23 Amit Khandekar <amit.khandekar@enterprisedb.com>
On 23 September 2013 10:10, Pavel Stehule <pavel.stehule@gmail.com> wrote:2013/9/22 Jaime Casanova <jaime@2ndquadrant.com>
El 21/09/2013 17:16, "Jaime Casanova" <jaime@2ndquadrant.com> escribió:
>
> On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko@joh.to> wrote:
> > On 9/20/13 12:09 PM, Amit Khandekar wrote:
> >>
> >> On 16 September 2013 03:43, Marko Tiikkaja <marko@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 solutionsyntax - enhanced current RAISERAISE ASSERT WHEN boolean expressionRAISE ASSERT 'some message' WHEN expressionand we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql optionsassert_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?RegardsPavelp.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
2013/9/23 Marko Tiikkaja <marko@joh.to>
On 9/23/13 10:50 AM, I wrote: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.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.
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
2013/9/23 Andres Freund <andres@2ndquadrant.com>
RAISE ASSERT ... assert_level = PANIC :P.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.
:) maybe some little bit less than PANIC
Pavel
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
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
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
2013/9/23 Marko Tiikkaja <marko@joh.to>
On 9/23/13 11:12 AM, I wrote:Should've probably proofread that one. I meant: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;
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
On 23 September 2013 14:33, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2013/9/23 Amit Khandekar <amit.khandekar@enterprisedb.com>On 23 September 2013 10:10, Pavel Stehule <pavel.stehule@gmail.com> wrote:2013/9/22 Jaime Casanova <jaime@2ndquadrant.com>
El 21/09/2013 17:16, "Jaime Casanova" <jaime@2ndquadrant.com> escribió:
>
> On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko@joh.to> wrote:
> > On 9/20/13 12:09 PM, Amit Khandekar wrote:
> >>
> >> On 16 September 2013 03:43, Marko Tiikkaja <marko@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 solutionsyntax - enhanced current RAISERAISE ASSERT WHEN boolean expressionRAISE ASSERT 'some message' WHEN expressionand we can have a GUC that controls asserts per database - possibly overwritten by plpgsql option - similar to current plpgsql optionsassert_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?RegardsPavelp.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
On Mon, Sep 23, 2013 at 5:48 AM, Amit Khandekar <amit.khandekar@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
Hello
2013/9/24 Robert Haas <robertmhaas@gmail.com>
On Mon, Sep 23, 2013 at 5:48 AM, Amit KhandekarI think that's right: ISTM that at this point there are two different
<amit.khandekar@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.
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
Attachment
On Wed, Oct 9, 2013 at 12:57 PM, Pavel Stehule <pavel.stehule@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
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.
rebased patch
Regards2013/11/14 Peter Eisentraut <peter_e@gmx.net>
On Wed, 2013-10-09 at 18:57 +0200, Pavel Stehule wrote:Your patch needs to be rebased.
> here is a patch for RAISE WHEN clause
Attachment
Pavel Stehule <pavel.stehule@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
2013/11/17 Tom Lane <tgl@sss.pgh.pa.us>
Pavel Stehule <pavel.stehule@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
On Sun, Nov 17, 2013 at 5:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Pavel Stehule <pavel.stehule@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
2013/11/19 Robert Haas <robertmhaas@gmail.com>
This is a fair point. I think the goal was to get to RAISE ASSERTOn Sun, Nov 17, 2013 at 5:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Pavel Stehule <pavel.stehule@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?
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
On Tue, Nov 19, 2013 at 10:59 AM, Pavel Stehule <pavel.stehule@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
2013/11/19 Robert Haas <robertmhaas@gmail.com>
On Tue, Nov 19, 2013 at 10:59 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:I think that's true, but not particularly relevant. CTEs are
>> 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.
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
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.
<div dir="ltr"><br /><div class="gmail_extra"><br /><br /><div class="gmail_quote">2013/11/27 Peter Eisentraut <span dir="ltr"><<ahref="mailto:peter_e@gmx.net" target="_blank">peter_e@gmx.net</a>></span><br /><blockquote class="gmail_quote"style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Tue, 2013-11-19at 10:40 -0500, Robert Haas wrote:<br /> > I think the goal was to get to RAISE ASSERT<br /> > WHEN ...;then, if assertions are off, you do nothing; if they're on,<br /> > you error. IF condition THEN RAISE..." isn'ta suitable surrogate in<br /> > that case because you incur the overhead of testing the condition<br /> > regardless.<br/><br /></div>So if I do RAISE ASSERT WHEN condition and assertions are off, then<br /> condition wouldn'teven be evaluated? But what about RAISE NOTICE WHEN,<br /> when log_min_messages is error? What about the sideeffects of the<br /> format string? This is all just getting too weird.<br /><br /> I don't see anything wrong withconsidering a separate ASSERT command<br /> with its own semantics, like in many other programming languages.<br /><br/><br /></blockquote></div>My objection against ASSERT command was one - it was too simply (against to cost of possiblecollision from introduction new (wide used) keyword.<br /><br /></div><div class="gmail_extra">I can live with ASSERTstatement - but I expect as minimum a possibility to specify level (failure, tracing, ...) and specify a message relatedto assert. Assert with only expression is not enough.<br /><br /></div><div class="gmail_extra">Regards<br /><br />Pavel<br /></div></div>