Thread: INS/UPD/DEL RETURNING for 8.2

INS/UPD/DEL RETURNING for 8.2

From

"Jonah H. Harris"

Date:

02 March 2006, 21:51:23

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

Attachment

pg82-iudret.patch

Re: INS/UPD/DEL RETURNING for 8.2

From

Neil Conway

Date:

02 March 2006, 22:44:40

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

Re: INS/UPD/DEL RETURNING for 8.2

From

Tom Lane

Date:

03 March 2006, 00:21:04

Neil Conway <neilc@samurai.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

Re: INS/UPD/DEL RETURNING for 8.2

From

Tom Lane

Date:

03 March 2006, 00:33:41

"Jonah H. Harris" <jonah.harris@gmail.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

Re: INS/UPD/DEL RETURNING for 8.2

From

"Jonah H. Harris"

Date:

03 March 2006, 00:47:03

On 3/2/06, Tom Lane <tgl@sss.pgh.pa.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.2 but 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

Re: INS/UPD/DEL RETURNING for 8.2

From

"Jonah H. Harris"

Date:

03 March 2006, 00:48:22

On 3/2/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Jonah H. Harris" <jonah.harris@gmail.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

Re: INS/UPD/DEL RETURNING for 8.2

From

"Jonah H. Harris"

Date:

03 March 2006, 19:01:56

On 3/2/06, Jonah H. Harris <jonah.harris@gmail.com> wrote:

On 3/2/06, Tom Lane <tgl@sss.pgh.pa.us > wrote:
"Jonah H. Harris" <jonah.harris@gmail.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

Re: INS/UPD/DEL RETURNING for 8.2

From

"Pavel Stehule"

Date:

05 March 2006, 07:25:53

>
>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/

Re: INS/UPD/DEL RETURNING for 8.2

From

Bruce Momjian

Date:

27 April 2006, 02:45:50

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.harris@gmail.com> wrote:
> >
> > On 3/2/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > > "Jonah H. Harris" <jonah.harris@gmail.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. +

Re: INS/UPD/DEL RETURNING for 8.2

From

"Jonah H. Harris"

Date:

04 May 2006, 16:24:14

On 3/2/06, Tom Lane <tgl@sss.pgh.pa.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