

'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


Fri, 14 Jun 2002 17:49:08 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
I must be blind...

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


Fri, 14 Jun 2002 18:23:23 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
Re: I must be blind...
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


Fri, 14 Jun 2002 10:47:40 -0700
Joe Conway <mail@joeconway.com>
Re: I must be blind...
"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

Fri, 14 Jun 2002 14:12:13 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: I must be blind... 
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

Fri, 14 Jun 2002 17:35:58 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: I must be blind... 

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


Fri, 14 Jun 2002 23:09:49 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
Re: I must be blind...

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


Fri, 14 Jun 2002 23:22:35 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
Re: I must be blind...
"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

Fri, 14 Jun 2002 18:25:53 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: I must be blind... 
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)


Fri, 14 Jun 2002 20:12:10 -0400 (CLT)
Alvaro Herrera <alvherre@atentus.com>
Re: I must be blind... 

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);
  	}


Sat, 15 Jun 2002 16:28:19 +0100 (BST)
"Nigel J. Andrews" <nandrews@investsystems.co.uk>
PERFORM effects FOUND patch (Was: I must be blind...)
"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

Sat, 15 Jun 2002 12:28:16 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: PERFORM effects FOUND patch (Was: I must be blind...) 
"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 #

Mon, 17 Jun 2002 09:38:36 -0400
Jan Wieck <JanWieck@Yahoo.com>
Re: [HACKERS] PERFORM effects FOUND patch (Was: I must be 
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

Tue, 18 Jun 2002 10:34:04 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] PERFORM effects FOUND patch (Was: I must be 
> 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);
  	}



Mon, 24 Jun 2002 19:14:25 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] PERFORM effects FOUND patch (Was: I must be 