Re: [PATCHES] plpgsql raise - parameters can be expressions
Neil Conway [EMAIL PROTECTED] writes: BTW, is there any value in a separate EXCEPTION type? ISTM that an exception is just a SQLSTATE, which is in turn just a string. A separate exception type does simplify the parsing of RAISE, but I wonder if it would be useful to be able to also allow specifying the SQLSTATE code as a string literal. It would save some typing, but I do not think we can make the proposed syntax work if we do it that way: RAISE level [ exception_name , ] format_string [ , parameters ] ; where level is one of the already-defined level keywords? How will you tell whether the string literal just after level is meant to be a SQLSTATE or a format? Maybe with some ad-hoc tests, but ugh ... regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] plpgsql raise - parameters can be expressions
Pavel Stehule [EMAIL PROTECTED] writes: Next problem, visibility custom exceptions. When I define exception variable I can't rethrown exceptions outside block when is defined. What is outside - some custom exception? I don't think this is an issue. A custom exception is really just a name for a SQLSTATE value, so you can throw it in any case. An outer block that does not know that name can only catch it as WHEN OTHERS, but so what? I would also expect that matching is by SQLSTATE, that is if I write DECLARE foo EXCEPTION = '12345'; ... RAISE ERROR foo, ...; then some outer block written as DECLARE bar EXCEPTION = '12345'; ... EXCEPTION WHEN bar THEN ... would catch this error. disadvantage - I have to define format string everywhere where I wont to raise exception. Why is that a disadvantage? You should not be able to throw an error without providing a useful message --- that's just basic good programming. From OOP view exception is object. But I need define more properties than one. SQLSTATE is only first, second message, level, meybe next I think we are better off defining exception names as SQLSTATEs and nothing else. That's essentially how we have done it in the backend code and it has worked well over a very large body of code. You are arguing for a less flexible definition on the basis of nothing more than a vague appeal to OOP principles. You have neither stated exactly which principles nor exactly why they dictate this choice, so I see nothing convincing in your argument. I think so we need more then one exception level. I can use user's exception for easy way of write to log. Well, can we get away with making the syntax be RAISE level [ exception_name , ] format_string [ , parameters ] ; where level is one of the already-defined level keywords? Normally I would think that this would be unacceptably ambiguous, but as long as the exception_name is constrained to be either a built-in or previously defined exception name, this is probably workable from a syntactic point of view. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plpgsql raise - parameters can be expressions
I would also expect that matching is by SQLSTATE, that is if I write DECLARE foo EXCEPTION = '12345'; ... RAISE ERROR foo, ...; then some outer block written as DECLARE bar EXCEPTION = '12345'; ... EXCEPTION WHEN bar THEN ... if we accept exception is like variable, then there is rules for variables. I don't see problem there. EXCEPTION WHEN bar is equivalent of EXCEPTION SQLSTATE = 12345 THEN where I see bar, I can see bar. But isn't possible two exception handlers in one block with one SQLSTATE. would catch this error. disadvantage - I have to define format string everywhere where I wont to raise exception. Why is that a disadvantage? You should not be able to throw an error without providing a useful message --- that's just basic good programming. it's ok. Exception without error message is wrong. One type of exception with different error messages (parametrized message is ok) is wrong too. But it's my personal opinion. Maybe one message string is not sufficient, better is message string and hint string (like now). From OOP view exception is object. But I need define more properties than one. SQLSTATE is only first, second message, level, meybe next I think we are better off defining exception names as SQLSTATEs and nothing else. That's essentially how we have done it in the backend code and it has worked well over a very large body of code. You are arguing for a less flexible definition on the basis of nothing more than a vague appeal to OOP principles. You have neither stated exactly which principles nor exactly why they dictate this choice, so I see nothing convincing in your argument. I have less. All is only my ideas. I don't wont PLPGSQL like full OOP language. I speak only about possible ways now. I see usefull global definition of exception on package (or schema, database) level - like others db objects (sequences). Packages not exists and all is in future. On procedural level I have to agree with you. Your syntax don't exude my syntax. If I choise level, format_string,.. in raise stmt, then are used this params. If not, then are used default parames (in future). I think so we need more then one exception level. I can use user's exception for easy way of write to log. Well, can we get away with making the syntax be RAISE level [ exception_name , ] format_string [ , parameters ] ; I agree. I unlike big steps. About level: I think so already defined levels are good. I have idea about somethink between levels NOTICE and EXCEPTION. I can't catch NOTICE and I have to catch EXCEPTION, maybe RAISE EVENT. I can catch it, if I wont. And this level don't rollback transaction, and should be return back from exception handler. It's more like calling subrutine. Can be usefull. Question: What do you think about I specify minimal level of event in log. But when I have user's exception I can specify list of user's exception for log. like log_level NOTICE log_exceptions myexception1, myexception2, ... where level is one of the already-defined level keywords? Normally I would think that this would be unacceptably ambiguous, but as long as the exception_name is constrained to be either a built-in or previously defined exception name, this is probably workable from a syntactic point of view. Pavel Stehule ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plpgsql raise - parameters can be expressions
Tom Lane wrote: That doesn't bother me either, seeing that an undefined variable isn't detected at compile time anywhere else. However, fixing the SQLSTATE tests by removing them doesn't seem like a great solution ... Yeah, true, I can just invoke the function to trigger the undefined variable error. Actually, the reason I didn't do something about RAISE in 8.0 was that I thought we should reconsider the whole design of the statement The ensuing discussion on this sounds good to me; should I apply Pavel's RAISE patch now, or wait for the subsequent work on specifying a particular SQLSTATE? -Neil ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plpgsql raise - parameters can be expressions
Neil Conway [EMAIL PROTECTED] writes: Tom Lane wrote: Actually, the reason I didn't do something about RAISE in 8.0 was that I thought we should reconsider the whole design of the statement The ensuing discussion on this sounds good to me; should I apply Pavel's RAISE patch now, or wait for the subsequent work on specifying a particular SQLSTATE? The patch seems to me to be OK as far as it goes. I brought up the other points only because I wanted to be sure that it wouldn't be inconsistent with the next step; but it seems we're pretty well agreed that we aren't going to do anything that would break this. So I have no problem with applying as-is, rather than waiting for an all-inclusive patch. But you had mentioned wanting to look at reducing overhead by using exec_eval_expr(); were you intending to do that before committing? As far as the subsequent discussion itself goes, Pavel and I seem to be pretty unsuccessful at convincing each other of our respective visions of what an exception ought to be. Any opinions? Should we be taking this thread to -hackers for a wider audience? regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] plpgsql raise - parameters can be expressions
Tom Lane wrote: But you had mentioned wanting to look at reducing overhead by using exec_eval_expr(); were you intending to do that before committing? I'm a bit busy with other matters at the moment, so I'll probably look at it later. One question is whether we should make use of exec_eval_expr() by representing the RAISE parameter list as a list of expressions (each of which would likely be simple enough to evaluate via ExecEvalExpr()), or whether we want to extend exec_eval_expr() to handle expressions that yield multiple attributes. -Neil ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] plpgsql raise - parameters can be expressions
On Mon, 13 Jun 2005, Michael Glaesemann wrote: On Jun 13, 2005, at 2:07 PM, Pavel Stehule wrote: I did trivial patch which eliminate limit raise command. Using expressions instead of variables are a little bit expensive, but little. I'm right in thinking this essentially does the same thing as first assigning the value of the expression to a variable and then using the variable in the RAISE statement, correct? This patch just eliminates the step of assigning the value of the expression to the variable? If so, I'd expect the cost to be in line with the cost of explicitly assigning to a variable. true, total cost is less Pavel ---(end of broadcast)--- TIP 8: explain analyze is your friend
