Re: proposal: plpgsql - Assert statement

Hello

Assert is usually implemented as custom functions and used via PERFORM
statement now

-- usual current solution
PERFORM Assert(some expression)

I would to implement Assert as plpgsql internal statement due bigger
possibilities to design syntax and internal implementation now and in
future. More - as plpgsql statement should be compatible with any current
code - because there should not be collision between SQL and PLpgSQL space.
So this design doesn't break any current code.

I propose following syntax with following ecosystem:

ASSERT [ NOTICE, WARNING, >>EXCEPTION<< ]
[ string expression or literal - explicit message ]
[ USING clause - same as RAISE stmt (possible in future ) ]
( ROW_COUNT ( = | <> ) ( 1 | 0 ) |
( QUERY some query should not be empty ) |
( CHECK some expression should be true )
( IS NOT NULL expression should not be null )

Every variant (ROW_COUNT, QUERY, CHECK, IS NOT NULL) has own default message

These asserts can be controlled by set of options (by default asserts are
enabled):

#option asserts_disable
#option asserts_disable_notice .. don't check thin asserts
#option asserts_not_stop .. raise warning instead exception

some examples:

UPDATE tab SET c = 1 WHERE pk = somevar;
ASSERT ROW_COUNT = 1; -- knows what is previous DML or Dynamic DML

ASSERT CHECK a < 100;

ASSERT IS NOT NULL pk;

ASSERT QUERY SELECT id FROM tab WHERE x = 1;

ASSERT CHECK 2 = (SELECT count(*) FROM tab WHERE x = 1);

ASSERT WARNING "data are there" QUERY SELECT ...

Shorter variant should to work

CREATE OR REPLACE FUNCTION assert(boolean)
RETURNS void AS $$
BEGIN
ASSERT CHECK $1;
END;
$$ LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION assert(boolean, text)
RETURNS void AS $$
BEGIN
ASSERT $1 CHECK $2;
END;
$$ LANGUAGE plpgsql;

Usage:

PERFORM assert(a <> 10);
PERFORM assert(a <> 10, "a should be 10");

Comments, notices?

Regards

Pavel

This design should not break any current solution, it allows a static
analyses, and it doesn't close a door for future enhancing.

On 2014-09-05 08:16, Pavel Stehule wrote:
> Assert is usually implemented as custom functions and used via PERFORM
> statement now
>
> -- usual current solution
> PERFORM Assert(some expression)
>
> I would to implement Assert as plpgsql internal statement due bigger
> possibilities to design syntax and internal implementation now and in
> future. More - as plpgsql statement should be compatible with any current
> code - because there should not be collision between SQL and PLpgSQL space.
> So this design doesn't break any current code.

It does require making ASSERT an unreserved keyword, no? That would
break code where someone used "assert" as a variable name, for example.

> I propose following syntax with following ecosystem:
>
> ASSERT [ NOTICE, WARNING, >>EXCEPTION<< ]
> [ string expression or literal - explicit message ]
> [ USING clause - same as RAISE stmt (possible in future ) ]
> ( ROW_COUNT ( = | <> ) ( 1 | 0 ) |
> ( QUERY some query should not be empty ) |
> ( CHECK some expression should be true )
> ( IS NOT NULL expression should not be null )

> UPDATE tab SET c = 1 WHERE pk = somevar;
> ASSERT ROW_COUNT = 1; -- knows what is previous DML or Dynamic DML
> ASSERT CHECK a < 100;
> ASSERT IS NOT NULL pk;
> ASSERT QUERY SELECT id FROM tab WHERE x = 1;
> ASSERT CHECK 2 = (SELECT count(*) FROM tab WHERE x = 1);

I don't see the need for specialized syntax. If the syntax was just
ASSERT (<expr>), these could be written as:

ASSERT (row_count = 1); -- assuming we provide a special variable
instead of having to do GET DIAGNOSTICS
ASSERT (a < 100); -- or perhaps ASSERT((a < 100) IS TRUE); depending on
how NULLs are handled
ASSERT (pk IS NOT NULL);
ASSERT (EXISTS(SELECT id FROM tab WHERE x = 1));
ASSERT (2 = (SELECT count(*) FROM tab WHERE x = 1));

the idea being that it gets turned into SELECT <expr>; and then evaluated.

> ASSERT WARNING "data are there" QUERY SELECT ...

I think this could still be parsed correctly, though I'm not 100% sure
on that:

ASSERT WARNING (EXISTS(SELECT ..)), 'data are there';

For extra points the error detail could work similarly to
print_strict_params. e.g. ASSERT(row_count = 1); would display the
value of row_count in the DETAIL line, since row_count was a parameter
to the expression.

.marko

2014-09-05 9:52 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 2014-09-05 08:16, Pavel Stehule wrote:
>
>> Assert is usually implemented as custom functions and used via PERFORM
>> statement now
>>
>> -- usual current solution
>> PERFORM Assert(some expression)
>>
>> I would to implement Assert as plpgsql internal statement due bigger
>> possibilities to design syntax and internal implementation now and in
>> future. More - as plpgsql statement should be compatible with any current
>> code - because there should not be collision between SQL and PLpgSQL
>> space.
>> So this design doesn't break any current code.
>>
>
> It does require making ASSERT an unreserved keyword, no? That would break
> code where someone used "assert" as a variable name, for example.
>

sure, sorry

>
> I propose following syntax with following ecosystem:
>>
>> ASSERT [ NOTICE, WARNING, >>EXCEPTION<< ]
>> [ string expression or literal - explicit message ]
>> [ USING clause - same as RAISE stmt (possible in future ) ]
>> ( ROW_COUNT ( = | <> ) ( 1 | 0 ) |
>> ( QUERY some query should not be empty ) |
>> ( CHECK some expression should be true )
>> ( IS NOT NULL expression should not be null )
>>
>
> UPDATE tab SET c = 1 WHERE pk = somevar;
>> ASSERT ROW_COUNT = 1; -- knows what is previous DML or Dynamic DML
>> ASSERT CHECK a < 100;
>> ASSERT IS NOT NULL pk;
>> ASSERT QUERY SELECT id FROM tab WHERE x = 1;
>> ASSERT CHECK 2 = (SELECT count(*) FROM tab WHERE x = 1);
>>
>
> I don't see the need for specialized syntax. If the syntax was just
> ASSERT (<expr>), these could be written as:
>
> ASSERT (row_count = 1); -- assuming we provide a special variable instead
> of having to do GET DIAGNOSTICS
> ASSERT (a < 100); -- or perhaps ASSERT((a < 100) IS TRUE); depending on
> how NULLs are handled
> ASSERT (pk IS NOT NULL);
> ASSERT (EXISTS(SELECT id FROM tab WHERE x = 1));
> ASSERT (2 = (SELECT count(*) FROM tab WHERE x = 1));
>

I disagree. Your design is expression based design with following
disadvantages:

a) there is only one possible default message -- "Assertation fault"

b) there is not possibility to show statement for ASSERT ROW_COUNT

c) any static analyse is blocked, because there is not clean semantic

d) In PLpgSQL language a syntax STATEMENT '(' expression ')' is new - there
is nothing yet --- it is discuss from yesterday -- still I am speaking
about plpgsql -- I don't would to refactor plpgsql parser.

e) for your proposal we don't need any special - you can do it as custom
function - then there is no sense to define it. Maximally it can live as
some extension in some shared repository

>
> the idea being that it gets turned into SELECT <expr>; and then
> evaluated.
>
> ASSERT WARNING "data are there" QUERY SELECT ...
>>
>
> I think this could still be parsed correctly, though I'm not 100% sure on
> that:
>
> ASSERT WARNING (EXISTS(SELECT ..)), 'data are there';
>

PLpgSQL uses a ';' or some plpgsql keyword as SQL statement delimiter. It
reason why RETURN QUERY ... ';' So in this case can practical to place SQL
statement on the end of plpgsql statement.

parenthesis are not practical, because it is hard to identify bug ..

A simplicity of integration SQL and PLpgSQL is in using "smart" keywords -
It is more verbose, and it allow to well diagnostics

>
> For extra points the error detail could work similarly to
> print_strict_params. e.g. ASSERT(row_count = 1); would display the value
> of row_count in the DETAIL line, since row_count was a parameter to the
> expression.
>
>
>
> .marko
>

On 9/5/14 10:09 AM, Pavel Stehule wrote:
>> I think this could still be parsed correctly, though I'm not 100% sure on
>> that:
>>
>> ASSERT WARNING (EXISTS(SELECT ..)), 'data are there';
>>
>
> PLpgSQL uses a ';' or some plpgsql keyword as SQL statement delimiter. It
> reason why RETURN QUERY ... ';' So in this case can practical to place SQL
> statement on the end of plpgsql statement.

*shrug* There are lots of cases where a comma is used as well, e.g.
RAISE NOTICE '..', <expr>, <expr>;

> parenthesis are not practical, because it is hard to identify bug ..

I don't see why. The PL/PgSQL SQL parser goes to great lengths to
identify unmatched parenthesis. But the parens probably aren't
necessary in the first place; you could just omit them and keep parsing
until the next comma AFAICT. So the syntax would be:

RAISE [ NOTICE | WARNING | EXCEPTION/ASSERT/WHATEVER ]
boolean_expr [, error_message [, error_message_param [, ... ] ] ];

> A simplicity of integration SQL and PLpgSQL is in using "smart" keywords -
> It is more verbose, and it allow to well diagnostics

I disagree. The new keywords provide nothing of value here. They even
encourage the use of quirky syntax in *exchange* for verbosity ("IS NOT
NULL pk"? really?).

.marko

2014-09-05 10:33 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 9/5/14 10:09 AM, Pavel Stehule wrote:
>
>> I think this could still be parsed correctly, though I'm not 100% sure on
>>> that:
>>>
>>> ASSERT WARNING (EXISTS(SELECT ..)), 'data are there';
>>>
>>>
>> PLpgSQL uses a ';' or some plpgsql keyword as SQL statement delimiter. It
>> reason why RETURN QUERY ... ';' So in this case can practical to place SQL
>> statement on the end of plpgsql statement.
>>
>
> *shrug* There are lots of cases where a comma is used as well, e.g. RAISE
> NOTICE '..', <expr>, <expr>;
>
> parenthesis are not practical, because it is hard to identify bug ..
>>
>
> I don't see why. The PL/PgSQL SQL parser goes to great lengths to
> identify unmatched parenthesis. But the parens probably aren't necessary
> in the first place; you could just omit them and keep parsing until the
> next comma AFAICT. So the syntax would be:
>
> RAISE [ NOTICE | WARNING | EXCEPTION/ASSERT/WHATEVER ]
> boolean_expr [, error_message [, error_message_param [, ... ] ] ];
>

extension RAISE about ASSERT level has minimal new value

>
> A simplicity of integration SQL and PLpgSQL is in using "smart" keywords -
>> It is more verbose, and it allow to well diagnostics
>>
>
> I disagree. The new keywords provide nothing of value here. They even
> encourage the use of quirky syntax in *exchange* for verbosity ("IS NOT
> NULL pk"? really?).
>

It is about semantic and conformity of proposed tools. Sure, all can
reduced to ASSERT(expr) .. but where is some benefit against function call

I am able to do without any change of language as plpgsql extension - there
is no necessary to do any change for too thin proposal

>
>
> .marko
>

On 9/5/14 10:40 AM, Pavel Stehule wrote:
> 2014-09-05 10:33 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:
>> I don't see why. The PL/PgSQL SQL parser goes to great lengths to
>> identify unmatched parenthesis. But the parens probably aren't necessary
>> in the first place; you could just omit them and keep parsing until the
>> next comma AFAICT. So the syntax would be:
>>
>> RAISE [ NOTICE | WARNING | EXCEPTION/ASSERT/WHATEVER ]
>> boolean_expr [, error_message [, error_message_param [, ... ] ] ];
>>
>
> extension RAISE about ASSERT level has minimal new value

Oops. I meant to type ASSERT there, instead of RAISE. Does that make
more sense?

>> I disagree. The new keywords provide nothing of value here. They even
>> encourage the use of quirky syntax in *exchange* for verbosity ("IS NOT
>> NULL pk"? really?).
>>
>
> It is about semantic and conformity of proposed tools. Sure, all can
> reduced to ASSERT(expr) .. but where is some benefit against function call

I see several benefits:

1) Standard syntax, available anywhere
2) Since the RAISE EXCEPTION happens at the caller's site, we can
provide information not available to an ordinary function, such as the
values of the parameters passed to it
3) We can make the exception uncatchable
4) ASSERTs can be easily disabled (if we choose to allow that), even
per-function

> I am able to do without any change of language as plpgsql extension - there
> is no necessary to do any change for too thin proposal

What *exactly* about my proposal is "too thin"? What does your thing do
that mine doesn't? If you're saying your suggestion allows us to give a
better error message, I disagree:

