Re: INS/UPD/DEL RETURNING for 8.2

Patch for core and PL/pgSQL to support the INSERT/UPDATE/DELETE RETURNING
syntax in 8.2

INSERT/UPDATE/DELETE seem to work fine in normal operation but there is an
error with DELETE RETURNING when used through PL/pgSQL.

Here's an example PL/pgSQL test:

CREATE SEQUENCE test_id_seq START 1 INCREMENT 1;

CREATE TABLE test_tbl (
test_id BIGINT NOT NULL
DEFAULT nextval('test_id_seq'),
test_name VARCHAR(64) NOT NULL,
PRIMARY KEY (test_id));

CREATE OR REPLACE FUNCTION test_func (test_nm VARCHAR)
RETURNS VOID AS $$
DECLARE
current_rec RECORD;
BEGIN
-- Test INSERT RETURNING
INSERT INTO test_tbl (test_name) VALUES (test_nm)
RETURNING * INTO current_rec;

RAISE NOTICE 'test_id is %', current_rec.test_id;
RAISE NOTICE 'test_name is %', current_rec.test_name;

-- Test UPDATE RETURNING
UPDATE test_tbl SET test_name = 'Uncle Bob'
WHERE test_id = current_rec.test_id
RETURNING * INTO current_rec;

RAISE NOTICE 'test_id is %', current_rec.test_id;
RAISE NOTICE 'test_name is %', current_rec.test_name;

-- Test DELETE RETURNING
DELETE FROM test_tbl WHERE test_id = current_rec.test_id
RETURNING * INTO current_rec;

-- This DOES NOT WORK
RAISE NOTICE 'test_id is %', current_rec.test_id;
RAISE NOTICE 'test_name is %', current_rec.test_name;

RETURN;
END;
$$ LANGUAGE plpgsql;

--
Jonah H. Harris, Database Internals Architect
EnterpriseDB Corporation
732.331.1324

On Thu, 2006-03-02 at 17:51 -0500, Jonah H. Harris wrote:
> Patch for core and PL/pgSQL to support the INSERT/UPDATE/DELETE
> RETURNING syntax in 8.2

I wonder if we should try to consistently treat an INSERT/UPDATE/DELETE
with a RETURNING clause like a SELECT with an equivalent target list.
For example, should it be possible to write:

FOR x in DELETE FROM t1 WHERE ... RETURNING t1.x LOOP
...
END LOOP;

Or open a cursor for a data-modifying statement with a RETURNING clause,
etc.

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> I wonder if we should try to consistently treat an INSERT/UPDATE/DELETE
> with a RETURNING clause like a SELECT with an equivalent target list.
> For example, should it be possible to write:

> FOR x in DELETE FROM t1 WHERE ... RETURNING t1.x LOOP

Seems like you'd want to get there eventually, if not in the first cut.

This might tie into something that was bothering me about Jonah's
first-cut patch, which was that he was introducing special cases into
places where it didn't seem real appropriate (like printtup.c). I
wonder if we should rejigger the representation of Query so that a
FOO-RETURNING command actually *is* a SELECT in some sense, so that
there's no need for special cases.

I'm a bit fuzzy about how this would work exactly --- you still need to
keep track of two targetlists it seems --- but it's worth thinking
about. I've had a bee in my bonnet for literally years about the fact
that INSERT/SELECT really needs two levels of targetlist, as does UNION.
Maybe if we thought a little bit larger we could clean up all of that
messiness at one stroke.

BTW, looking at the patch's test output reminds me that the appearance
of a resultset causes psql to suppress showing the command status.
I think this is reasonable and expected for SELECT, but I wonder whether
we shouldn't force it to appear when it's something else.

regards, tom lane

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> INSERT/UPDATE/DELETE seem to work fine in normal operation but there is an
> error with DELETE RETURNING when used through PL/pgSQL.

Probably other places too. I don't see any provision here for ensuring
that the variables used in the RETURNING list are actually computed by
the plan. This would be masked in the INSERT and non-join UPDATE cases
by the fact that the plan has to compute all columns of the target table
anyway ... but in a DELETE it'd be an issue.

I think set-returning functions in the RETURNING list might give you
some fits too ...

regards, tom lane

On 3/2/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> This might tie into something that was bothering me about Jonah's
> first-cut patch, which was that he was introducing special cases into
> places where it didn't seem real appropriate (like printtup.c). I
> wonder if we should rejigger the representation of Query so that a
> FOO-RETURNING command actually *is* a SELECT in some sense, so that
> there's no need for special cases.

I was thinking along the same lines. This is Omar's patch updated to
8.2but as I get to looking through it, there are a couple things that
could be
cleaned up. I paced around a bit today trying to theorize how this could be
done without a lot of changes and retaining the speed increase gained by not
performing two separate operations.

I'm a bit fuzzy about how this would work exactly --- you still need to
> keep track of two targetlists it seems --- but it's worth thinking
> about. I've had a bee in my bonnet for literally years about the fact
> that INSERT/SELECT really needs two levels of targetlist, as does UNION.
> Maybe if we thought a little bit larger we could clean up all of that
> messiness at one stroke.

