subject:"Re\: \[HACKERS\] proposal\: plpgsql \- Assert statement"

2014-11-19 17:13 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

 Andrew Dunstan and...@dunslane.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-19 17:43 GMT+01:00 Robert Haas robertmh...@gmail.com:

 On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Andrew Dunstan and...@dunslane.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-19 18:01 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

 Robert Haas robertmh...@gmail.com writes:
  On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane t...@sss.pgh.pa.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-19 Thread Marko Tiikkaja

On 2014-11-19 23:18, Pavel Stehule wrote:

2014-11-19 18:01 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

Robert Haas robertmh...@gmail.com writes:

On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane t...@sss.pgh.pa.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.1010...@vmware.com.
And in the same thread you also said you like it:
http://www.postgresql.org/message-id/CAFj8pRAC-ZWDrbU-uj=xqowqtbaqr5oxsm1xyoyhzmyeuvz...@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_ijk3xhsytxb7dv0awgvwkmeurntovez...@mail.gmail.com

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-19 Thread Tom Lane

Marko Tiikkaja ma...@joh.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_ijk3xhsytxb7dv0awgvwkmeurntovez...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-19 23:38 GMT+01:00 Marko Tiikkaja ma...@joh.to:

 On 2014-11-19 23:18, Pavel Stehule wrote:

 2014-11-19 18:01 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

  Robert Haas robertmh...@gmail.com writes:

 On Wed, Nov 19, 2014 at 11:13 AM, Tom Lane t...@sss.pgh.pa.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.1010...@vmware.com. And in
 the same thread you also said you like it: http://www.postgresql.org/
 message-id/CAFj8pRAC-ZWDrbU-uj=xQOWQtbAqR5oXsM1xYOyhZmyeuvZvQ
 a...@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_
 ijk3xhsytxb7dv0awgvwkmeurntovez...@mail.gmail.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-19 23:54 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

Marko Tiikkaja ma...@joh.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_ijk3xhsytxb7dv0awgvwkmeurntovez...@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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-19 Thread Tom Lane

Pavel Stehule pavel.steh...@gmail.com writes:
 2014-11-19 23:54 GMT+01:00 Tom Lane t...@sss.pgh.pa.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 Thread Simon Riggs

On 18 November 2014 01:00, Jim Nasby jim.na...@bluetreble.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 10:23 GMT+01:00 Simon Riggs si...@2ndquadrant.com:

 On 18 November 2014 01:00, Jim Nasby jim.na...@bluetreble.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 Thread Andrew Dunstan



On 11/18/2014 04:23 AM, Simon Riggs wrote:

On 18 November 2014 01:00, Jim Nasby jim.na...@bluetreble.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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 17:08 GMT+01:00 Simon Riggs si...@2ndquadrant.com:

 On 18 November 2014 12:29, Pavel Stehule pavel.steh...@gmail.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 Thread Jim Nasby

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 Thread Andrew Dunstan



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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 21:27 GMT+01:00 Andrew Dunstan and...@dunslane.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 Thread Petr Jelinek


On 18/11/14 22:11, Pavel Stehule wrote:


2014-11-18 21:27 GMT+01:00 Andrew Dunstan and...@dunslane.net
mailto:and...@dunslane.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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 Thread Andrew Dunstan



On 11/18/2014 04:11 PM, Pavel Stehule wrote:



2014-11-18 21:27 GMT+01:00 Andrew Dunstan and...@dunslane.net 
mailto:and...@dunslane.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



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-18 22:28 GMT+01:00 Andrew Dunstan and...@dunslane.net:


 On 11/18/2014 04:11 PM, Pavel Stehule wrote:



 2014-11-18 21:27 GMT+01:00 Andrew Dunstan and...@dunslane.net mailto:
 and...@dunslane.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-17 Thread Simon Riggs

 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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-17 Thread Jim Nasby


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-11-17 Thread Pavel Stehule

2014-11-17 23:58 GMT+01:00 Simon Riggs si...@2ndquadrant.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-10-17 Thread Petr Jelinek


On 16/10/14 13:29, Pavel Stehule wrote:

Hi

2014-10-14 22:57 GMT+02:00 Petr Jelinek p...@2ndquadrant.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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-10-17 Thread Pavel Stehule

2014-10-17 9:14 GMT+02:00 Petr Jelinek p...@2ndquadrant.com:

 On 16/10/14 13:29, Pavel Stehule wrote:

 Hi

 2014-10-14 22:57 GMT+02:00 Petr Jelinek p...@2ndquadrant.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-10-16 Thread Pavel Stehule

2014-10-15 11:57 GMT+02:00 Ali Akbar the.ap...@gmail.com:

 2014-09-30 10:04 GMT+07:00 Jim Nasby j...@nasby.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-10-16 Thread Pavel Stehule