( ROW_COUNT ( = | <> ) ( 1 | 0 ) |

I've already addressed this: we can print the parameters and their
values automatically, so printing the row count here doesn't give any
additional value.

( QUERY some query should not be empty ) |

With this definition, absolutely zero value over ASSERT EXISTS(..);

( CHECK some expression should be true )

No additional value; it's either NULL, FALSE or TRUE and both syntaxes
can display what the expression evaluated to.

( IS NOT NULL expression should not be null )

It's either NULL or it isn't. No additional value.

.marko

2014-09-05 10:57 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 9/5/14 10:40 AM, Pavel Stehule wrote:
>
>> 2014-09-05 10:33 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:
>>
>>> I don't see why. The PL/PgSQL SQL parser goes to great lengths to
>>> identify unmatched parenthesis. But the parens probably aren't necessary
>>> in the first place; you could just omit them and keep parsing until the
>>> next comma AFAICT. So the syntax would be:
>>>
>>> RAISE [ NOTICE | WARNING | EXCEPTION/ASSERT/WHATEVER ]
>>> boolean_expr [, error_message [, error_message_param [, ... ] ] ];
>>>
>>>
>> extension RAISE about ASSERT level has minimal new value
>>
>
> Oops. I meant to type ASSERT there, instead of RAISE. Does that make
> more sense?
>

for me a basic limit is boolean_expr

>
> I disagree. The new keywords provide nothing of value here. They even
>>> encourage the use of quirky syntax in *exchange* for verbosity ("IS NOT
>>> NULL pk"? really?).
>>>
>>>
>> It is about semantic and conformity of proposed tools. Sure, all can
>> reduced to ASSERT(expr) .. but where is some benefit against function call
>>
>
> I see several benefits:
>
> 1) Standard syntax, available anywhere
> 2) Since the RAISE EXCEPTION happens at the caller's site, we can
> provide information not available to an ordinary function, such as the
> values of the parameters passed to it
> 3) We can make the exception uncatchable
> 4) ASSERTs can be easily disabled (if we choose to allow that), even
> per-function
>

All points can be solved by extension on PGXN .. and your proposed syntax
can be better processed on Postgres level than PLpgSQL level.

>
> I am able to do without any change of language as plpgsql extension -
>> there
>> is no necessary to do any change for too thin proposal
>>
>
> What *exactly* about my proposal is "too thin"? What does your thing do
> that mine doesn't? If you're saying your suggestion allows us to give a
> better error message, I disagree:
>

"Too thin" it means so there is not possibility to extensibility,
possibility to define dafaults, possibility to use it for static analyse.

>
>
> ( ROW_COUNT ( = | <> ) ( 1 | 0 ) |
>
> I've already addressed this: we can print the parameters and their values
> automatically, so printing the row count here doesn't give any additional
> value.
>

In this case, I can use a plpgsql parser for static analyse - I can do
warning, if related query has not filter PK and similar.

>
> ( QUERY some query should not be empty ) |
>
> With this definition, absolutely zero value over ASSERT EXISTS(..);
>
> ( CHECK some expression should be true )
>
> No additional value; it's either NULL, FALSE or TRUE and both syntaxes can
> display what the expression evaluated to.
>
> ( IS NOT NULL expression should not be null )
>
> It's either NULL or it isn't. No additional value.
>
>
>
> .marko
>

On 9/5/14 11:08 AM, Pavel Stehule wrote:
> 2014-09-05 10:57 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:
>> Oops. I meant to type ASSERT there, instead of RAISE. Does that make
>> more sense?
>
> for me a basic limit is boolean_expr

I don't understand what you mean by this.

>>> It is about semantic and conformity of proposed tools. Sure, all can
>>> reduced to ASSERT(expr) .. but where is some benefit against function call
>>>
>>
>> I see several benefits:
>>
>> 1) Standard syntax, available anywhere
>> 2) Since the RAISE EXCEPTION happens at the caller's site, we can
>> provide information not available to an ordinary function, such as the
>> values of the parameters passed to it
>> 3) We can make the exception uncatchable
>> 4) ASSERTs can be easily disabled (if we choose to allow that), even
>> per-function
>>
>
> All points can be solved by extension on PGXN ..

#3 probably. Being able to satisfy #1 through PGXN is ridiculous. #2
and #4 would require at least hooking into PL/PgSQL, which might be
possible, but it's intrusive and honestly feels fragile.

But perhaps more to the point, why would that criticism apply to my
suggestion, but not yours? Why don't we just reject any ASSERT syntax?

> and your proposed syntax
> can be better processed on Postgres level than PLpgSQL level.

*shrug* Doing it in SQL would probably break more stuff. I'm trying to
contain the damage. And arguably, this is mostly only useful in PL/PgSQL.

>> I am able to do without any change of language as plpgsql extension -
>>> there
>>> is no necessary to do any change for too thin proposal
>>>
>>
>> What *exactly* about my proposal is "too thin"? What does your thing do
>> that mine doesn't? If you're saying your suggestion allows us to give a
>> better error message, I disagree:
>>
>
> "Too thin" it means so there is not possibility to extensibility,
> possibility to define dafaults, possibility to use it for static analyse.

Again, why does this criticism only apply to my suggestion and not yours?

>> ( ROW_COUNT ( = | <> ) ( 1 | 0 ) |
>>
>> I've already addressed this: we can print the parameters and their values
>> automatically, so printing the row count here doesn't give any additional
>> value.
>>
>
> In this case, I can use a plpgsql parser for static analyse - I can do
> warning, if related query has not filter PK and similar.

You can do that anyway, you just need to work a bit harder. I don't see
the problem, really.

.marko

2014-09-05 11:17 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 9/5/14 11:08 AM, Pavel Stehule wrote:
>
>> 2014-09-05 10:57 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:
>>
>>> Oops. I meant to type ASSERT there, instead of RAISE. Does that make
>>> more sense?
>>>
>>
>> for me a basic limit is boolean_expr
>>
>
> I don't understand what you mean by this.
>
> It is about semantic and conformity of proposed tools. Sure, all can
>>>> reduced to ASSERT(expr) .. but where is some benefit against function
>>>> call
>>>>
>>>>
>>> I see several benefits:
>>>
>>> 1) Standard syntax, available anywhere
>>> 2) Since the RAISE EXCEPTION happens at the caller's site, we can
>>> provide information not available to an ordinary function, such as the
>>> values of the parameters passed to it
>>> 3) We can make the exception uncatchable
>>> 4) ASSERTs can be easily disabled (if we choose to allow that), even
>>> per-function
>>>
>>>
>> All points can be solved by extension on PGXN ..
>>
>
> #3 probably. Being able to satisfy #1 through PGXN is ridiculous. #2 and
> #4 would require at least hooking into PL/PgSQL, which might be possible,
> but it's intrusive and honestly feels fragile.
>
> But perhaps more to the point, why would that criticism apply to my
> suggestion, but not yours? Why don't we just reject any ASSERT syntax?
>
> and your proposed syntax
>> can be better processed on Postgres level than PLpgSQL level.
>>
>
> *shrug* Doing it in SQL would probably break more stuff. I'm trying to
> contain the damage. And arguably, this is mostly only useful in PL/PgSQL.
>
> I am able to do without any change of language as plpgsql extension -
>>>
>>>> there
>>>> is no necessary to do any change for too thin proposal
>>>>
>>>>
>>> What *exactly* about my proposal is "too thin"? What does your thing do
>>> that mine doesn't? If you're saying your suggestion allows us to give a
>>> better error message, I disagree:
>>>
>>>
>> "Too thin" it means so there is not possibility to extensibility,
>> possibility to define dafaults, possibility to use it for static analyse.
>>
>
> Again, why does this criticism only apply to my suggestion and not yours?
>
>
There is one stronger difference - there is clean, what is targer of
assertation, because there is defined semantic.

When all is just any expression, there is nothing specified semantic

> ( ROW_COUNT ( = | <> ) ( 1 | 0 ) |
>>>
>>> I've already addressed this: we can print the parameters and their values
>>> automatically, so printing the row count here doesn't give any additional
>>> value.
>>>
>>>
>> In this case, I can use a plpgsql parser for static analyse - I can do
>> warning, if related query has not filter PK and similar.
>>
>
> You can do that anyway, you just need to work a bit harder. I don't see
> the problem, really.
>

bit harder, with possibility to false identification

>
>
> .marko
>

On 09/05/2014 04:40 AM, Pavel Stehule wrote:
>
>
>
> 2014-09-05 10:33 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to
> <mailto:marko(at)joh(dot)to>>:
>
> On 9/5/14 10:09 AM, Pavel Stehule wrote:
>
> I think this could still be parsed correctly, though I'm not
> 100% sure on
> that:
>
> ASSERT WARNING (EXISTS(SELECT ..)), 'data are there';
>
>
> PLpgSQL uses a ';' or some plpgsql keyword as SQL statement
> delimiter. It
> reason why RETURN QUERY ... ';' So in this case can practical to
> place SQL
> statement on the end of plpgsql statement.
>
>
> *shrug* There are lots of cases where a comma is used as well, e.g.
> RAISE NOTICE '..', <expr>, <expr>;
>
> parenthesis are not practical, because it is hard to identify bug ..
>
>
> I don't see why. The PL/PgSQL SQL parser goes to great lengths to
> identify unmatched parenthesis. But the parens probably aren't
> necessary in the first place; you could just omit them and keep
> parsing until the next comma AFAICT. So the syntax would be:
>
> RAISE [ NOTICE | WARNING | EXCEPTION/ASSERT/WHATEVER ]
> boolean_expr [, error_message [, error_message_param [, ... ] ] ];
>
>
> extension RAISE about ASSERT level has minimal new value

Adding a WHEN clause to RAISE would have the benefit of not needing any
new keywords at all.

RAISE EXCEPTION 'format' [, expr ...] WHEN row_count <> 1;

Regards,
Jan

>
>
> A simplicity of integration SQL and PLpgSQL is in using "smart"
> keywords -
> It is more verbose, and it allow to well diagnostics
>
>
> I disagree. The new keywords provide nothing of value here. They
> even encourage the use of quirky syntax in *exchange* for verbosity
> ("IS NOT NULL pk"? really?).
>
>
> It is about semantic and conformity of proposed tools. Sure, all can
> reduced to ASSERT(expr) .. but where is some benefit against function call
>
> I am able to do without any change of language as plpgsql extension -
> there is no necessary to do any change for too thin proposal
>
>
>
> .marko
>
>

--
Jan Wieck
Senior Software Engineer
http://slony.info

2014-09-05 14:35 GMT+02:00 Jan Wieck <jan(at)wi3ck(dot)info>:

> On 09/05/2014 04:40 AM, Pavel Stehule wrote:
>
>>
>>
>>
>> 2014-09-05 10:33 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to
>> <mailto:marko(at)joh(dot)to>>:
>>
>> On 9/5/14 10:09 AM, Pavel Stehule wrote:
>>
>> I think this could still be parsed correctly, though I'm not
>> 100% sure on
>> that:
>>
>> ASSERT WARNING (EXISTS(SELECT ..)), 'data are there';
>>
>>
>> PLpgSQL uses a ';' or some plpgsql keyword as SQL statement
>> delimiter. It
>> reason why RETURN QUERY ... ';' So in this case can practical to
>> place SQL
>> statement on the end of plpgsql statement.
>>
>>
>> *shrug* There are lots of cases where a comma is used as well, e.g.
>> RAISE NOTICE '..', <expr>, <expr>;
>>
>> parenthesis are not practical, because it is hard to identify bug
>> ..
>>
>>
>> I don't see why. The PL/PgSQL SQL parser goes to great lengths to
>> identify unmatched parenthesis. But the parens probably aren't
>> necessary in the first place; you could just omit them and keep
>> parsing until the next comma AFAICT. So the syntax would be:
>>
>> RAISE [ NOTICE | WARNING | EXCEPTION/ASSERT/WHATEVER ]
>> boolean_expr [, error_message [, error_message_param [, ... ] ] ];
>>
>>
>> extension RAISE about ASSERT level has minimal new value
>>
>
> Adding a WHEN clause to RAISE would have the benefit of not needing any
> new keywords at all.
>
> RAISE EXCEPTION 'format' [, expr ...] WHEN row_count <> 1;
>

It was one my older proposal.

Can we find a agreement there?

Pavel

>
>
> Regards,
> Jan
>
>
>
>>
>> A simplicity of integration SQL and PLpgSQL is in using "smart"
>> keywords -
>> It is more verbose, and it allow to well diagnostics
>>
>>
>> I disagree. The new keywords provide nothing of value here. They
>> even encourage the use of quirky syntax in *exchange* for verbosity
>> ("IS NOT NULL pk"? really?).
>>
>>
>> It is about semantic and conformity of proposed tools. Sure, all can
>> reduced to ASSERT(expr) .. but where is some benefit against function call
>>
>> I am able to do without any change of language as plpgsql extension -
>> there is no necessary to do any change for too thin proposal
>>
>>
>>
>> .marko
>>
>>
>>
>
> --
> Jan Wieck
> Senior Software Engineer
> http://slony.info
>

On 05/09/14 14:35, Jan Wieck wrote:
> On 09/05/2014 04:40 AM, Pavel Stehule wrote:
>
> Adding a WHEN clause to RAISE would have the benefit of not needing any
> new keywords at all.
>
> RAISE EXCEPTION 'format' [, expr ...] WHEN row_count <> 1;
>

+1

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-09-06 7:27 AM, Pavel Stehule wrote:
> 2014-09-05 14:35 GMT+02:00 Jan Wieck <jan(at)wi3ck(dot)info>:
>> Adding a WHEN clause to RAISE would have the benefit of not needing any
>> new keywords at all.
>>
>> RAISE EXCEPTION 'format' [, expr ...] WHEN row_count <> 1;
>>
>
> It was one my older proposal.
>
> Can we find a agreement there?

I find:

1) The syntax less readable than IF row_count <> 1 THEN RAISE
EXCEPTION ..; END IF;
2) It needless to require the user to specify an error message for
every assertion.
3) Allowing these to be disabled would be weird (though I might be
the only one who wants that feature at this point).
4) It would also be weird to display the parameters passed to the
WHEN clause like I suggested here:
http://www.postgresql.org/message-id/54096BA4.5030600@joh.to . I think
that's a crucial part of the feature.

