Thread: plpgsql: RAISE

plpgsql: RAISE

From
"Richard Huxton"
Date:
Muggins here volunteered to get RAISE to accept any expression that
evaluates to a string rather than just a string constant. Think I can see
why it wasn't that way already.

Had a look, and this is easy enough:

RAISE NOTICE ''Hello '' || $1 || '' World'';

Also, I can do:

RAISE NOTICE ''Hello '' || ''% World'',$1;

But I don't think I can do both. Haven't looked at lex+9yacc since
university days, but I think we've only got one token of look-ahead. This
means we can't read the expression and *then* decide which option we are in.

(For those who haven't looked at that bit of the code recently
plpgsql_read_expression() slurps up to and including a closing token -
either a ';' or ',' above. You've then lost that finishing token)

The closest I can get is something like:

RAISE NOTICE ''Hello '' || ''% World'',$1;      -- Old style
RAISE IN NOTICE ''Hello '' || $1 || '' World''; -- New "INline" style

Obviously we can use a new token rather than "IN", but it'll do while I'm
testing.

AFAICT there are 4 options:

1. Break the old % format - it's redundant now anyway
2. Add a new token as above
3. Add another parameter to plpgsql_read_expression() so we can unget the
closing token.
4. Alter plpgsql_read_expression() to accept more than one closing token,
and it stops when it reads any of them.

I'm averse to #1 (unless maybe it was accompanied with a small sed/perl
script to automatically fix any code in a pg_dump file)

I don't like gratuitously adding syntax noise with #2.

I don't know whether #3 is even possible. Does anyone more familiar with
yacc/bison know?

The solution for #4 is going to add complexity to read_expression() -
presumably not a speed problem (we're only parsing once) but it'd be nice to
keep things as simple as possible.

The only other things I need to look at are checking I've freed up any store
that's been allocated and casting the expression to text where PG can't
figure that out for itself. These are obviously just a matter of getting a
little more familiar with the code.

Any advice/suggestions gratefully received people.

- Richard Huxton



Re: plpgsql: RAISE

From
Tom Lane
Date:
"Richard Huxton" <dev@archonet.com> writes:
> (For those who haven't looked at that bit of the code recently
> plpgsql_read_expression() slurps up to and including a closing token -
> either a ';' or ',' above. You've then lost that finishing token)

The real problem is that this *isn't* yacc ... if plpgsql had an actual
grammar symbol for "expression" then the change would be trivial.

plpgsql_read_expression is not usable as-is for this purpose, because
"read until token X" is far too simplistic (consider a function call
with two parameters --- the comma between the parameters would be
taken as ending the whole expression).

It might work to add some understanding of nested-parenthesis counting
to the routine; not sure if there are any other shortcomings besides
that one.  But in any case, you need to do significant surgery on that
routine, so adding another return parameter shouldn't bother you.

> 4. Alter plpgsql_read_expression() to accept more than one closing token,
> and it stops when it reads any of them.

AFAICT it already stops on ';' (hardwired into the routine).  So if you
make it pass back what it stopped on, you're set: the grammar entry
becomes just

stmt_raise : K_RAISE lno raise_level

and then the code takes care of swallowing expressions until ';',
similarly to the way SQL commands are handled.  (plpgsql's parsing
methodology is sinfully ugly, isn't it?  But I don't suppose you
want to try to replace it...)
        regards, tom lane


Re: plpgsql: RAISE

From
Jan Wieck
Date:
Tom Lane wrote:
> and then the code takes care of swallowing expressions until ';',
> similarly to the way SQL commands are handled.  (plpgsql's parsing
> methodology is sinfully ugly, isn't it?  But I don't suppose you
> want to try to replace it...)
   It  is,  indeed,  and I'm sorry for that. But it was the only   way I saw to make new features in the PostgreSQL
main query   engine  automatically  available in PL/pgSQL without a single   change.
 


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #



_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com



Re: plpgsql: RAISE

From
"Richard Huxton"
Date:
From: "Jan Wieck" <JanWieck@yahoo.com>

> Tom Lane wrote:
> > and then the code takes care of swallowing expressions until ';',
> > similarly to the way SQL commands are handled.  (plpgsql's parsing
> > methodology is sinfully ugly, isn't it?  But I don't suppose you
> > want to try to replace it...)
>
>     It  is,  indeed,  and I'm sorry for that. But it was the only
>     way I saw to make new features in the PostgreSQL  main  query
>     engine  automatically  available in PL/pgSQL without a single
>     change.

Actually, I like the idea of using the SQL system to evaluate expressions -
why reinvent the wheel?

The only thing needed for this is a grammar for expressions so we can mix
and match with RAISE a bit better. First draft doesn't look too bad - I can
not deal with function-calls and brackets and still have something useful.

- Richard Huxton



Re: plpgsql: RAISE

From
Tom Lane
Date:
"Richard Huxton" <dev@archonet.com> writes:
> Actually, I like the idea of using the SQL system to evaluate expressions -
> why reinvent the wheel?

Sure, that part is great --- it's just the parsing (or lack of it,
to be more accurate) that's an ugly hack.
        regards, tom lane


Re: plpgsql: RAISE

From
Bruce Momjian
Date:
Can I ask where we are on this?


> "Richard Huxton" <dev@archonet.com> writes:
> > Actually, I like the idea of using the SQL system to evaluate expressions -
> > why reinvent the wheel?
> 
> Sure, that part is great --- it's just the parsing (or lack of it,
> to be more accurate) that's an ugly hack.
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


Re: plpgsql: RAISE

From
Richard Huxton
Date:
Bruce Momjian wrote:
> 
> Can I ask where we are on this?

Sure - posted a follow up to the list a while ago. Subject was

"RAISE <level> <expr> <params>: state of play and request for advice"

Currently, this works:

CREATE FUNCTION foo_raise_loop(text) RETURNS text AS '
DECLARE   a ALIAS FOR $1;   i integer;   myrec RECORD;
BEGIN   i:=0;   FOR myrec IN SELECT * FROM colours LOOP       i:=i+1;       RAISE NOTICE a || '' : '' || '' colour % is
''|| myrec.c_name ||
 
''.'', i, myrec.c_id;   END LOOP;   RETURN ''done''::text;
END;' LANGUAGE 'plpgsql';

More details in the msg of a few days ago. Busy at the moment, probably
for the next week at least. If you'd like the patch against current CVS
let me know and I'll try and do it this weekend.

- Richard Huxton