I'm definitely open to looking into it. Any suggestions are always
welcome.

--
Jonah H. Harris, Database Internals Architect
EnterpriseDB Corporation
732.331.1324

On 3/2/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> > INSERT/UPDATE/DELETE seem to work fine in normal operation but there is
> an
> > error with DELETE RETURNING when used through PL/pgSQL.
>
> Probably other places too. I don't see any provision here for ensuring
> that the variables used in the RETURNING list are actually computed by
> the plan. This would be masked in the INSERT and non-join UPDATE cases
> by the fact that the plan has to compute all columns of the target table
> anyway ... but in a DELETE it'd be an issue.
>
> I think set-returning functions in the RETURNING list might give you
> some fits too ...

Yeah, I got to looking into the special tuple handling code in execUtils for
retrieving the old (deleted) tuple and there's something definitely getting
lost along the way in some cases.

--
Jonah H. Harris, Database Internals Architect
EnterpriseDB Corporation
732.331.1324

On 3/2/06, Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> wrote:
>
> On 3/2/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> > > INSERT/UPDATE/DELETE seem to work fine in normal operation but there
> > is an
> > > error with DELETE RETURNING when used through PL/pgSQL.
> >
> > Probably other places too. I don't see any provision here for ensuring
> > that the variables used in the RETURNING list are actually computed by
> > the plan. This would be masked in the INSERT and non-join UPDATE cases
> > by the fact that the plan has to compute all columns of the target table
> > anyway ... but in a DELETE it'd be an issue.
> >
> > I think set-returning functions in the RETURNING list might give you
> > some fits too ...
>
>
> Yeah, I got to looking into the special tuple handling code in execUtils
> for retrieving the old (deleted) tuple and there's something definitely
> getting lost along the way in some cases.
>

I've been stewing on this and haven't yet come up with anything. Have you
any thoughts on how we can accomplish this better?

--
Jonah H. Harris, Database Internals Architect
EnterpriseDB Corporation
732.331.1324

>
>I wonder if we should try to consistently treat an INSERT/UPDATE/DELETE
>with a RETURNING clause like a SELECT with an equivalent target list.
>For example, should it be possible to write:
>
>FOR x in DELETE FROM t1 WHERE ... RETURNING t1.x LOOP
> ...
>END LOOP;
>
>Or open a cursor for a data-modifying statement with a RETURNING clause,
>etc.
>
>-Neil

good idea
Pavel

_________________________________________________________________
Citite se osamele? Poznejte nekoho vyjmecneho diky Match.com.
http://www.msn.cz/

Jonah, where are we on this patch? Do you have a version ready for
review?

---------------------------------------------------------------------------

Jonah H. Harris wrote:
> On 3/2/06, Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> wrote:
> >
> > On 3/2/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > > "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> > > > INSERT/UPDATE/DELETE seem to work fine in normal operation but there
> > > is an
> > > > error with DELETE RETURNING when used through PL/pgSQL.
> > >
> > > Probably other places too. I don't see any provision here for ensuring
> > > that the variables used in the RETURNING list are actually computed by
> > > the plan. This would be masked in the INSERT and non-join UPDATE cases
> > > by the fact that the plan has to compute all columns of the target table
> > > anyway ... but in a DELETE it'd be an issue.
> > >
> > > I think set-returning functions in the RETURNING list might give you
> > > some fits too ...
> >
> >
> > Yeah, I got to looking into the special tuple handling code in execUtils
> > for retrieving the old (deleted) tuple and there's something definitely
> > getting lost along the way in some cases.
> >
>
> I've been stewing on this and haven't yet come up with anything. Have you
> any thoughts on how we can accomplish this better?
>
>
> --
> Jonah H. Harris, Database Internals Architect
> EnterpriseDB Corporation
> 732.331.1324

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On 3/2/06, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > For example, should it be possible to write:
> > FOR x in DELETE FROM t1 WHERE ... RETURNING t1.x LOOP
>
> Seems like you'd want to get there eventually, if not in the first cut.

I'd like to get this into the first release of RETURNING for 8.2.

> I wonder if we should rejigger the representation of Query so that a
> FOO-RETURNING command actually *is* a SELECT in some sense, so that
> there's no need for special cases.

I want to get rid of all the special case code and move in this
direction, that way we can make better use of code that already exists
and is well-tested.

> I'm a bit fuzzy about how this would work exactly --- you still need to
> keep track of two targetlists it seems --- but it's worth thinking
> about. I've had a bee in my bonnet for literally years about the fact
> that INSERT/SELECT really needs two levels of targetlist, as does UNION.
> Maybe if we thought a little bit larger we could clean up all of that
> messiness at one stroke.

Have you had any ideas on how to best accomplish this?

--
Jonah H. Harris, Database Internals Architect
EnterpriseDB Corporation
732.331.1324