So at least the vote isn't unanimous: -1 from me.

.marko

2014-09-06 19:26 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 2014-09-06 7:27 AM, Pavel Stehule wrote:
>
>> 2014-09-05 14:35 GMT+02:00 Jan Wieck <jan(at)wi3ck(dot)info>:
>>
>>> Adding a WHEN clause to RAISE would have the benefit of not needing any
>>> new keywords at all.
>>>
>>> RAISE EXCEPTION 'format' [, expr ...] WHEN row_count <> 1;
>>>
>>>
>> It was one my older proposal.
>>
>> Can we find a agreement there?
>>
>
> I find:
>
> 1) The syntax less readable than IF row_count <> 1 THEN RAISE EXCEPTION
> ..; END IF;
> 2) It needless to require the user to specify an error message for every
> assertion.
> 3) Allowing these to be disabled would be weird (though I might be the
> only one who wants that feature at this point).
> 4) It would also be weird to display the parameters passed to the WHEN
> clause like I suggested here: http://www.postgresql.org/
> message-id/54096BA4(dot)5030600(at)joh(dot)to . I think that's a crucial part of
> the feature.
>
> So at least the vote isn't unanimous: -1 from me.
>

this doesn't to supply assertions, it is just shorter form

Pavel

>
>
> .marko
>

On 2014-09-06 7:49 PM, Pavel Stehule wrote:
> this doesn't to supply assertions, it is just shorter form

The original proposal very clearly seems to be "why don't we do this
*instead* of assertions?" And in that case all of my points apply, and
I'm very much against this syntax. If this is supposed to be in the
language *in addition to* assertions, let's please be clear about that.
(In that case I wouldn't object, though I probably wouldn't use this
feature either.)

.marko

2014-09-06 19:59 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 2014-09-06 7:49 PM, Pavel Stehule wrote:
>
>> this doesn't to supply assertions, it is just shorter form
>>
>
> The original proposal very clearly seems to be "why don't we do this
> *instead* of assertions?" And in that case all of my points apply, and I'm
> very much against this syntax. If this is supposed to be in the language
> *in addition to* assertions, let's please be clear about that. (In that
> case I wouldn't object, though I probably wouldn't use this feature either.)

It was just my reaction to Jan proposal in this thread.

pavel

>
>
> .marko
>

On Fri, Sep 5, 2014 at 2:16 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Assert is usually implemented as custom functions and used via PERFORM
> statement now
>
> -- usual current solution
> PERFORM Assert(some expression)
>
> I would to implement Assert as plpgsql internal statement due bigger
> possibilities to design syntax and internal implementation now and in
> future. More - as plpgsql statement should be compatible with any current
> code - because there should not be collision between SQL and PLpgSQL space.
> So this design doesn't break any current code.
>
> I propose following syntax with following ecosystem:
>
> ASSERT [ NOTICE, WARNING, >>EXCEPTION<< ]
> [ string expression or literal - explicit message ]
> [ USING clause - same as RAISE stmt (possible in future ) ]
> ( ROW_COUNT ( = | <> ) ( 1 | 0 ) |
> ( QUERY some query should not be empty ) |
> ( CHECK some expression should be true )
> ( IS NOT NULL expression should not be null )
>
> Every variant (ROW_COUNT, QUERY, CHECK, IS NOT NULL) has own default message

That's probably not the ugliest syntax I've *ever* seen, but it's
definitely the ugliest syntax I've seen today.

I previously proposed RAISE ASSERT ... WHERE, which seems like a nice
variant of what we've already got, but perhaps this whole discussion
merely illustrates that it's hard to get more than 1 vote for any
proposal in this area.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 09/05/2014 05:21 PM, Pavel Stehule wrote:
>
> *shrug* Doing it in SQL would probably break more stuff. I'm trying to
> contain the damage. And arguably, this is mostly only useful in PL/PgSQL.

I've wanted assertions in SQL enough that I often write trivial wrappers
around `raise` in PL/PgSQL for use in `CASE` statements etc.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 09/09/2014 05:20 AM, Robert Haas wrote:
>
> I previously proposed RAISE ASSERT ... WHERE, which seems like a nice
> variant of what we've already got, but perhaps this whole discussion
> merely illustrates that it's hard to get more than 1 vote for any
> proposal in this area.

Well, you have Petr's for RAISE EXCEPTION ... WHEN, and I'd also like
that or RAISE ASSERT ... WHEN.

Much (much) saner than the other proposals on this thread IMO.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-09-09 07:54, Craig Ringer wrote:
> On 09/05/2014 05:21 PM, Pavel Stehule wrote:
>>
>> *shrug* Doing it in SQL would probably break more stuff. I'm trying to
>> contain the damage. And arguably, this is mostly only useful in PL/PgSQL.
>
> I've wanted assertions in SQL enough that I often write trivial wrappers
> around `raise` in PL/PgSQL for use in `CASE` statements etc.

Yeah, as have I. I've also advocated that there should be a
raise_exception(text, anyelement) anyelement function shipped with
postgres.

But this is something different; this is just a single statement which
asserts that some expression evaluates to true. Even if we allowed it
to be used as a scalar expression, there's still the problem anyelement
is commonly used to work around.

.marko

2014-09-09 7:55 GMT+02:00 Craig Ringer <craig(at)2ndquadrant(dot)com>:

> On 09/09/2014 05:20 AM, Robert Haas wrote:
> >
> > I previously proposed RAISE ASSERT ... WHERE, which seems like a nice
> > variant of what we've already got, but perhaps this whole discussion
> > merely illustrates that it's hard to get more than 1 vote for any
> > proposal in this area.
>
> Well, you have Petr's for RAISE EXCEPTION ... WHEN, and I'd also like
> that or RAISE ASSERT ... WHEN.
>

Ada is language with strong character, and PLpgSQL is little bit strange
fork - so it isn't easy to find some form, how to solve all requirements.

My requests:

* be consistent with current PLpgSQL syntax and logic
* allow some future extensibility
* allow a static analyses without hard expression processing

But I am thinking so there are some points where can be some agreement -
although it is not ASSERT implementation.

enhancing RAISE WHEN - please, see attached patch -

I prefer RAISE WHEN again RAISE WHERE due consistency with EXIT and
CONTINUE [ WHEN ];

Next we can reserve some SQLCODE for assertation and we can implement it as
not handled exception. It is only "cancel" now, and it is not usable .
Probably it should be implement on SQL level - not on plpgsql only.

Regards

Pavel

>
> Much (much) saner than the other proposals on this thread IMO.
>
> --
> Craig Ringer http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

2014-09-09 7:54 GMT+02:00 Craig Ringer <craig(at)2ndquadrant(dot)com>:

> On 09/05/2014 05:21 PM, Pavel Stehule wrote:
> >
> > *shrug* Doing it in SQL would probably break more stuff. I'm trying to
> > contain the damage. And arguably, this is mostly only useful in
> PL/PgSQL.
>
> I've wanted assertions in SQL enough that I often write trivial wrappers
> around `raise` in PL/PgSQL for use in `CASE` statements etc.
>

In this moment we have no agreement on syntax, but there was not defined a
requirements for aggregations. I looked on assertions in some languages and
implementation and design of assertions is really varied.

I though about it, and "Assertions" is not plpgsql only issue. It must be
supported by core, and by other PL.

There are two usual requests for Assertions:

a) Isn't possible handle a assertion exception anywhere .. it enforce
ROLLBACK in 100%

b) Assertions should be disabled globally .. I am not sure, it it is a good
idea, but I can understand so some tests based on queries to data can be
performance issue.

Important question is a relation assertations and exceptions. Is it only
shortcut for exception or some different?

Comments?

Regards

Pavel

>
> --
> Craig Ringer http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

On 09/14/2014 02:25 PM, Pavel Stehule wrote:
> a) Isn't possible handle a assertion exception anywhere .. it enforce
> ROLLBACK in 100%
>
> b) Assertions should be disabled globally .. I am not sure, it it is a
> good idea, but I can understand so some tests based on queries to data
> can be performance issue.
>
> Important question is a relation assertations and exceptions. Is it only
> shortcut for exception or some different?

I think that most data integrity issues can be handled by a well
designed database schema that uses UNIQUE, NOT NULL, REFERENCES and
CHECK constraints. Assertions are usually found inside of complex code
constructs to check values of local variables. I don't think it is even
a good idea to implement assertions that can query arbitrary data.

Jan

--
Jan Wieck
Senior Software Engineer
http://slony.info

Jan Wieck wrote:
> On 09/14/2014 02:25 PM, Pavel Stehule wrote:
> >a) Isn't possible handle a assertion exception anywhere .. it enforce
> >ROLLBACK in 100%
> >
> >b) Assertions should be disabled globally .. I am not sure, it it is a
> >good idea, but I can understand so some tests based on queries to data
> >can be performance issue.
> >
> >Important question is a relation assertations and exceptions. Is it only
> >shortcut for exception or some different?
>
> I think that most data integrity issues can be handled by a well
> designed database schema that uses UNIQUE, NOT NULL, REFERENCES and
> CHECK constraints. Assertions are usually found inside of complex
> code constructs to check values of local variables. I don't think it
> is even a good idea to implement assertions that can query arbitrary
> data.

Actually Peter Eisentraut posted a patch for SQL assertions:
http://www.postgresql.org/message-id/1384486216.5008.17.camel@vanquo.pezone.net

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 9/16/14 12:01 AM, Alvaro Herrera wrote:
> Jan Wieck wrote:
>> I think that most data integrity issues can be handled by a well
>> designed database schema that uses UNIQUE, NOT NULL, REFERENCES and
>> CHECK constraints. Assertions are usually found inside of complex
>> code constructs to check values of local variables. I don't think it
>> is even a good idea to implement assertions that can query arbitrary
>> data.
>
> Actually Peter Eisentraut posted a patch for SQL assertions:
> http://www.postgresql.org/message-id/1384486216.5008.17.camel@vanquo.pezone.net

SQL assertions are just a kind of CHECK constraint, so fully
Jan-compliant. ;-)

I don't mind PL/pgSQL having an "assert" statement like many programming
languages, but I find a lot of the proposed details dubious.

On 9/14/14 2:49 PM, Jan Wieck wrote:
> I don't think it is even a good idea to implement assertions that can
> query arbitrary data.

In a normal programming language, an assertion is usually a static fault
in your program. If the assertion ever fails, you fix your program and
then it hopefully never happens again.

Assertion that query the state of the database or result row counts are
pushing that concept quite a bit. Those are not assertions, those are
just plain old error handling.

On 9/17/14, 9:00 PM, Peter Eisentraut wrote:
> On 9/14/14 2:49 PM, Jan Wieck wrote:
>> I don't think it is even a good idea to implement assertions that can
>> query arbitrary data.
>
> In a normal programming language, an assertion is usually a static fault
> in your program. If the assertion ever fails, you fix your program and
> then it hopefully never happens again.
>
> Assertion that query the state of the database or result row counts are
> pushing that concept quite a bit. Those are not assertions, those are
> just plain old error handling.

*shrug* I don't see them as error handling if they're just checking
conditions which should never happen.

That said, in PL/PgSQL these expressions would likely have to be SQL
expressions, and then you'd have to go out of your way to implement
assertions which *can't* query arbitrary data. And that just seems silly.

.marko

2014-09-17 21:00 GMT+02:00 Peter Eisentraut <peter_e(at)gmx(dot)net>:

> On 9/14/14 2:49 PM, Jan Wieck wrote:
> > I don't think it is even a good idea to implement assertions that can
> > query arbitrary data.
>
> In a normal programming language, an assertion is usually a static fault
> in your program. If the assertion ever fails, you fix your program and
> then it hopefully never happens again.
>
> Assertion that query the state of the database or result row counts are
> pushing that concept quite a bit. Those are not assertions, those are
> just plain old error handling.
>
>
What is difference between content of variable or content of database? You
can test any prerequisite, but when this prerequisite is not solved, than
exception is very very hard without possible handling.

Pavel

On 9/17/14 3:04 PM, Pavel Stehule wrote:
> What is difference between content of variable or content of database?
> You can test any prerequisite, but when this prerequisite is not solved,
> than exception is very very hard without possible handling.

If the assertion tests arbitrary Boolean expressions, then we can't stop
the user from abusing them.

But it's another thing if we design specific syntax that encourages such
abuse, as proposed earlier.

2014-09-17 21:36 GMT+02:00 Peter Eisentraut <peter_e(at)gmx(dot)net>:

> On 9/17/14 3:04 PM, Pavel Stehule wrote:
> > What is difference between content of variable or content of database?
> > You can test any prerequisite, but when this prerequisite is not solved,
> > than exception is very very hard without possible handling.
>
> If the assertion tests arbitrary Boolean expressions, then we can't stop
> the user from abusing them.
>
>
I am thinking so unhandled signal can be good defence. (and possibility to
disable assertions)

We design a database system, so we should to reflect it - plpgsql (or any
PL environment) are not classic language. There are lot of database
specific constructs.

> But it's another thing if we design specific syntax that encourages such
> abuse, as proposed earlier.
>
>
Other note - I am thinking so ANSI SQL Assertions and PL assertions are
independent features. Although they can have some common goals.

