Re: 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

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

On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja <marko(at)joh(dot)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

On 2013-09-14 21:55, Jaime Casanova wrote:
> On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja <marko(at)joh(dot)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".

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 <marko(at)joh(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

El 14/09/2013 15:18, "Marko Tiikkaja" <marko(at)joh(dot)to> escribió:
>
> On 2013-09-14 21:55, Jaime Casanova wrote:
>>
>> On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja <marko(at)joh(dot)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

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(at)joh(dot)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
>

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

Dne 14. 9. 2013 23:35 "Marko Tiikkaja" <marko(at)joh(dot)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

Dne 14. 9. 2013 23:55 "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com> napsal(a):
>
>
> Dne 14. 9. 2013 23:35 "Marko Tiikkaja" <marko(at)joh(dot)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

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

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(at)joh(dot)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
>

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

El 14/09/2013 15:25, "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)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

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

El 15/09/2013 17:13, "Marko Tiikkaja" <marko(at)joh(dot)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

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:

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 <marko(at)joh(dot)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(at)postgresql(dot)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(at)joh(dot)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
>

On 9/14/13 11:55 PM, Pavel Stehule wrote:
>
>
>
> 2013/9/15 Marko Tiikkaja <marko(at)joh(dot)to <mailto:marko(at)joh(dot)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 jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

2013/9/18 Jim Nasby <jim(at)nasby(dot)net>

> On 9/14/13 11:55 PM, Pavel Stehule wrote:
>
>>
>>
>>
>> 2013/9/15 Marko Tiikkaja <marko(at)joh(dot)to <mailto:marko(at)joh(dot)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 jim(at)nasby(dot)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't yet looked at how it
could be implemented.

Regards,
Marko Tiikkaja

2013/9/19 Marko Tiikkaja <marko(at)joh(dot)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
>

On Sat, Sep 14, 2013 at 6:09 PM, Marko Tiikkaja <marko(at)joh(dot)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(at)joh(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/**mailpref/pgsql-hackers<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(at)joh(dot)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(at)joh(dot)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(at)gmail(dot)com>

> On Fri, Sep 20, 2013 at 6:24 AM, Marko Tiikkaja <marko(at)joh(dot)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
>

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(at)2ndquadrant(dot)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 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; 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 jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> On 9/20/13 12:09 PM, Amit Khandekar wrote:
>>
>> On 16 September 2013 03:43, Marko Tiikkaja <marko(at)joh(dot)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

El 21/09/2013 17:16, "Jaime Casanova" <jaime(at)2ndquadrant(dot)com> escribió:
>
> On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> > On 9/20/13 12:09 PM, Amit Khandekar wrote:
> >>
> >> On 16 September 2013 03:43, Marko Tiikkaja <marko(at)joh(dot)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

2013/9/22 Jaime Casanova <jaime(at)2ndquadrant(dot)com>

>
> El 21/09/2013 17:16, "Jaime Casanova" <jaime(at)2ndquadrant(dot)com> escribió:
>
> >
> > On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> > > On 9/20/13 12:09 PM, Amit Khandekar wrote:
> > >>
> > >> On 16 September 2013 03:43, Marko Tiikkaja <marko(at)joh(dot)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(at)joh(dot)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
>

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(dot)stehule(at)gmail(dot)com> wrote:

>
>
>
> 2013/9/22 Jaime Casanova <jaime(at)2ndquadrant(dot)com>
>
>>
>> El 21/09/2013 17:16, "Jaime Casanova" <jaime(at)2ndquadrant(dot)com> escribió:
>>
>> >
>> > On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
>> > > On 9/20/13 12:09 PM, Amit Khandekar wrote:
>> > >>
>> > >> On 16 September 2013 03:43, Marko Tiikkaja <marko(at)joh(dot)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
>>
>
>

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(dot)khandekar(at)enterprisedb(dot)com>

>
>
>
> On 23 September 2013 10:10, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>>
>>
>>
>> 2013/9/22 Jaime Casanova <jaime(at)2ndquadrant(dot)com>
>>
>>>
>>> El 21/09/2013 17:16, "Jaime Casanova" <jaime(at)2ndquadrant(dot)com> escribió:
>>>
>>> >
>>> > On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
>>> > > On 9/20/13 12:09 PM, Amit Khandekar wrote:
>>> > >>
>>> > >> On 16 September 2013 03:43, Marko Tiikkaja <marko(at)joh(dot)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
>>>
>>
>>
>

2013/9/23 Marko Tiikkaja <marko(at)joh(dot)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
>

2013/9/23 Andres Freund <andres(at)2ndquadrant(dot)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
>

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(at)joh(dot)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
>

On 23 September 2013 14:33, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

>
>
>
> 2013/9/23 Amit Khandekar <amit(dot)khandekar(at)enterprisedb(dot)com>
>
>>
>>
>>
>> On 23 September 2013 10:10, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>wrote:
>>
>>>
>>>
>>>
>>> 2013/9/22 Jaime Casanova <jaime(at)2ndquadrant(dot)com>
>>>
>>>>
>>>> El 21/09/2013 17:16, "Jaime Casanova" <jaime(at)2ndquadrant(dot)com> escribió:
>>>>
>>>> >
>>>> > On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
>>>> > > On 9/20/13 12:09 PM, Amit Khandekar wrote:
>>>> > >>
>>>> > >> On 16 September 2013 03:43, Marko Tiikkaja <marko(at)joh(dot)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
>>>>
>>>
>>>
>>
>

On Mon, Sep 23, 2013 at 5:48 AM, Amit Khandekar
<amit(dot)khandekar(at)enterprisedb(dot)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(at)gmail(dot)com>

> On Mon, Sep 23, 2013 at 5:48 AM, Amit Khandekar
> <amit(dot)khandekar(at)enterprisedb(dot)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
>

On Wed, Oct 9, 2013 at 12:57 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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

Regards

Pavel

2013/11/14 Peter Eisentraut <peter_e(at)gmx(dot)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.
>
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)us>

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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(at)gmail(dot)com>

> On Sun, Nov 17, 2013 at 5:10 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

On Tue, Nov 19, 2013 at 10:59 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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(at)gmail(dot)com>

> On Tue, Nov 19, 2013 at 10:59 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

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.

2013/11/27 Peter Eisentraut <peter_e(at)gmx(dot)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