Hi

2014-10-14 22:57 GMT+02:00 Petr Jelinek p...@2ndquadrant.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

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index f195495..eae72f6
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*** END LOOP optional replaceablelabel/
*** 3369,3379 
  raise errors.
  
  synopsis
! RAISE optional replaceable class=parameterlevel/replaceable /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
! RAISE optional replaceable class=parameterlevel/replaceable /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
! RAISE optional replaceable class=parameterlevel/replaceable /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
! RAISE optional replaceable class=parameterlevel/replaceable /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional;
! RAISE ;
  /synopsis
  
  The replaceable class=parameterlevel/replaceable option specifies
--- 3369,3379 
  raise errors.
  
  synopsis
! RAISE optional replaceable class=parameterlevel/replaceable /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional optional WHEN replaceable class=parameterboolean-expression/replaceable /optional;
! RAISE optional replaceable class=parameterlevel/replaceable /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional optional WHEN replaceable class=parameterboolean-expression/replaceable /optional;
! RAISE optional replaceable class=parameterlevel/replaceable /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional optional WHEN replaceable class=parameterboolean-expression/replaceable /optional;
! RAISE optional replaceable class=parameterlevel/replaceable /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional optional WHEN replaceable class=parameterboolean-expression/replaceable /optional;
! RAISE optional WHEN replaceable class=parameterboolean-expression/replaceable /optional;
  /synopsis
  
  The replaceable class=parameterlevel/replaceable option specifies
*** RAISE unique_violation USING MESSAGE = '
*** 3548,3553 
--- 3548,3561 
  /para
 /note
  
+para
+  If literalWHEN/literal is specified, the message or error is raised
+  only if replaceableboolean-expression/ is true.
+

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-10-15 Thread Ali Akbar

2014-09-30 10:04 GMT+07:00 Jim Nasby j...@nasby.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-10-14 Thread Petr Jelinek


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-17 Thread Peter Eisentraut

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.ca...@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.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-17 Thread Peter Eisentraut

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.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-17 Thread Marko Tiikkaja


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-17 Thread Pavel Stehule

2014-09-17 21:00 GMT+02:00 Peter Eisentraut pete...@gmx.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-17 Thread Peter Eisentraut

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.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-17 Thread Pavel Stehule

2014-09-17 21:36 GMT+02:00 Peter Eisentraut pete...@gmx.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.

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-17 Thread Jan Wieck


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-15 Thread Alvaro Herrera

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.ca...@vanquo.pezone.net

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-14 Thread Pavel Stehule

2014-09-09 7:54 GMT+02:00 Craig Ringer cr...@2ndquadrant.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-14 Thread Jan Wieck


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-09 Thread Marko Tiikkaja


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-09 Thread Pavel Stehule

2014-09-09 7:55 GMT+02:00 Craig Ringer cr...@2ndquadrant.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

commit 77ff7203d83e889aa9f5190fe54b04dc7c26a5aa
Author: Pavel Stehule pavel.steh...@gooddata.com
Date:   Tue Sep 9 12:11:15 2014 +0200

initial

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 11cb47b..f8ac200 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2890,8 +2890,20 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 	char	   *err_datatype = NULL;
 	char	   *err_table = NULL;
 	char	   *err_schema = NULL;
+	
 	ListCell   *lc;
 
+	if (stmt-cond != NULL)
+	{
+		bool	value;
+		bool	isnull;
+
+		value = exec_eval_boolean(estate, stmt-cond, isnull);
+		exec_eval_cleanup(estate);
+		if (isnull || value == false)
+			return PLPGSQL_RC_OK;
+	}
+
 	/* RAISE with no parameters: re-throw current exception */
 	if (stmt-condname == NULL  stmt-message == NULL 
 		stmt-options == NIL)
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 893f3a4..1f0b861 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -63,6 +63,7 @@ static	void			current_token_is_not_variable(int tok);
 static	PLpgSQL_expr	*read_sql_construct(int until,
 			int until2,
 			int until3,
+			int until4,
 			const char *expected,
 			const char *sqlstart,
 			bool isexpression,
@@ -105,7 +106,7 @@ static	void			 check_labels(const char *start_label,
 	  int end_location);
 static	PLpgSQL_expr	*read_cursor_args(PLpgSQL_var *cursor,
 		  int until, const char *expected);
-static	List			*read_raise_options(void);
+static	List			*read_raise_options(int *endtok);
 static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 
 %}