On 09/17/2014 03:36 PM, Peter Eisentraut wrote:
> On 9/17/14 3:04 PM, Pavel Stehule wrote:
>> What is difference between content of variable or content of database?
>> You can test any prerequisite, but when this prerequisite is not solved,
>> than exception is very very hard without possible handling.
>
> If the assertion tests arbitrary Boolean expressions, then we can't stop
> the user from abusing them.

Exactly. Doing something like

ASSERT (select count(*) from foo
where fk not in (select pk from bar)) = 0;

is a perfectly fine, arbitrary boolean expression. It will probably work
well in a development environment too. And I am very sure that it will
not scale well once that code gets deployed. And I know how DBAs react
to the guaranteed following performance problem. They will disable ALL
assert ... or was there some sort of assert class system proposed that I
missed?

>
> But it's another thing if we design specific syntax that encourages such
> abuse, as proposed earlier.

The design should explicitly discourage that sort of nonsense.

Jan

--
Jan Wieck
Senior Software Engineer
http://slony.info

On 9/17/14, 7:40 PM, Jan Wieck wrote:
> Exactly. Doing something like
>
> ASSERT (select count(*) from foo
> where fk not in (select pk from bar)) = 0;
>
> is a perfectly fine, arbitrary boolean expression. It will probably work well in a development environment too. And I am very sure that it will not scale well once that code gets deployed. And I know how DBAs react to the guaranteed following performance problem. They will disable ALL assert ... or was there some sort of assert class system proposed that I missed?

Keep in mind that nothing we come up with will be immune to abuse, and trying to solve what is fundamentally an education problem through technology rarely turns out well.

We're also putting too much weight on the term "assert" here. C-style asserts are generally not nearly as useful in a database as general sanity-checking or error handling, especially if you're trying to use the database to enforce data sanity.

My wish-list for "asserts" is:

- Works at a SQL level
- Unique/clear way to identify asserts (so you're not guessing where the assert came from)
- Allows for over-riding individual asserts (so if you need to do something you're "not supposed to do" you still have the protection of all other asserts)
- Less verbose than IF THEN RAISE END IF
--
Jim C. Nasby, Data Architect jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

On 09/09/14 17:37, Pavel Stehule wrote:
> Ada is language with strong character, and PLpgSQL is little bit strange
> fork - so it isn't easy to find some form, how to solve all requirements.
>
> My requests:
>
> * be consistent with current PLpgSQL syntax and logic
> * allow some future extensibility
> * allow a static analyses without hard expression processing
>
> But I am thinking so there are some points where can be some agreement -
> although it is not ASSERT implementation.
>
> enhancing RAISE WHEN - please, see attached patch -
>
> I prefer RAISE WHEN again RAISE WHERE due consistency with EXIT and
> CONTINUE [ WHEN ];
>

Short review of the patch. Note that this has nothing to do with actual
assertions, at the moment it's just enhancement of RAISE statement to
make it optionally conditional. As I was one of the people who voted for
it I do think we want this and I like the syntax.

Code applies cleanly, seems formatted according to project standards -
there is unnecessary whitespace added in variable declaration part of
exec_stmt_raise which should be removed.

Passes make check, I would prefer to have little more complex expression
than just "true" in the new regression test added for this feature.

Did some manual testing, seems to work as advertised.

There are no docs at the moment which is the only show-stopper that I
can see so far.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2014-09-30 10:04 GMT+07:00 Jim Nasby <jim(at)nasby(dot)net>:

> On 9/17/14, 7:40 PM, Jan Wieck wrote:
>
>> Exactly. Doing something like
>>
>> ASSERT (select count(*) from foo
>> where fk not in (select pk from bar)) = 0;
>>
>> is a perfectly fine, arbitrary boolean expression. It will probably work
>> well in a development environment too. And I am very sure that it will not
>> scale well once that code gets deployed. And I know how DBAs react to the
>> guaranteed following performance problem. They will disable ALL assert ...
>> or was there some sort of assert class system proposed that I missed?
>>
>
Actually, compared with for example Java or C, in production systems,
usually all asserts are disabled for performance (in java removed by JIT,
in C we define NDEBUG).

> We're also putting too much weight on the term "assert" here. C-style
> asserts are generally not nearly as useful in a database as general
> sanity-checking or error handling, especially if you're trying to use the
> database to enforce data sanity.
>

+1.
without any query capability, assert will become much less useful. If we
cannot query in assert, we will code:

-- perform some query
ASSERT WHEN some_check_on_query_result;

.. and disabling the query in production system will become another trouble.

My wish-list for "asserts" is:
>
> - Works at a SQL level
> - Unique/clear way to identify asserts (so you're not guessing where the
> assert came from)
>
+1

> - Allows for over-riding individual asserts (so if you need to do
> something you're "not supposed to do" you still have the protection of all
> other asserts)
> - Less verbose than IF THEN RAISE END IF
>
+1

--
Ali Akbar

2014-10-15 11:57 GMT+02:00 Ali Akbar <the(dot)apaan(at)gmail(dot)com>:

> 2014-09-30 10:04 GMT+07:00 Jim Nasby <jim(at)nasby(dot)net>:
>
>> On 9/17/14, 7:40 PM, Jan Wieck wrote:
>>
>>> Exactly. Doing something like
>>>
>>> ASSERT (select count(*) from foo
>>> where fk not in (select pk from bar)) = 0;
>>>
>>> is a perfectly fine, arbitrary boolean expression. It will probably work
>>> well in a development environment too. And I am very sure that it will not
>>> scale well once that code gets deployed. And I know how DBAs react to the
>>> guaranteed following performance problem. They will disable ALL assert ...
>>> or was there some sort of assert class system proposed that I missed?
>>>
>>
> Actually, compared with for example Java or C, in production systems,
> usually all asserts are disabled for performance (in java removed by JIT,
> in C we define NDEBUG).
>

This argument should not be too valid for plpgsql - possible bottlenecks
are in SQL execution (or should be)

>
>
>> We're also putting too much weight on the term "assert" here. C-style
>> asserts are generally not nearly as useful in a database as general
>> sanity-checking or error handling, especially if you're trying to use the
>> database to enforce data sanity.
>>
>
> +1.
> without any query capability, assert will become much less useful. If we
> cannot query in assert, we will code:
>
> -- perform some query
> ASSERT WHEN some_check_on_query_result;
>
> .. and disabling the query in production system will become another
> trouble.
>
> My wish-list for "asserts" is:
>>
>> - Works at a SQL level
>> - Unique/clear way to identify asserts (so you're not guessing where the
>> assert came from)
>>
> +1
>
>
>> - Allows for over-riding individual asserts (so if you need to do
>> something you're "not supposed to do" you still have the protection of all
>> other asserts)
>> - Less verbose than IF THEN RAISE END IF
>>
> +1
>
> --
> Ali Akbar
>

Hi

2014-10-14 22:57 GMT+02:00 Petr Jelinek <petr(at)2ndquadrant(dot)com>:

> On 09/09/14 17:37, Pavel Stehule wrote:
>
>> Ada is language with strong character, and PLpgSQL is little bit strange
>> fork - so it isn't easy to find some form, how to solve all requirements.
>>
>> My requests:
>>
>> * be consistent with current PLpgSQL syntax and logic
>> * allow some future extensibility
>> * allow a static analyses without hard expression processing
>>
>> But I am thinking so there are some points where can be some agreement -
>> although it is not ASSERT implementation.
>>
>> enhancing RAISE WHEN - please, see attached patch -
>>
>> I prefer RAISE WHEN again RAISE WHERE due consistency with EXIT and
>> CONTINUE [ WHEN ];
>>
>>
> Short review of the patch. Note that this has nothing to do with actual
> assertions, at the moment it's just enhancement of RAISE statement to make
> it optionally conditional. As I was one of the people who voted for it I do
> think we want this and I like the syntax.
>
> Code applies cleanly, seems formatted according to project standards -
> there is unnecessary whitespace added in variable declaration part of
> exec_stmt_raise which should be removed.
>

fixed

>
> Passes make check, I would prefer to have little more complex expression
> than just "true" in the new regression test added for this feature.
>

fixed

>
> Did some manual testing, seems to work as advertised.
>
> There are no docs at the moment which is the only show-stopper that I can
> see so far.
>

fixed

Regards

Pavel

>
> --
> Petr Jelinek http://www.2ndQuadrant.com/
>
> PostgreSQL Development, 24x7 Support, Training & Services
>

On 16/10/14 13:29, Pavel Stehule wrote:
> Hi
>
> 2014-10-14 22:57 GMT+02:00 Petr Jelinek <petr(at)2ndquadrant(dot)com
>
> Short review of the patch. Note that this has nothing to do with
> actual assertions, at the moment it's just enhancement of RAISE
> statement to make it optionally conditional. As I was one of the
> people who voted for it I do think we want this and I like the syntax.
>
> Code applies cleanly, seems formatted according to project standards
> - there is unnecessary whitespace added in variable declaration part
> of exec_stmt_raise which should be removed.
>
>
> fixed
>
>
> Passes make check, I would prefer to have little more complex
> expression than just "true" in the new regression test added for
> this feature.
>
>
> fixed
>
>
> Did some manual testing, seems to work as advertised.
>
> There are no docs at the moment which is the only show-stopper that
> I can see so far.
>
>
> fixed
>

Great, looks good to me, marking as ready for committer.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2014-10-17 9:14 GMT+02:00 Petr Jelinek <petr(at)2ndquadrant(dot)com>:

> On 16/10/14 13:29, Pavel Stehule wrote:
>
>> Hi
>>
>> 2014-10-14 22:57 GMT+02:00 Petr Jelinek <petr(at)2ndquadrant(dot)com
>>
>> Short review of the patch. Note that this has nothing to do with
>> actual assertions, at the moment it's just enhancement of RAISE
>> statement to make it optionally conditional. As I was one of the
>> people who voted for it I do think we want this and I like the syntax.
>>
>> Code applies cleanly, seems formatted according to project standards
>> - there is unnecessary whitespace added in variable declaration part
>> of exec_stmt_raise which should be removed.
>>
>>
>> fixed
>>
>>
>> Passes make check, I would prefer to have little more complex
>> expression than just "true" in the new regression test added for
>> this feature.
>>
>>
>> fixed
>>
>>
>> Did some manual testing, seems to work as advertised.
>>
>> There are no docs at the moment which is the only show-stopper that
>> I can see so far.
>>
>>
>> fixed
>>
>>
> Great, looks good to me, marking as ready for committer.

Thank you very much

Pavel

>
>
> --
> Petr Jelinek http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

>> Great, looks good to me, marking as ready for committer.

What is wrong with using IF ?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 11/17/14, 4:58 PM, Simon Riggs wrote:
>>> Great, looks good to me, marking as ready for committer.
>
> What is wrong with using IF ?

It's a hell of a lot wordier. I've previously created a more sophisticated "assert" framework to allow more control over things, but ended up also using it just for simple sanity checking because it was much nicer than typeing IF THEN RAISE ERROR END IF.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

2014-11-17 23:58 GMT+01:00 Simon Riggs <simon(at)2ndquadrant(dot)com>:

> >> Great, looks good to me, marking as ready for committer.
>
> What is wrong with using IF ?
>

It significantly increase code' length .. and decrease readability when you
intensive use a pattern IF THEN RAISE END IF - when you check every
parameter, when you check every result.

RAISE ... WHEN ... is shorter with full power of RAISE statement and
possibility for future enhancing.

Regards

Pavel

>
> --
> Simon Riggs http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

On 18 November 2014 01:00, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> wrote:
> On 11/17/14, 4:58 PM, Simon Riggs wrote:
>>>>
>>>> Great, looks good to me, marking as ready for committer.
>>
>>
>> What is wrong with using IF ?
>
>
> It's a hell of a lot wordier. I've previously created a more sophisticated
> "assert" framework to allow more control over things, but ended up also
> using it just for simple sanity checking because it was much nicer than
> typeing IF THEN RAISE ERROR END IF.

Why is that not a requirement for a less wordier form of IF?

IF (something) THEN action

Why is this problem specific to RAISE?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2014-11-18 10:23 GMT+01:00 Simon Riggs <simon(at)2ndquadrant(dot)com>:

> On 18 November 2014 01:00, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> wrote:
> > On 11/17/14, 4:58 PM, Simon Riggs wrote:
> >>>>
> >>>> Great, looks good to me, marking as ready for committer.
> >>
> >>
> >> What is wrong with using IF ?
> >
> >
> > It's a hell of a lot wordier. I've previously created a more
> sophisticated
> > "assert" framework to allow more control over things, but ended up also
> > using it just for simple sanity checking because it was much nicer than
> > typeing IF THEN RAISE ERROR END IF.
>
> Why is that not a requirement for a less wordier form of IF?
>
> IF (something) THEN action
>

statement IF is a control statement - and syntax, pattern for control
statements in plpgsql is consistent. I don't want to break it (more,
probably it is hardly implemented due problems in bison). PL/pgSQL, PL/SQL,
Ada are well designed (in my opinion). Conditional statement has precedent
in PL/pgSQL now. We support EXIT and CONTINUE WHEN, so we don't propose a
new pattern, only reuse some existing.

Regards

Pavel

>
>
> Why is this problem specific to RAISE?
>
>
> --
> Simon Riggs http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

On 11/18/2014 04:23 AM, Simon Riggs wrote:
> On 18 November 2014 01:00, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> wrote:
>> On 11/17/14, 4:58 PM, Simon Riggs wrote:
>>>>> Great, looks good to me, marking as ready for committer.
>>>
>>> What is wrong with using IF ?
>>
>> It's a hell of a lot wordier. I've previously created a more sophisticated
>> "assert" framework to allow more control over things, but ended up also
>> using it just for simple sanity checking because it was much nicer than
>> typeing IF THEN RAISE ERROR END IF.
> Why is that not a requirement for a less wordier form of IF?
>
> IF (something) THEN action
>
> Why is this problem specific to RAISE?
>
>

Please, no. The use of closed form rather than open form IF statements
is one of the things Ada (and by inheritance PLPGSQL) got right.

Frankly, I find this whole proposal, and all the suggested alternatives,
somewhat ill-conceived. PLPGSQL is a wordy language. If you want
something more terse, use something else. Adding these sorts of
syntactic sugar warts onto the language doesn't seem like a terribly
good way to proceed.

cheers

andrew

On 18 November 2014 12:29, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

>> Why is that not a requirement for a less wordier form of IF?
>>
>> IF (something) THEN action
>
>
> statement IF is a control statement - and syntax, pattern for control
> statements in plpgsql is consistent. I don't want to break it (more,
> probably it is hardly implemented due problems in bison). PL/pgSQL, PL/SQL,
> Ada are well designed (in my opinion). Conditional statement has precedent
> in PL/pgSQL now. We support EXIT and CONTINUE WHEN, so we don't propose a
> new pattern, only reuse some existing.

