Thread: I must be blind...
'Afternoon folks, I think I'm going blind I just can't spot what I've done wrong. Can someone have a quick glance at this function, and relevent table definitions, and tell me what I've got wrong please? The error message I'm getting when I try to use it with: SELECT new_transaction_fn(9, 444, 4, 'B', now(), 'C'); is: NOTICE: Error occurred while executing PL/pgSQL function new_transaction_fn NOTICE: line 11 at assignment ERROR: parser: parse error at or near "SELECT" (The select works and returns one row as I expect it to btw) -- -- Tables -- CREATE TABLE orders ( id INTEGER NOT NULL DEFAULT nextval('order_seq') PRIMARY KEY, type INTEGER REFERENCES order_type(id), instrument INTEGER REFERENCES instrument(id), time TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), market_price FLOAT8, price FLOAT8, quantity INTEGER, direction CHAR(1) CHECK(direction = 'B' OR direction = 'S') ) WITHOUT OIDS; CREATE TABLE transaction ( id INTEGER NOT NULL DEFAULT nextval('transaction_seq') PRIMARY KEY, order_id INTEGER REFERENCES orders(id), time TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), price FLOAT8, quantity INTEGER, status CHAR(1) CHECK(status = 'c' OR status = 'C') ) WITHOUT OIDS; -- -- Function -- CREATE OR REPLACE FUNCTION new_transaction_fn ( integer,float8,integer,char(1),timestamp,char(1) ) RETURNS boolean AS ' DECLARE ordid ALIAS FOR $1; price ALIAS FOR $2; quantity ALIAS FOR $3; dirn ALIAS FOR $4; time ALIAS FOR $5; status ALIAS FOR $6; BEGIN -- check against order PERFORM SELECT 1 FROM orders WHERE id = ordid AND direction = dirn; IF NOT FOUND THEN RAISE EXCEPTION ''No order matching % / % found'', ordid, dirn; END IF; INSERT INTO transaction VALUES ( nextval(''transaction_seq''), ordid, COALESCE(time, now()), price, quantity, COALESCE(status, ''C'') ); RETURN TRUE; END; ' LANGUAGE 'plpgsql'; -- -- -- Thanks, -- Nigel J. Andrews Director --- Logictree Systems Limited Computer Consultants
On Fri, 14 Jun 2002, Nigel J. Andrews wrote: > > [snip] > The error message I'm getting when I try to use it with: > > SELECT new_transaction_fn(9, 444, 4, 'B', now(), 'C'); > > is: > > NOTICE: Error occurred while executing PL/pgSQL function new_transaction_fn > NOTICE: line 11 at assignment > ERROR: parser: parse error at or near "SELECT" > > [snip] Ah, it's the perform. I was wondering about the validity of a FOUND test after discarding the results of a query. If I just write the result into a dummy variable without using PERFORM it progresses to a different error (but that's down to a check constraint I haven't changed after changing my mind about the values in the field). So I wasn't blind, just dim. -- Nigel J. Andrews Director --- Logictree Systems Limited Computer Consultants
Nigel J. Andrews wrote: <snip> > dirn ALIAS FOR $4; > time ALIAS FOR $5; > status ALIAS FOR $6; > BEGIN > -- check against order > PERFORM > SELECT 1 > FROM orders > WHERE > id = ordid > AND > direction = dirn; > IF NOT FOUND THEN </snip> I don't think you can use PERFORM like that. Try: <snip> dirn ALIAS FOR $4; time ALIAS FOR $5; status ALIAS FOR $6; buf INT; BEGIN -- check against order SELECT 1 INTO buf FROM orders WHERE id = ordid AND direction = dirn; IF NOT FOUND THEN </snip> HTH, Joe
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes: > The error message I'm getting when I try to use it with: > SELECT new_transaction_fn(9, 444, 4, 'B', now(), 'C'); > is: > NOTICE: Error occurred while executing PL/pgSQL function new_transaction_fn > NOTICE: line 11 at assignment > ERROR: parser: parse error at or near "SELECT" I'm not seeing the problem either. Try turning on debug_print_query and running the function; then look in the postmaster log to see exactly what got fed down to the main SQL engine by plpgsql. This might shed some light. regards, tom lane
Joe Conway <mail@joeconway.com> writes: > I don't think you can use PERFORM like that. Try: Actually I believe he can; after looking at the manual I realized that the problem is that PERFORM is syntactically a substitute for SELECT. In other words he needed to write PERFORM 1 FROM orders ... not PERFORM SELECT 1 FROM orders ... regards, tom lane
On Fri, 14 Jun 2002, Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: > > I don't think you can use PERFORM like that. Try: > > Actually I believe he can; after looking at the manual I realized that > the problem is that PERFORM is syntactically a substitute for SELECT. > In other words he needed to write > > PERFORM 1 FROM orders ... > not > PERFORM SELECT 1 FROM orders ... Thanks Tom, I didn't get that from the manual at all. I think I should go reread it. -- Nigel J. Andrews Director --- Logictree Systems Limited Computer Consultants
On Fri, 14 Jun 2002, Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: > > I don't think you can use PERFORM like that. Try: > > Actually I believe he can; after looking at the manual I realized that > the problem is that PERFORM is syntactically a substitute for SELECT. > In other words he needed to write > > PERFORM 1 FROM orders ... > not > PERFORM SELECT 1 FROM orders ... Yes, indeed if one reads what is there rather than reading things that aren't it does say that PERFORM substitutes for SELECT syntactically. However, because PERFORM discards the results of a query it is only useful for side effects of the query. My usage of it was wrong since I wasn't using it for side effects merely for determining the existance of a result without having to store that result since it wasn't required. Therefore, with the correct syntax of PERFORM <query> my function doesn't generate an 'unprogrammed' error but the test of FOUND always fails, i.e. result is NOT FOUND. Therefore SELECT INTO dummy ... is still the correct thing for me to be doing. I just thought I'd clear that up in case anyone was wondering, and yes, I have tested it. -- Nigel J. Andrews Director --- Logictree Systems Limited Computer Consultants
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes: > However, because PERFORM discards the results of a query it is only > useful for side effects of the query. My usage of it was wrong since I > wasn't using it for side effects merely for determining the existance > of a result without having to store that result since it wasn't > required. Therefore, with the correct syntax of PERFORM <query> my > function doesn't generate an 'unprogrammed' error but the test of > FOUND always fails, i.e. result is NOT FOUND. Therefore SELECT INTO > dummy ... is still the correct thing for me to be doing. Okay. I guess the next question is whether PERFORM *should* be setting FOUND. Seems like it might be a reasonable thing to do. Does PERFORM exist in Oracle's plsql? If so, what does it do? regards, tom lane
Tom Lane dijo: > "Nigel J. Andrews" <nandrews@investsystems.co.uk> writes: > > However, because PERFORM discards the results of a query it is only > > useful for side effects of the query. > Okay. I guess the next question is whether PERFORM *should* be setting > FOUND. Seems like it might be a reasonable thing to do. Well, actually FOUND _is_ a side effect of PERFORM, IMHO. I also tried to do the very same thing, and also had to use the dummy variable, which seems like a waste to me. I do not know anything about Oracle's PERFORM, though a quick search on Google shows nothing relevant. -- Alvaro Herrera (<alvherre[a]atentus.com>) "La conclusion que podemos sacar de esos estudios es que no podemos sacar ninguna conclusion de ellos" (Tanenbaum)
On Fri, 14 Jun 2002, Alvaro Herrera wrote: > Tom Lane dijo: > > > "Nigel J. Andrews" <nandrews@investsystems.co.uk> writes: > > > However, because PERFORM discards the results of a query it is only > > > useful for side effects of the query. > > > Okay. I guess the next question is whether PERFORM *should* be setting > > FOUND. Seems like it might be a reasonable thing to do. > > Well, actually FOUND _is_ a side effect of PERFORM, IMHO. I also tried > to do the very same thing, and also had to use the dummy variable, which > seems like a waste to me. > > I do not know anything about Oracle's PERFORM, though a quick search on > Google shows nothing relevant. I know nothing of Oracle's use of PERFORM either. Indeed I have looked in 4 Oracle books 'Oracle 8i The Complete Reference', 'Oracle8i DBA Bible', 'Oracle PL/SQL Language Pocket Reference' and one on PL/SQL Builtins (on the off chance), and couldn't find any reference to PERFORM. I even scanned, by eye, every page of the PL/SQL reference and saw nothing. On that basis I've included a patch that sets FOUND to true if a PERFORM <query> 'processes' a row. From looking at other routines in pl_exec.c I believe that I have used the correct test. As FOUND isn't testing as true after a PERFORM at the moment I also presume there is no need for an explicit set to false. I have tested this in 7.3dev with the regression tests and the case that caused me to come across this situation and no errors occured. The patch is at the bottom of this message. I don't know if this should be applied though. There seems to be one vote for it, at least, but there is a question over what other systems do in this situation. -- Nigel J. Andrews Director --- Logictree Systems Limited Computer Consultants Index: src/pl/plpgsql/src/pl_exec.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v retrieving revision 1.55 diff -c -r1.55 pl_exec.c *** src/pl/plpgsql/src/pl_exec.c 2002/03/25 07:41:10 1.55 --- src/pl/plpgsql/src/pl_exec.c 2002/06/15 15:10:38 *************** *** 981,989 **** if (expr->plan == NULL) exec_prepare_plan(estate, expr); ! rc = exec_run_select(estate, expr, 0, NULL); if (rc != SPI_OK_SELECT) elog(ERROR, "query \"%s\" didn't return data", expr->query); exec_eval_cleanup(estate); } --- 981,992 ---- if (expr->plan == NULL) exec_prepare_plan(estate, expr); ! rc = exec_run_select(estate, expr, 1, NULL); if (rc != SPI_OK_SELECT) elog(ERROR, "query \"%s\" didn't return data", expr->query); + + if (estate->eval_processed != 0) + exec_set_found(estate, true); exec_eval_cleanup(estate); }
"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes: > + if (estate->eval_processed != 0) > + exec_set_found(estate, true); To be actually useful the command would have to set FOUND to either true or false depending on whether it computed a row or not. So the correct patch would be more like + exec_set_found(estate, (estate->eval_processed != 0)); Also, changing the parameter to exec_run_select as you did is wrong. A multi-row query should be allowed to run to completion, I'd think. As for whether to apply it or not --- the change seems reasonable if we were working in a vacuum. But I don't believe we invented PERFORM out of whole cloth; surely there are other systems that we need to consider compatibility with. regards, tom lane
"Nigel J. Andrews" wrote: > > On Fri, 14 Jun 2002, Alvaro Herrera wrote: > > > Tom Lane dijo: > > > > > "Nigel J. Andrews" <nandrews@investsystems.co.uk> writes: > > > > However, because PERFORM discards the results of a query it is only > > > > useful for side effects of the query. > > > > > Okay. I guess the next question is whether PERFORM *should* be setting > > > FOUND. Seems like it might be a reasonable thing to do. > > > > Well, actually FOUND _is_ a side effect of PERFORM, IMHO. I also tried > > to do the very same thing, and also had to use the dummy variable, which > > seems like a waste to me. > > > > I do not know anything about Oracle's PERFORM, though a quick search on > > Google shows nothing relevant. > > I know nothing of Oracle's use of PERFORM either. Indeed I have looked in 4 > Oracle books 'Oracle 8i The Complete Reference', 'Oracle8i DBA Bible', 'Oracle > PL/SQL Language Pocket Reference' and one on PL/SQL Builtins (on the off > chance), and couldn't find any reference to PERFORM. I even scanned, by eye, > every page of the PL/SQL reference and saw nothing. Perform has nothing to do with ORACLE. It was added because people tried to call other "procedures" and didn't want any result back. Using SELECT function(); didn't look right, so we made it PERFORM function(); Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Jan Wieck <JanWieck@Yahoo.com> writes: > Perform has nothing to do with ORACLE. It was added because people tried > to call other "procedures" and didn't want any result back. Well, in that case we can do what we want with it. Does anyone object to making it set FOUND? regards, tom lane
> Jan Wieck <JanWieck@Yahoo.com> writes: >> Perform has nothing to do with ORACLE. It was added because people tried >> to call other "procedures" and didn't want any result back. > Well, in that case we can do what we want with it. > Does anyone object to making it set FOUND? Given the lack of objection, I have committed the attached patch for 7.3, along with a suitable documentation update. regards, tom lane *** src/pl/plpgsql/src/pl_exec.c.orig Mon Mar 25 02:41:10 2002 --- src/pl/plpgsql/src/pl_exec.c Mon Jun 24 18:23:11 2002 *************** *** 969,977 **** else { /* ! * PERFORM: evaluate query and discard result. This cannot share ! * code with the assignment case since we do not wish to ! * constraint the discarded result to be only one row/column. */ int rc; --- 969,979 ---- else { /* ! * PERFORM: evaluate query and discard result (but set FOUND ! * depending on whether at least one row was returned). ! * ! * This cannot share code with the assignment case since we do not ! * wish to constrain the discarded result to be only one row/column. */ int rc; *************** *** 984,989 **** --- 986,993 ---- rc = exec_run_select(estate, expr, 0, NULL); if (rc != SPI_OK_SELECT) elog(ERROR, "query \"%s\" didn't return data", expr->query); + + exec_set_found(estate, (estate->eval_processed != 0)); exec_eval_cleanup(estate); }