Thread: I must be blind...

I must be blind...

From
"Nigel J. Andrews"
Date:

'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


Re: I must be blind...

From
"Nigel J. Andrews"
Date:
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


Re: I must be blind...

From
Joe Conway
Date:
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


Re: I must be blind...

From
Tom Lane
Date:
"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

Re: I must be blind...

From
Tom Lane
Date:
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

Re: I must be blind...

From
"Nigel J. Andrews"
Date:
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


Re: I must be blind...

From
"Nigel J. Andrews"
Date:
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


Re: I must be blind...

From
Tom Lane
Date:
"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

Re: I must be blind...

From
Alvaro Herrera
Date:
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)


PERFORM effects FOUND patch (Was: I must be blind...)

From
"Nigel J. Andrews"
Date:
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);
      }


Re: PERFORM effects FOUND patch (Was: I must be blind...)

From
Tom Lane
Date:
"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

Re: [HACKERS] PERFORM effects FOUND patch (Was: I must be

From
Jan Wieck
Date:
"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 #

Re: [HACKERS] PERFORM effects FOUND patch (Was: I must be

From
Tom Lane
Date:
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

Re: [HACKERS] PERFORM effects FOUND patch (Was: I must be

From
Tom Lane
Date:
> 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);
      }