I commend your wish to improve PL/pgSQL, I'm sorry to say that I just
don't see how this moves us forwards.

What this does is introduce a fairly restricted new feature that
removes backwards compatibility and takes us further away from Oracle
compatibility.

If I want to write an Assert style test that fits on a single line, just write
PEFORM raise_error_when(boolean expression);

which requires a very short function like this
CREATE OR REPLACE FUNCTION raise_error_when(test boolean) returns void
language plpgsql
AS $$
DECLARE
BEGIN
IF (test) THEN
RAISE EXCEPTION 'assertion failure';
END IF;
END;
$$;

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2014-11-18 17:08 GMT+01:00 Simon Riggs <simon(at)2ndquadrant(dot)com>:

> On 18 November 2014 12:29, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
> >> Why is that not a requirement for a less wordier form of IF?
> >>
> >> IF (something) THEN action
> >
> >
> > statement IF is a control statement - and syntax, pattern for control
> > statements in plpgsql is consistent. I don't want to break it (more,
> > probably it is hardly implemented due problems in bison). PL/pgSQL,
> PL/SQL,
> > Ada are well designed (in my opinion). Conditional statement has
> precedent
> > in PL/pgSQL now. We support EXIT and CONTINUE WHEN, so we don't propose a
> > new pattern, only reuse some existing.
>
> I commend your wish to improve PL/pgSQL, I'm sorry to say that I just
> don't see how this moves us forwards.
>
>
It is not big step, but it open some doors

> What this does is introduce a fairly restricted new feature that
> removes backwards compatibility and takes us further away from Oracle
> compatibility.
>

It is not valid argument for this use case. RAISE statement is not
compatible with Oracle long time. WHEN clause change nothing.

>
> If I want to write an Assert style test that fits on a single line, just
> write
> PEFORM raise_error_when(boolean expression);
>

it is possibility too. But a) it is limited little bit, b) we didn't find a
agreement how to design it for upstream. c) I am thinking so there is a
space for enhancing RAISE statement for other use cases - tracing, global
condition assertions etc

>
> which requires a very short function like this
> CREATE OR REPLACE FUNCTION raise_error_when(test boolean) returns void
> language plpgsql
> AS $$
> DECLARE
> BEGIN
> IF (test) THEN
> RAISE EXCEPTION 'assertion failure';
> END IF;
> END;
> $$;
>
> --
> Simon Riggs http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

On 11/18/14, 9:31 AM, Andrew Dunstan wrote:
>
> Frankly, I find this whole proposal, and all the suggested alternatives, somewhat ill-conceived. PLPGSQL is a wordy language. If you want something more terse, use something else. Adding these sorts of syntactic sugar warts onto the language doesn't seem like a terribly good way to proceed.

Such as?

The enormous advantage of plpgsql is how easy it is to run SQL. Every other PL I've looked at makes that WAY harder. And that's assuming you're in an environment where you can install another PL.

And honestly, I've never really found plpgsql to be terribly wordy except in a few cases ("assert" being one of them). My general experience has been that when I'm doing an IF (other than assert), I'm doing multiple things in the IF block, so it's really not that big a deal.

As for why not do this in a separate function; yes, you can do that. But then you've needlessly added to your context stack, it's going to be a lot slower, and you can only really replace RAISE's functionality if you're in a version that has format().

If someone has another brain-flash on how to make this better I'm all ears. But I don't think arguments like "use another PL" or "it's just syntax sugar" improve things for our users.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

On 11/18/2014 02:53 PM, Jim Nasby wrote:
> On 11/18/14, 9:31 AM, Andrew Dunstan wrote:
>>
>> Frankly, I find this whole proposal, and all the suggested
>> alternatives, somewhat ill-conceived. PLPGSQL is a wordy language. If
>> you want something more terse, use something else. Adding these sorts
>> of syntactic sugar warts onto the language doesn't seem like a
>> terribly good way to proceed.
>
> Such as?
>
> The enormous advantage of plpgsql is how easy it is to run SQL. Every
> other PL I've looked at makes that WAY harder. And that's assuming
> you're in an environment where you can install another PL.
>
> And honestly, I've never really found plpgsql to be terribly wordy
> except in a few cases ("assert" being one of them). My general
> experience has been that when I'm doing an IF (other than assert), I'm
> doing multiple things in the IF block, so it's really not that big a
> deal.
>

I frequently write one-statement bodies of IF statements. To me that's
not a big deal either :-)

cheers

andrew

2014-11-18 21:27 GMT+01:00 Andrew Dunstan <andrew(at)dunslane(dot)net>:

>
> On 11/18/2014 02:53 PM, Jim Nasby wrote:
>
>> On 11/18/14, 9:31 AM, Andrew Dunstan wrote:
>>
>>>
>>> Frankly, I find this whole proposal, and all the suggested alternatives,
>>> somewhat ill-conceived. PLPGSQL is a wordy language. If you want something
>>> more terse, use something else. Adding these sorts of syntactic sugar warts
>>> onto the language doesn't seem like a terribly good way to proceed.
>>>
>>
>> Such as?
>>
>> The enormous advantage of plpgsql is how easy it is to run SQL. Every
>> other PL I've looked at makes that WAY harder. And that's assuming you're
>> in an environment where you can install another PL.
>>
>> And honestly, I've never really found plpgsql to be terribly wordy except
>> in a few cases ("assert" being one of them). My general experience has been
>> that when I'm doing an IF (other than assert), I'm doing multiple things in
>> the IF block, so it's really not that big a deal.
>>
>>
>
> I frequently write one-statement bodies of IF statements. To me that's not
> a big deal either :-)
>

anybody did it, but it doesn't need so it is perfect :) I understand well
to Jim' feeling.

I am looking to Ada 2005 language ... a design of RAISE WITH shows so RAISE
statement is extensible in Ada too. Sure - we can live without it, but I
don't think so we do some wrong with introduction RAISE WHEN and I am sure,
so a live with this feature can be more fun for someone, who intensive use
this pattern.

>
> cheers
>
> andrew
>

On 18/11/14 22:11, Pavel Stehule wrote:
>
> 2014-11-18 21:27 GMT+01:00 Andrew Dunstan <andrew(at)dunslane(dot)net
> <mailto:andrew(at)dunslane(dot)net>>:
>
>
> I frequently write one-statement bodies of IF statements. To me
> that's not a big deal either :-)
>
>
> anybody did it, but it doesn't need so it is perfect :) I understand
> well to Jim' feeling.
>
> I am looking to Ada 2005 language ... a design of RAISE WITH shows so
> RAISE statement is extensible in Ada too. Sure - we can live without it,
> but I don't think so we do some wrong with introduction RAISE WHEN and I
> am sure, so a live with this feature can be more fun for someone, who
> intensive use this pattern.
>

Personally, I see this as natural extension of the conditional block
control which we already have for loops with CONTINUE WHEN and EXIT
WHEN. This basically extends it to any block and it seems quite natural
to have it for me...

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 11/18/2014 04:11 PM, Pavel Stehule wrote:
>
>
> 2014-11-18 21:27 GMT+01:00 Andrew Dunstan <andrew(at)dunslane(dot)net
> <mailto:andrew(at)dunslane(dot)net>>:
>
>
> On 11/18/2014 02:53 PM, Jim Nasby wrote:
>
> On 11/18/14, 9:31 AM, Andrew Dunstan wrote:
>
>
> Frankly, I find this whole proposal, and all the suggested
> alternatives, somewhat ill-conceived. PLPGSQL is a wordy
> language. If you want something more terse, use something
> else. Adding these sorts of syntactic sugar warts onto the
> language doesn't seem like a terribly good way to proceed.
>
>
> Such as?
>
> The enormous advantage of plpgsql is how easy it is to run
> SQL. Every other PL I've looked at makes that WAY harder. And
> that's assuming you're in an environment where you can install
> another PL.
>
> And honestly, I've never really found plpgsql to be terribly
> wordy except in a few cases ("assert" being one of them). My
> general experience has been that when I'm doing an IF (other
> than assert), I'm doing multiple things in the IF block, so
> it's really not that big a deal.
>
>
>
> I frequently write one-statement bodies of IF statements. To me
> that's not a big deal either :-)
>
>
> anybody did it, but it doesn't need so it is perfect :) I understand
> well to Jim' feeling.
>
> I am looking to Ada 2005 language ... a design of RAISE WITH shows so
> RAISE statement is extensible in Ada too. Sure - we can live without
> it, but I don't think so we do some wrong with introduction RAISE WHEN
> and I am sure, so a live with this feature can be more fun for
> someone, who intensive use this pattern.
>
>

(drags out recently purchased copy of Barnes "Ada 2012")

Ada's

RAISE exception_name WITH "string";

is more or less the equivalent of our

RAISE level 'format_string';

So I don't think there's much analogy there.

I'm not going to die in a ditch over this, but it does seem to me very
largely unnecessary.

cheers

andrew

2014-11-18 22:28 GMT+01:00 Andrew Dunstan <andrew(at)dunslane(dot)net>:

>
> On 11/18/2014 04:11 PM, Pavel Stehule wrote:
>
>>
>>
>> 2014-11-18 21:27 GMT+01:00 Andrew Dunstan <andrew(at)dunslane(dot)net <mailto:
>> andrew(at)dunslane(dot)net>>:
>>
>>
>>
>> On 11/18/2014 02:53 PM, Jim Nasby wrote:
>>
>> On 11/18/14, 9:31 AM, Andrew Dunstan wrote:
>>
>>
>> Frankly, I find this whole proposal, and all the suggested
>> alternatives, somewhat ill-conceived. PLPGSQL is a wordy
>> language. If you want something more terse, use something
>> else. Adding these sorts of syntactic sugar warts onto the
>> language doesn't seem like a terribly good way to proceed.
>>
>>
>> Such as?
>>
>> The enormous advantage of plpgsql is how easy it is to run
>> SQL. Every other PL I've looked at makes that WAY harder. And
>> that's assuming you're in an environment where you can install
>> another PL.
>>
>> And honestly, I've never really found plpgsql to be terribly
>> wordy except in a few cases ("assert" being one of them). My
>> general experience has been that when I'm doing an IF (other
>> than assert), I'm doing multiple things in the IF block, so
>> it's really not that big a deal.
>>
>>
>>
>> I frequently write one-statement bodies of IF statements. To me
>> that's not a big deal either :-)
>>
>>
>> anybody did it, but it doesn't need so it is perfect :) I understand well
>> to Jim' feeling.
>>
>> I am looking to Ada 2005 language ... a design of RAISE WITH shows so
>> RAISE statement is extensible in Ada too. Sure - we can live without it,
>> but I don't think so we do some wrong with introduction RAISE WHEN and I am
>> sure, so a live with this feature can be more fun for someone, who
>> intensive use this pattern.
>>
>>
>>
>
> (drags out recently purchased copy of Barnes "Ada 2012")
>
> Ada's
>
> RAISE exception_name WITH "string";
>
> is more or less the equivalent of our
>
> RAISE level 'format_string';
>
> So I don't think there's much analogy there.
>
>
I used it as analogy of immutability of this statement in Ada,

>
> I'm not going to die in a ditch over this, but it does seem to me very
> largely unnecessary.
>
> cheers
>
> andrew
>
>

On 18 November 2014 21:19, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:

> Personally, I see this as natural extension of the conditional block control
> which we already have for loops with CONTINUE WHEN and EXIT WHEN. This
> basically extends it to any block and it seems quite natural to have it for
> me...

That's a reasonable argument to include it.

I seem to share the same opinion with Andrew: its not going to hurt to
include this, but its not gonna cause dancing in the streets either. I
would characterize that as 2 very neutral and unimpressed people, plus
3 in favour. Which seems enough to commit.

Perhaps I misunderstand, Andrew?

Any objectors, say so now or I will commit tomorrow.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 11/19/2014 06:35 AM, Simon Riggs wrote:
> On 18 November 2014 21:19, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
>
>> Personally, I see this as natural extension of the conditional block control
>> which we already have for loops with CONTINUE WHEN and EXIT WHEN. This
>> basically extends it to any block and it seems quite natural to have it for
>> me...
> That's a reasonable argument to include it.
>
> I seem to share the same opinion with Andrew: its not going to hurt to
> include this, but its not gonna cause dancing in the streets either. I
> would characterize that as 2 very neutral and unimpressed people, plus
> 3 in favour. Which seems enough to commit.
>
> Perhaps I misunderstand, Andrew?
>