@@ -1422,6 +1423,7 @@ for_control		: for_variable K_IN
 			expr1 = read_sql_construct(DOT_DOT,
 	   K_LOOP,
 	   0,
+	   0,
 	   LOOP,
 	   SELECT ,
 	   true,
@@ -1716,6 +1718,7 @@ stmt_raise		: K_RAISE
 	{
 		PLpgSQL_stmt_raise		*new;
 		int	tok;
+		int	endtok;
 
 		new = palloc(sizeof(PLpgSQL_stmt_raise));
 
@@ -1726,6 +1729,7 @@ stmt_raise		: K_RAISE
 		new-message	= NULL;
 		new-params		= NIL;
 		new-options	= NIL;
+		new-cond = NULL;
 
 		tok = yylex();
 		if (tok == 0)
@@ -1796,22 +1800,22 @@ stmt_raise		: K_RAISE
  * or USING to begin the options list.
  */
 tok = yylex();
-if (tok != ','  tok != ';'  tok != K_USING)
+if (tok != ','  tok != ';'  tok != K_USING  tok != K_WHEN)
 	yyerror(syntax error);
 
 while (tok == ',')
 {
 	PLpgSQL_expr *expr;
 
-	expr = read_sql_construct(',', ';', K_USING,
-			  , or ; or USING,
+	expr = read_sql_construct(',', ';', K_USING, K_WHEN,
+			  , or ; or USING or WHEN,
 			  SELECT ,
 			  true, true, true,
 			  NULL, tok);
 	new-params = lappend(new-params, expr);
 }
 			}
-			else if (tok != K_USING)
+			else if (tok != K_USING  tok != K_WHEN)
 			{
 /* must be condition name or SQLSTATE */
 if (tok_is_keyword(tok, yylval,
@@ -1847,7 +1851,13 @@ stmt_raise		: K_RAISE
 			}
 
 			if (tok == K_USING)
-new-options = read_raise_options();
+			{
+new-options = read_raise_options(endtok);
+if (endtok == K_WHEN)
+	new-cond = read_sql_expression(';', ;);
+			}
+			if (tok == K_WHEN)
+new-cond =

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-08 Thread Robert Haas

On Fri, Sep 5, 2014 at 2:16 AM, Pavel Stehule pavel.steh...@gmail.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-08 Thread Craig Ringer

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-08 Thread Craig Ringer

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-06 Thread Petr Jelinek


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-06 Thread Marko Tiikkaja


On 2014-09-06 7:27 AM, Pavel Stehule wrote:

2014-09-05 14:35 GMT+02:00 Jan Wieck j...@wi3ck.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.5030...@joh.to .  I think 
that's a crucial part of the feature.


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


.marko


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-06 Thread Pavel Stehule

2014-09-06 19:26 GMT+02:00 Marko Tiikkaja ma...@joh.to:

 On 2014-09-06 7:27 AM, Pavel Stehule wrote:

 2014-09-05 14:35 GMT+02:00 Jan Wieck j...@wi3ck.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.5030...@joh.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-06 Thread Marko Tiikkaja


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-06 Thread Pavel Stehule

2014-09-06 19:59 GMT+02:00 Marko Tiikkaja ma...@joh.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

Re: [HACKERS] proposal: plpgsql - Assert statement


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-05 9:52 GMT+02:00 Marko Tiikkaja ma...@joh.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

Re: [HACKERS] proposal: plpgsql - Assert statement


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-05 10:33 GMT+02:00 Marko Tiikkaja ma...@joh.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

Re: [HACKERS] proposal: plpgsql - Assert statement


On 9/5/14 10:40 AM, Pavel Stehule wrote:

2014-09-05 10:33 GMT+02:00 Marko Tiikkaja ma...@joh.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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-05 10:57 GMT+02:00 Marko Tiikkaja ma...@joh.to:

 On 9/5/14 10:40 AM, Pavel Stehule wrote:

 2014-09-05 10:33 GMT+02:00 Marko Tiikkaja ma...@joh.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

Re: [HACKERS] proposal: plpgsql - Assert statement


On 9/5/14 11:08 AM, Pavel Stehule wrote:

2014-09-05 10:57 GMT+02:00 Marko Tiikkaja ma...@joh.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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-05 11:17 GMT+02:00 Marko Tiikkaja ma...@joh.to:

 On 9/5/14 11:08 AM, Pavel Stehule wrote:

 2014-09-05 10:57 GMT+02:00 Marko Tiikkaja ma...@joh.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

Re: [HACKERS] proposal: plpgsql - Assert statement

2014-09-05 Thread Jan Wieck


On 09/05/2014 04:40 AM, Pavel Stehule wrote:




2014-09-05 10:33 GMT+02:00 Marko Tiikkaja ma...@joh.to
mailto:ma...@joh.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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: plpgsql - Assert statement