Thread: PERFORM effects FOUND patch (Was: [GENERAL] 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);
}
"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);
}