That's about right, although I would put it a bit stronger than that.
But if we're the only people unimpressed I'm not going to object further.

cheers

andrew

>
>
>> On 18 November 2014 21:19, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
>>
>> Personally, I see this as natural extension of the conditional block
>>> control
>>> which we already have for loops with CONTINUE WHEN and EXIT WHEN. This
>>> basically extends it to any block and it seems quite natural to have it
>>> for
>>> me...
>>>
>>
​This seems to me like a step in the direction of APL, where every
statement is a conditional.

Perl has the option of putting the conditional on the end of a statement as
suggested here for ASSERT. My experience has been that while it may "look
prettier" to some, the conditional is overlooked in reviews, etc., more
often than one would expect, giving a net loss in the overall
risk/productivity analysis.

As a code maintainer, I would be opposed to adding something like this for
no other reason than perceived aesthetics.

Your mileage may, of course, vary.

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 11/19/2014 06:35 AM, Simon Riggs wrote:
>> I seem to share the same opinion with Andrew: its not going to hurt to
>> include this, but its not gonna cause dancing in the streets either. I
>> would characterize that as 2 very neutral and unimpressed people, plus
>> 3 in favour. Which seems enough to commit.

> That's about right, although I would put it a bit stronger than that.
> But if we're the only people unimpressed I'm not going to object further.

FWIW, I would vote against it also. I do not find this to be a natural
extension of RAISE; it adds all sorts of semantic issues. (In particular,
what is the evaluation order of the WHEN versus the other subexpressions
of the RAISE?)

regards, tom lane

On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> On 11/19/2014 06:35 AM, Simon Riggs wrote:
>>> I seem to share the same opinion with Andrew: its not going to hurt to
>>> include this, but its not gonna cause dancing in the streets either. I
>>> would characterize that as 2 very neutral and unimpressed people, plus
>>> 3 in favour. Which seems enough to commit.
>
>> That's about right, although I would put it a bit stronger than that.
>> But if we're the only people unimpressed I'm not going to object further.
>
> FWIW, I would vote against it also. I do not find this to be a natural
> extension of RAISE; it adds all sorts of semantic issues. (In particular,
> what is the evaluation order of the WHEN versus the other subexpressions
> of the RAISE?)

What I liked about this syntax was that we could eventually have:

RAISE ASSERT WHEN stuff;

...and if assertions are disabled, we can skip evaluating the
condition. If you just write an IF .. THEN block you can't do that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> FWIW, I would vote against it also. I do not find this to be a natural
>> extension of RAISE; it adds all sorts of semantic issues. (In particular,
>> what is the evaluation order of the WHEN versus the other subexpressions
>> of the RAISE?)

> What I liked about this syntax was that we could eventually have:
> RAISE ASSERT WHEN stuff;
> ...and if assertions are disabled, we can skip evaluating the
> condition. If you just write an IF .. THEN block you can't do that.

Well, if that's what you want, let's just invent

ASSERT condition

and not tangle RAISE into it. The analogy to EXIT WHEN is a lot
cleaner in this form: no order-of-evaluation issues, no questions
of whether a sub-clause results in totally changing the meaning
of the command. And if your argument is partially based on
how much you have to type, doesn't this way dominate all others?

regards, tom lane

On Wed, Nov 19, 2014 at 12:01 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> FWIW, I would vote against it also. I do not find this to be a natural
>>> extension of RAISE; it adds all sorts of semantic issues. (In particular,
>>> what is the evaluation order of the WHEN versus the other subexpressions
>>> of the RAISE?)
>
>> What I liked about this syntax was that we could eventually have:
>> RAISE ASSERT WHEN stuff;
>> ...and if assertions are disabled, we can skip evaluating the
>> condition. If you just write an IF .. THEN block you can't do that.
>
> Well, if that's what you want, let's just invent
>
> ASSERT condition
>
> and not tangle RAISE into it. The analogy to EXIT WHEN is a lot
> cleaner in this form: no order-of-evaluation issues, no questions
> of whether a sub-clause results in totally changing the meaning
> of the command. And if your argument is partially based on
> how much you have to type, doesn't this way dominate all others?

That doesn't bother me any.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

2014-11-19 17:13 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > On 11/19/2014 06:35 AM, Simon Riggs wrote:
> >> I seem to share the same opinion with Andrew: its not going to hurt to
> >> include this, but its not gonna cause dancing in the streets either. I
> >> would characterize that as 2 very neutral and unimpressed people, plus
> >> 3 in favour. Which seems enough to commit.
>
> > That's about right, although I would put it a bit stronger than that.
> > But if we're the only people unimpressed I'm not going to object further.
>
> FWIW, I would vote against it also. I do not find this to be a natural
> extension of RAISE; it adds all sorts of semantic issues. (In particular,
> what is the evaluation order of the WHEN versus the other subexpressions
> of the RAISE?)
>

last query looks clean for me. First we evaluate WHEN expression, next (if
previous expression is true) we evaluate a expressions inside RAISE
statement.

Regards

Pavel

>
> regards, tom lane
>

2014-11-19 17:43 GMT+01:00 Robert Haas <robertmhaas(at)gmail(dot)com>:

> On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >> On 11/19/2014 06:35 AM, Simon Riggs wrote:
> >>> I seem to share the same opinion with Andrew: its not going to hurt to
> >>> include this, but its not gonna cause dancing in the streets either. I
> >>> would characterize that as 2 very neutral and unimpressed people, plus
> >>> 3 in favour. Which seems enough to commit.
> >
> >> That's about right, although I would put it a bit stronger than that.
> >> But if we're the only people unimpressed I'm not going to object
> further.
> >
> > FWIW, I would vote against it also. I do not find this to be a natural
> > extension of RAISE; it adds all sorts of semantic issues. (In
> particular,
> > what is the evaluation order of the WHEN versus the other subexpressions
> > of the RAISE?)
>
> What I liked about this syntax was that we could eventually have:
>
> RAISE ASSERT WHEN stuff;
>
> ...and if assertions are disabled, we can skip evaluating the
> condition. If you just write an IF .. THEN block you can't do that.
>

I share this idea. It is possible next step

Pavel

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

2014-11-19 18:01 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> FWIW, I would vote against it also. I do not find this to be a natural
> >> extension of RAISE; it adds all sorts of semantic issues. (In
> particular,
> >> what is the evaluation order of the WHEN versus the other subexpressions
> >> of the RAISE?)
>
> > What I liked about this syntax was that we could eventually have:
> > RAISE ASSERT WHEN stuff;
> > ...and if assertions are disabled, we can skip evaluating the
> > condition. If you just write an IF .. THEN block you can't do that.
>
> Well, if that's what you want, let's just invent
>
> ASSERT condition
>
>
there was this proposal .. ASSERT statement .. related discuss was
finished, because it needs a reserved keyword "ASSERT".

> and not tangle RAISE into it. The analogy to EXIT WHEN is a lot
> cleaner in this form: no order-of-evaluation issues, no questions
> of whether a sub-clause results in totally changing the meaning
> of the command. And if your argument is partially based on
> how much you have to type, doesn't this way dominate all others?
>
> regards, tom lane
>

On 2014-11-19 23:18, Pavel Stehule wrote:
> 2014-11-19 18:01 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> FWIW, I would vote against it also. I do not find this to be a natural
>>>> extension of RAISE; it adds all sorts of semantic issues. (In
>> particular,
>>>> what is the evaluation order of the WHEN versus the other subexpressions
>>>> of the RAISE?)
>>
>>> What I liked about this syntax was that we could eventually have:
>>> RAISE ASSERT WHEN stuff;
>>> ...and if assertions are disabled, we can skip evaluating the
>>> condition. If you just write an IF .. THEN block you can't do that.
>>
>> Well, if that's what you want, let's just invent
>>
>> ASSERT condition
>>
>>
> there was this proposal .. ASSERT statement .. related discuss was
> finished, because it needs a reserved keyword "ASSERT".

Finished? Really?

This was Heikki's take on the discussion that took place a good while
ago: http://www.postgresql.org/message-id/5405FF73.1010206@vmware.com.
And in the same thread you also said you like it:
http://www.postgresql.org/message-id/CAFj8pRAC-ZWDrbU-uj=xQOWQtbAqR5oXsM1xYOyhZmyeuvZvQA@mail.gmail.co.
But perhaps you've changed your mind since then (which is fine.) Or
maybe that was only in the case where we'd have a special mode where you
could opt-in if you're willing to accept the backwards compatibility issue?

I also went back to the original thread, and I think Heikki's summary
dismissed e.g. Robert's criticism quite lightly:
http://www.postgresql.org/message-id/CA+TgmobWoSSRNcV_iJK3xhsytXb7Dv0AWGvWkMEurNTOVEZYyw@mail.gmail.com

.marko

Marko Tiikkaja <marko(at)joh(dot)to> writes:
> I also went back to the original thread, and I think Heikki's summary
> dismissed e.g. Robert's criticism quite lightly:
> http://www.postgresql.org/message-id/CA+TgmobWoSSRNcV_iJK3xhsytXb7Dv0AWGvWkMEurNTOVEZYyw@mail.gmail.com

The core of that complaint is that we'd have to make ASSERT a plpgsql
reserved word, which is true enough as things stand today. However,
why is it that plpgsql statement-introducing keywords need to be
reserved? The only reason for that AFAICS is to allow us to distinguish
the statements from assignments. But it seems like that could possibly
be gotten around with some work. The first thing that comes to mind is
for the lexer to do one-token lookahead and assume that the second
token of an assignment must be ":=" (aka "="), ".", or "[". (Have
I missed any cases?) Then, any statement for which the second token
can't be one of those, which is surely most if not all of them, could
have an unreserved leading keyword. This would at a stroke dereserve
about half of plpgsql's existing reserved words, as well as remove a
roadblock for ASSERT and other new statements.

regards, tom lane

2014-11-19 23:38 GMT+01:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 2014-11-19 23:18, Pavel Stehule wrote:
>
>> 2014-11-19 18:01 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>
>>>> On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>
>>>>> FWIW, I would vote against it also. I do not find this to be a natural
>>>>> extension of RAISE; it adds all sorts of semantic issues. (In
>>>>>
>>>> particular,
>>>
>>>> what is the evaluation order of the WHEN versus the other subexpressions
>>>>> of the RAISE?)
>>>>>
>>>>
>>> What I liked about this syntax was that we could eventually have:
>>>> RAISE ASSERT WHEN stuff;
>>>> ...and if assertions are disabled, we can skip evaluating the
>>>> condition. If you just write an IF .. THEN block you can't do that.
>>>>
>>>
>>> Well, if that's what you want, let's just invent
>>>
>>> ASSERT condition
>>>
>>>
>>> there was this proposal .. ASSERT statement .. related discuss was
>> finished, because it needs a reserved keyword "ASSERT".
>>
>
> Finished? Really?
>
> This was Heikki's take on the discussion that took place a good while ago:
> http://www.postgresql.org/message-id/5405FF73.1010206@vmware.com. And in
> the same thread you also said you like it: http://www.postgresql.org/
> message-id/CAFj8pRAC-ZWDrbU-uj=xQOWQtbAqR5oXsM1xYOyhZmyeuvZvQ
> A(at)mail(dot)gmail(dot)co(dot) But perhaps you've changed your mind since then (which
> is fine.) Or maybe that was only in the case where we'd have a special
> mode where you could opt-in if you're willing to accept the backwards
> compatibility issue?
>
> I also went back to the original thread, and I think Heikki's summary
> dismissed e.g. Robert's criticism quite lightly:
> http://www.postgresql.org/message-id/CA+TgmobWoSSRNcV_
> iJK3xhsytXb7Dv0AWGvWkMEurNTOVEZYyw(at)mail(dot)gmail(dot)com
>
>
this discuss is too long. I shouldn't remember all details well. Proposal
of plpgsql statement ASSERT was there, but there was not a agreement of
syntax (as statement X as function call) and one objective disadvantage was
request of new keyword. So I throw this idea as unacceptable. I have no
objections against a statement ASSERT still - but there was not a strong
agreement, so my next proposal (and some common agreement was on RAISE
WHEN).

>
> .marko
>

2014-11-19 23:54 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Marko Tiikkaja <marko(at)joh(dot)to> writes:
> > I also went back to the original thread, and I think Heikki's summary
> > dismissed e.g. Robert's criticism quite lightly:
> >
> http://www.postgresql.org/message-id/CA+TgmobWoSSRNcV_iJK3xhsytXb7Dv0AWGvWkMEurNTOVEZYyw@mail.gmail.com
>
> The core of that complaint is that we'd have to make ASSERT a plpgsql
> reserved word, which is true enough as things stand today. However,
> why is it that plpgsql statement-introducing keywords need to be
> reserved? The only reason for that AFAICS is to allow us to distinguish
> the statements from assignments. But it seems like that could possibly
> be gotten around with some work. The first thing that comes to mind is
> for the lexer to do one-token lookahead and assume that the second
> token of an assignment must be ":=" (aka "="), ".", or "[". (Have
> I missed any cases?) Then, any statement for which the second token
> can't be one of those, which is surely most if not all of them, could
> have an unreserved leading keyword. This would at a stroke dereserve
> about half of plpgsql's existing reserved words, as well as remove a
> roadblock for ASSERT and other new statements.
>

Doesn't it close a doors to implement a procedures call without explicit
CALL statement (like PL/SQL) ?

Personally I doesn't feel to introduce lot of new keywords (statements) to
plpgsql. Probably only two - ASSERT (assertions), PRAGMA (some cooperation
with plpgsql extensions).

>
> regards, tom lane
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> 2014-11-19 23:54 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> The core of that complaint is that we'd have to make ASSERT a plpgsql
>> reserved word, which is true enough as things stand today. However,
>> why is it that plpgsql statement-introducing keywords need to be
>> reserved?

> Doesn't it close a doors to implement a procedures call without explicit
> CALL statement (like PL/SQL) ?

So, in order to leave the door open for implicit CALL (which nobody's
ever proposed or tried to implement AFAIR), you're willing to see every
other proposal for new plpgsql statements go down the drain due to
objections to new reserved words? I think your priorities are skewed.

(If we did want to allow implicit CALL, I suspect that things could be
hacked so that it worked for any function name that wasn't already an
unreserved keyword, anyway. So you'd be no worse off.)

> Personally I doesn't feel to introduce lot of new keywords (statements) to
> plpgsql. Probably only two - ASSERT (assertions), PRAGMA (some cooperation
> with plpgsql extensions).

I can't say that either of those excite me particularly, so the idea
that those two are the only new statements we'd ever want to add seems
improbable.

regards, tom lane

On 19 November 2014 23:29, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> 2014-11-19 23:54 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>> The core of that complaint is that we'd have to make ASSERT a plpgsql
>>> reserved word, which is true enough as things stand today. However,
>>> why is it that plpgsql statement-introducing keywords need to be
>>> reserved?
>
>> Doesn't it close a doors to implement a procedures call without explicit
>> CALL statement (like PL/SQL) ?
>
> So, in order to leave the door open for implicit CALL (which nobody's
> ever proposed or tried to implement AFAIR), you're willing to see every
> other proposal for new plpgsql statements go down the drain due to
> objections to new reserved words? I think your priorities are skewed.
>
> (If we did want to allow implicit CALL, I suspect that things could be
> hacked so that it worked for any function name that wasn't already an
> unreserved keyword, anyway. So you'd be no worse off.)

Implictly CALLed procedures/function-that-return-void would be a great
feature for 9.5

Great proposal.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

I wrote:
> The core of that complaint is that we'd have to make ASSERT a plpgsql
> reserved word, which is true enough as things stand today. However,
> why is it that plpgsql statement-introducing keywords need to be
> reserved? The only reason for that AFAICS is to allow us to distinguish
> the statements from assignments. But it seems like that could possibly
> be gotten around with some work.

Attached is a draft patch that de-reserves 17 of plpgsql's existing
reserved words, leaving 24 still reserved (of which only 9 are not also
reserved in the core grammar). This would make it painless to invent an
ASSERT statement, as well as any other new statement type that's not
associated with looping or block structure. (The limiting factor on those
is that if a statement could have an opt_block_label, its keyword still
has to be reserved, unless we want to complicate matters a bunch more.)

regards, tom lane

On Sun, Nov 23, 2014 at 4:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> The core of that complaint is that we'd have to make ASSERT a plpgsql
>> reserved word, which is true enough as things stand today. However,
>> why is it that plpgsql statement-introducing keywords need to be
>> reserved? The only reason for that AFAICS is to allow us to distinguish
>> the statements from assignments. But it seems like that could possibly
>> be gotten around with some work.
>
> Attached is a draft patch that de-reserves 17 of plpgsql's existing
> reserved words, leaving 24 still reserved (of which only 9 are not also
> reserved in the core grammar). This would make it painless to invent an
> ASSERT statement, as well as any other new statement type that's not
> associated with looping or block structure. (The limiting factor on those
> is that if a statement could have an opt_block_label, its keyword still
> has to be reserved, unless we want to complicate matters a bunch more.)

I like the idea of making these keywords less-reserved, but I'm
wondering how future-proof it is. It seems to rely heavily on the
fact that the syntax for lvalues is extremely restricted. Allowing
foo(bar) as an lvalue, for example, would pretty much require
completely reverting this, AFAICS, as would any other type of lvalue
that needs more than one token of lookahead to identify. How sure are
we that we're never going to want to do something like that?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sun, Nov 23, 2014 at 4:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Attached is a draft patch that de-reserves 17 of plpgsql's existing
>> reserved words, leaving 24 still reserved (of which only 9 are not also
>> reserved in the core grammar). This would make it painless to invent an
>> ASSERT statement, as well as any other new statement type that's not
>> associated with looping or block structure. (The limiting factor on those
>> is that if a statement could have an opt_block_label, its keyword still
>> has to be reserved, unless we want to complicate matters a bunch more.)

> I like the idea of making these keywords less-reserved, but I'm
> wondering how future-proof it is. It seems to rely heavily on the
> fact that the syntax for lvalues is extremely restricted. Allowing
> foo(bar) as an lvalue, for example, would pretty much require
> completely reverting this, AFAICS, as would any other type of lvalue
> that needs more than one token of lookahead to identify. How sure are
> we that we're never going to want to do something like that?

Two comments on that:

1. What is the likely use-case for such a thing (considering SQL is not
exactly friendly to pointers or reference types), and is it more
interesting than new statements that we are going to reject on the grounds
that we don't want to reserve any more plpgsql keywords?

2. The actual behavior would be the same as it would be for the case of
implicit-CALL that we discussed upthread. Namely, that it'd work just
fine as long as "foo" isn't any unreserved keyword. So the net effect
would be that plpgsql keywords would be sort of semi-reserved, much like
col_name_keywords in the core grammar: you can use 'em as variable names
but not necessarily as function names. With the current behavior where
they're fully reserved, you can't use them as either, not even where the
context is completely unambiguous. I have not heard anyone complaining
that col_name_keyword is a dumb idea and we should make all keywords fully
reserved.

regards, tom lane

On Mon, Nov 24, 2014 at 2:07 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Two comments on that:
>
> 1. What is the likely use-case for such a thing (considering SQL is not
> exactly friendly to pointers or reference types),

I happen to know that Oracle supports more possible LHS syntaxes in
PL/SQL than we do in PL/pgsql, including things like foo(1) := 3.
There is more than one problem with supporting that syntax in
PL/pgSQL, and I haven't heard anyone complaining about its absence.
But it doesn't have to be that thing particularly: anything that even
vaguely resembles a general expression syntax on the LHS is going to
run into this.

> and is it more
> interesting than new statements that we are going to reject on the grounds
> that we don't want to reserve any more plpgsql keywords?

Probably not, but my crystal ball isn't working too well today.

> 2. The actual behavior would be the same as it would be for the case of
> implicit-CALL that we discussed upthread. Namely, that it'd work just
> fine as long as "foo" isn't any unreserved keyword. So the net effect
> would be that plpgsql keywords would be sort of semi-reserved, much like
> col_name_keywords in the core grammar: you can use 'em as variable names
> but not necessarily as function names. With the current behavior where
> they're fully reserved, you can't use them as either, not even where the
> context is completely unambiguous. I have not heard anyone complaining
> that col_name_keyword is a dumb idea and we should make all keywords fully
> reserved.

I see. Well, that sounds fine, then.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi all

so one problem is closed, and we can start speaking about ASSERT statement.

I propose a (old) syntax

ASSERT expr [, message]

possible questions:

* should be assertions globally enabled/disabled? - I have no personal
preference in this question.

* can be ASSERT exception handled ? - I prefer this be unhandled exception
- like query_canceled because It should not be masked by plpgsql EXCEPTION
WHEN OTHERS ...

Ideas, notices?

Regards

Pavel

On 11/26/14 8:55 AM, Pavel Stehule wrote:
> * should be assertions globally enabled/disabled? - I have no personal
> preference in this question.

I think so. The way I would use this function is to put expensive
checks into strategic locations which would only run when developing
locally (and additionally perhaps in one of the test environments.) And
in that case I'd like to globally disable them for the live environment.

> * can be ASSERT exception handled ? - I prefer this be unhandled exception
> - like query_canceled because It should not be masked by plpgsql EXCEPTION
> WHEN OTHERS ...

I don't care much either way, as long as we get good information about
what went wrong. A stack trace and hopefully something like
print_strict_params for parameters to the "expr".

.marko

2014-11-26 13:31 GMT+01:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 11/26/14 8:55 AM, Pavel Stehule wrote:
>
>> * should be assertions globally enabled/disabled? - I have no personal
>> preference in this question.
>>
>
> I think so. The way I would use this function is to put expensive checks
> into strategic locations which would only run when developing locally (and
> additionally perhaps in one of the test environments.) And in that case
> I'd like to globally disable them for the live environment.
>

ok

>
> * can be ASSERT exception handled ? - I prefer this be unhandled exception
>> - like query_canceled because It should not be masked by plpgsql EXCEPTION
>> WHEN OTHERS ...
>>
>
> I don't care much either way, as long as we get good information about
> what went wrong. A stack trace and hopefully something like
> print_strict_params for parameters to the "expr".
>

There is more ways, I can live with both

Pavel

>
>
> .marko
>

Hi

2014-11-26 16:46 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

>
>
> 2014-11-26 13:31 GMT+01:00 Marko Tiikkaja <marko(at)joh(dot)to>:
>
>> On 11/26/14 8:55 AM, Pavel Stehule wrote:
>>
>>> * should be assertions globally enabled/disabled? - I have no personal
>>> preference in this question.
>>>
>>
>> I think so. The way I would use this function is to put expensive checks
>> into strategic locations which would only run when developing locally (and
>> additionally perhaps in one of the test environments.) And in that case
>> I'd like to globally disable them for the live environment.
>>
>
> ok
>
>
>>
>> * can be ASSERT exception handled ? - I prefer this be unhandled
>>> exception
>>> - like query_canceled because It should not be masked by plpgsql
>>> EXCEPTION
>>> WHEN OTHERS ...
>>>
>>
>> I don't care much either way, as long as we get good information about
>> what went wrong. A stack trace and hopefully something like
>> print_strict_params for parameters to the "expr".
>>
>
> There is more ways, I can live with both
>

here is proof concept

what do you think about it?

Regards

Pavel

>
> Pavel
>
>
>
>>
>>
>> .marko
>>
>
>

Hi

any comments to last proposal and patch?

Pavel

2014-11-26 19:52 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>
> Hi
>
> 2014-11-26 16:46 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>
>>
>>
>> 2014-11-26 13:31 GMT+01:00 Marko Tiikkaja <marko(at)joh(dot)to>:
>>
>>> On 11/26/14 8:55 AM, Pavel Stehule wrote:
>>>
>>>> * should be assertions globally enabled/disabled? - I have no personal
>>>> preference in this question.
>>>>
>>>
>>> I think so. The way I would use this function is to put expensive
>>> checks into strategic locations which would only run when developing
>>> locally (and additionally perhaps in one of the test environments.) And in
>>> that case I'd like to globally disable them for the live environment.
>>>
>>
>> ok
>>
>>
>>>
>>> * can be ASSERT exception handled ? - I prefer this be unhandled
>>>> exception
>>>> - like query_canceled because It should not be masked by plpgsql
>>>> EXCEPTION
>>>> WHEN OTHERS ...
>>>>
>>>
>>> I don't care much either way, as long as we get good information about
>>> what went wrong. A stack trace and hopefully something like
>>> print_strict_params for parameters to the "expr".
>>>
>>
>> There is more ways, I can live with both
>>
>
> here is proof concept
>
> what do you think about it?
>
> Regards
>
> Pavel
>
>
>>
>> Pavel
>>
>>
>>
>>>
>>>
>>> .marko
>>>
>>
>>
>

Pavel Stehule wrote:
> Hi
>
> any comments to last proposal and patch?

WTH is that pstrdup("hint is null") thing? Debugging leftover?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

2014-12-14 18:57 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
>
> Pavel Stehule wrote:
> > Hi
> >
> > any comments to last proposal and patch?
>
> WTH is that pstrdup("hint is null") thing? Debugging leftover?
>

No, only not well error message. I propose a syntax ASSERT
boolean_expression [, text expression ] -- text expression is >>hint<< for
assertion debugging

This text expression should not be null when it is used. I am not sure,
what is well behave - so when assertions fails and text expression is null,
then I use message "hint is null" as hint.

Regards

Pavel

>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Hi all

is there some agreement on this behave of ASSERT statement?

I would to assign last patch to next commitfest.

Possible changes:

* I would to simplify a behave of evaluating of message expression -
probably I disallow NULL there.
* GUC enable_asserts will be supported
* a assert exception should not be handled by PLpgSQL handler -- like
CANCEL exception

Regards

Pavel

2014-12-14 19:54 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

>
>
> 2014-12-14 18:57 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
>>
>> Pavel Stehule wrote:
>> > Hi
>> >
>> > any comments to last proposal and patch?
>>
>> WTH is that pstrdup("hint is null") thing? Debugging leftover?
>>
>
> No, only not well error message. I propose a syntax ASSERT
> boolean_expression [, text expression ] -- text expression is >>hint<< for
> assertion debugging
>
> This text expression should not be null when it is used. I am not sure,
> what is well behave - so when assertions fails and text expression is null,
> then I use message "hint is null" as hint.
>
> Regards
>
> Pavel
>
>
>>
>> --
>> Álvaro Herrera http://www.2ndQuadrant.com/
>> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>>
>

On 1/21/15 3:10 PM, Pavel Stehule wrote:
>
> is there some agreement on this behave of ASSERT statement?
>
> I would to assign last patch to next commitfest.
>
> Possible changes:
>
> * I would to simplify a behave of evaluating of message expression - probably I disallow NULL there.

Well, the only thing I could see you doing there is throwing a different error if the hint is null. I don't see that as an improvement. I'd just leave it as-is.

> * GUC enable_asserts will be supported

That would be good. Would that allow for enabling/disabling on a per-function basis too?

> * a assert exception should not be handled by PLpgSQL handler -- like CANCEL exception

+1
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

Hi

here is updated patch

2015-01-21 23:28 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:

> On 1/21/15 3:10 PM, Pavel Stehule wrote:
>
>>
>> is there some agreement on this behave of ASSERT statement?
>>
>> I would to assign last patch to next commitfest.
>>
>> Possible changes:
>>
>> * I would to simplify a behave of evaluating of message expression -
>> probably I disallow NULL there.
>>
>
> Well, the only thing I could see you doing there is throwing a different
> error if the hint is null. I don't see that as an improvement. I'd just
> leave it as-is.
>

I enabled a NULL - but enforced a WARNING before.

>
> * GUC enable_asserts will be supported
>>
>
> That would be good. Would that allow for enabling/disabling on a
> per-function basis too?
>

sure - there is only question if we develop a #option
enable|disable_asserts. I have no string idea.

>
> * a assert exception should not be handled by PLpgSQL handler -- like
>> CANCEL exception
>>
>
> +1
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

On 1/22/15 2:01 PM, Pavel Stehule wrote:
>
> * I would to simplify a behave of evaluating of message expression - probably I disallow NULL there.
>
>
> Well, the only thing I could see you doing there is throwing a different error if the hint is null. I don't see that as an improvement. I'd just leave it as-is.
>
>
> I enabled a NULL - but enforced a WARNING before.

I don't see the separate warning as being helpful. I'd just do something like

+ (err_hint != NULL) ? errhint("%s", err_hint) : errhint("Message attached to failed assertion is null") ));

There should also be a test case for a NULL message.

> * GUC enable_asserts will be supported
>
>
> That would be good. Would that allow for enabling/disabling on a per-function basis too?
>
>
> sure - there is only question if we develop a #option enable|disable_asserts. I have no string idea.

The option would be nice, but I don't think it's strictly necessary. The big thing is being able to control this on a per-function basis (which I think you can do with ALTER FUNCTION SET?)
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

2015-01-26 22:34 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:

> On 1/22/15 2:01 PM, Pavel Stehule wrote:
>
>>
>> * I would to simplify a behave of evaluating of message
>> expression - probably I disallow NULL there.
>>
>>
>> Well, the only thing I could see you doing there is throwing a
>> different error if the hint is null. I don't see that as an improvement.
>> I'd just leave it as-is.
>>
>>
>> I enabled a NULL - but enforced a WARNING before.
>>
>
> I don't see the separate warning as being helpful. I'd just do something
> like
>
> + (err_hint != NULL) ? errhint("%s",
> err_hint) : errhint("Message attached to failed assertion is null") ));
>
>
ok

> There should also be a test case for a NULL message.
>
> * GUC enable_asserts will be supported
>>
>>
>> That would be good. Would that allow for enabling/disabling on a
>> per-function basis too?
>>
>>
>> sure - there is only question if we develop a #option
>> enable|disable_asserts. I have no string idea.
>>
>
> The option would be nice, but I don't think it's strictly necessary. The
> big thing is being able to control this on a per-function basis (which I
> think you can do with ALTER FUNCTION SET?)

you can do it without any change now

>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

2015-01-26 22:34 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:

> On 1/22/15 2:01 PM, Pavel Stehule wrote:
>
>>
>> * I would to simplify a behave of evaluating of message
>> expression - probably I disallow NULL there.
>>
>>
>> Well, the only thing I could see you doing there is throwing a
>> different error if the hint is null. I don't see that as an improvement.
>> I'd just leave it as-is.
>>
>>
>> I enabled a NULL - but enforced a WARNING before.
>>
>
> I don't see the separate warning as being helpful. I'd just do something
> like
>
> + (err_hint != NULL) ? errhint("%s",
> err_hint) : errhint("Message attached to failed assertion is null") ));
>

done

>
> There should also be a test case for a NULL message.
>

is there, if I understand well

Regards

Pavel

>
> * GUC enable_asserts will be supported
>>
>>
>> That would be good. Would that allow for enabling/disabling on a
>> per-function basis too?
>>
>>
>> sure - there is only question if we develop a #option
>> enable|disable_asserts. I have no string idea.
>>
>
> The option would be nice, but I don't think it's strictly necessary. The
> big thing is being able to control this on a per-function basis (which I
> think you can do with ALTER FUNCTION SET?)
>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

On 1/27/15 1:30 PM, Pavel Stehule wrote:
> I don't see the separate warning as being helpful. I'd just do something like
>
> + (err_hint != NULL) ? errhint("%s", err_hint) : errhint("Message attached to failed assertion is null") ));
>
>
> done
>
>
> There should also be a test case for a NULL message.
>
>
> is there, if I understand well

I see it now. Looks good.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

2015-01-28 0:13 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:

> On 1/27/15 1:30 PM, Pavel Stehule wrote:
>
>> I don't see the separate warning as being helpful. I'd just do
>> something like
>>
>> + (err_hint != NULL) ? errhint("%s",
>> err_hint) : errhint("Message attached to failed assertion is null") ));
>>
>>
>> done
>>
>>
>> There should also be a test case for a NULL message.
>>
>>
>> is there, if I understand well
>>
>
> I see it now. Looks good.

please, can you enhance a documentation part - I am sorry, my English
skills are not enough

Thank you

Pavel

>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

2015-01-28 0:13 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:

> On 1/27/15 1:30 PM, Pavel Stehule wrote:
>
>> I don't see the separate warning as being helpful. I'd just do
>> something like
>>
>> + (err_hint != NULL) ? errhint("%s",
>> err_hint) : errhint("Message attached to failed assertion is null") ));
>>
>>
>> done
>>
>>
>> There should also be a test case for a NULL message.
>>
>>
>> is there, if I understand well
>>
>
> I see it now. Looks good.

updated version with Jim Nasby's doc and rebase against last changes in
plpgsql.

Regards

Pavel

>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested

Note that pgcrypto is failing 3 tests, same as in master.

The new status of this patch is: Ready for Committer

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> updated version with Jim Nasby's doc and rebase against last changes in
> plpgsql.

I started looking at this patch. ISTM there are some pretty questionable
design decisions in it:

1. Why create a core GUC to control a behavior that's plpgsql-only?
I think it'd make more sense for this to be a plgsql custom GUC
(ie, "plpgsql.enable_asserts" or some such name).

2. I find the use of errdetail to report whether the assertion condition
evaluated to FALSE or to NULL to be pretty useless. (BTW, is considering
NULL to be a failure the right thing? SQL CHECK conditions consider NULL
to be allowed ...) I also don't care at all for reporting the internal
text of the assertion expression in the errdetail: that will expose
implementation details (ie, exactly what does plpgsql convert the user
expression to, does it remove comments, etc) which we will then be
constrained from changing for fear of breaking client code that expects a
particular spelling of the condition. I think we should just drop that
whole business. The user can report the condition in her message, if she
feels the need to.

3. If we drop the errdetail as per #2, then reporting the optional
user-supplied string as a HINT would be just plain bizarre; not that it
wasn't bizarre already, because there's no good reason to suppose that
whatever the programmer has to say about the assertion is merely a hint.
I also find the behavior for string-evaluates-to-NULL bizarre; it'd be
saner just to leave out the message field, same as if the argument weren't
there. I would suggest that we adopt one of these two definitions for the
optional string:

3a. If string is present and not null, use it as the primary message text
(otherwise use "assertion failed").

3b. If string is present and not null, use it as errdetail, with the
primary message text always being "assertion failed".

I mildly prefer #3a, but could be talked into #3b.

Comments?

regards, tom lane

2015-03-25 0:17 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > updated version with Jim Nasby's doc and rebase against last changes in
> > plpgsql.
>
> I started looking at this patch. ISTM there are some pretty questionable
> design decisions in it:
>
> 1. Why create a core GUC to control a behavior that's plpgsql-only?
> I think it'd make more sense for this to be a plgsql custom GUC
> (ie, "plpgsql.enable_asserts" or some such name).
>

This type of assertations can be implemented in any PL language - so I
prefer global setting. But I have not strong option in this case - this is
question about granularity - and more ways are valid.

>
> 2. I find the use of errdetail to report whether the assertion condition
> evaluated to FALSE or to NULL to be pretty useless. (BTW, is considering
> NULL to be a failure the right thing? SQL CHECK conditions consider NULL
> to be allowed ...)

This is a question - I am happy with SQL CHECK for data, but I am not sure
if same behave is safe for plpgsql (procedural) assert. More stricter
behave is safer - and some bugs in procedures are based on unhandled NULLs
in variables. So in this topic I prefer implemented behave. It is some like:

IF expression THEN
-- I am able do all
...
ELSE
RAISE EXCEPTION 'some is wrong';
END IF;

> I also don't care at all for reporting the internal
> text of the assertion expression in the errdetail: that will expose
> implementation details (ie, exactly what does plpgsql convert the user
> expression to, does it remove comments, etc) which we will then be
> constrained from changing for fear of breaking client code that expects a
> particular spelling of the condition. I think we should just drop that
> whole business. The user can report the condition in her message, if she
> feels the need to.
>
>
+1

> 3. If we drop the errdetail as per #2, then reporting the optional
> user-supplied string as a HINT would be just plain bizarre; not that it
> wasn't bizarre already, because there's no good reason to suppose that
> whatever the programmer has to say about the assertion is merely a hint.
> I also find the behavior for string-evaluates-to-NULL bizarre; it'd be
> saner just to leave out the message field, same as if the argument weren't
> there. I would suggest that we adopt one of these two definitions for the
> optional string:
>
> 3a. If string is present and not null, use it as the primary message text
> (otherwise use "assertion failed").
>
> 3b. If string is present and not null, use it as errdetail, with the
> primary message text always being "assertion failed".
>
> I mildly prefer #3a, but could be talked into #3b.
>

I prefer #3b - there is more informations.

Regards

Pavel

>
> Comments?
>
> regards, tom lane
>

On 3/25/15 1:21 AM, Pavel Stehule wrote:
> 2015-03-25 0:17 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
> <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>>:
>
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com
> <mailto:pavel(dot)stehule(at)gmail(dot)com>> writes:
> > updated version with Jim Nasby's doc and rebase against last changes in
> > plpgsql.
>
> I started looking at this patch. ISTM there are some pretty
> questionable
> design decisions in it:
>
> 1. Why create a core GUC to control a behavior that's plpgsql-only?
> I think it'd make more sense for this to be a plgsql custom GUC
> (ie, "plpgsql.enable_asserts" or some such name).
>
>
> This type of assertations can be implemented in any PL language - so I
> prefer global setting. But I have not strong option in this case - this
> is question about granularity - and more ways are valid.

+1

> 2. I find the use of errdetail to report whether the assertion condition
> evaluated to FALSE or to NULL to be pretty useless. (BTW, is
> considering
> NULL to be a failure the right thing? SQL CHECK conditions consider
> NULL
> to be allowed ...)
>
>
> This is a question - I am happy with SQL CHECK for data, but I am not
> sure if same behave is safe for plpgsql (procedural) assert. More
> stricter behave is safer - and some bugs in procedures are based on
> unhandled NULLs in variables. So in this topic I prefer implemented
> behave. It is some like:

+1. I think POLA here is that an assert must be true and only true to be
valid. If someone was unhappy with that they could always coalesce(...,
true).
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com> writes:
> On 3/25/15 1:21 AM, Pavel Stehule wrote:
>> 2015-03-25 0:17 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
>> <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>>:
>>> (BTW, is considering
>>> NULL to be a failure the right thing? SQL CHECK conditions consider
>>> NULL to be allowed ...)

>> This is a question - I am happy with SQL CHECK for data, but I am not
>> sure if same behave is safe for plpgsql (procedural) assert. More
>> stricter behave is safer - and some bugs in procedures are based on
>> unhandled NULLs in variables. So in this topic I prefer implemented
>> behave. It is some like:

> +1. I think POLA here is that an assert must be true and only true to be
> valid. If someone was unhappy with that they could always coalesce(...,
> true).

Fair enough. Committed with the other changes.

regards, tom lane

2015-03-26 0:08 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com> writes:
> > On 3/25/15 1:21 AM, Pavel Stehule wrote:
> >> 2015-03-25 0:17 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
> >> <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>>:
> >>> (BTW, is considering
> >>> NULL to be a failure the right thing? SQL CHECK conditions consider
> >>> NULL to be allowed ...)
>
> >> This is a question - I am happy with SQL CHECK for data, but I am not
> >> sure if same behave is safe for plpgsql (procedural) assert. More
> >> stricter behave is safer - and some bugs in procedures are based on
> >> unhandled NULLs in variables. So in this topic I prefer implemented
> >> behave. It is some like:
>
> > +1. I think POLA here is that an assert must be true and only true to be
> > valid. If someone was unhappy with that they could always coalesce(...,
> > true).
>
> Fair enough. Committed with the other changes.
>

Thank you very much

regards

Pavel

>
> regards, tom lane
>