insert/update/delete returning and rules

Started by Jaime Casanovaover 19 years ago10 messages
#1Jaime Casanova
systemguards@gmail.com

Hi,

I'm doing some tests of Bernd's updatable views patch and found
something interesting about the RETURNING behavior

testing_uv=# create table bar (field1 integer);
CREATE TABLE
testing_uv=# create view v_bar as select * from bar;
CREATE VIEW

the rules are created as:

"_DELETE" AS
ON DELETE TO v_bar DO INSTEAD DELETE FROM ONLY bar
WHERE
CASE
WHEN old.field1 IS NOT NULL THEN old.field1 = bar.field1
ELSE bar.field1 IS NULL
END

"_INSERT" AS
ON INSERT TO v_bar DO INSTEAD INSERT INTO bar (field1)
VALUES (new.field1)

"_UPDATE" AS
ON UPDATE TO v_bar DO INSTEAD UPDATE ONLY bar SET field1 = new.field1
WHERE
CASE
WHEN old.field1 IS NOT NULL THEN old.field1 = bar.field1
ELSE bar.field1 IS NULL
END

Now, if i insert directly into the table i get:

testing_uv=# insert into bar values (1), (2) returning *;
field1
--------
1
2
(2 rows)

INSERT 0 2

but if i insert using the rules the returning clause is ignored

testing_uv=# insert into v_bar values (3), (4) returning *;
INSERT 0 2

any comments?

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jaime Casanova (#1)
Re: insert/update/delete returning and rules

"Jaime Casanova" <systemguards@gmail.com> writes:

I'm doing some tests of Bernd's updatable views patch and found
something interesting about the RETURNING behavior
...
but if i insert using the rules the returning clause is ignored
testing_uv=# insert into v_bar values (3), (4) returning *;
INSERT 0 2

What are you testing exactly? I think this recent fix might be
relevant:
http://archives.postgresql.org/pgsql-committers/2006-08/msg00299.php

regards, tom lane

#3Jaime Casanova
systemguards@gmail.com
In reply to: Tom Lane (#2)
Re: insert/update/delete returning and rules

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

"Jaime Casanova" <systemguards@gmail.com> writes:

I'm doing some tests of Bernd's updatable views patch and found
something interesting about the RETURNING behavior
...
but if i insert using the rules the returning clause is ignored
testing_uv=# insert into v_bar values (3), (4) returning *;
INSERT 0 2

What are you testing exactly? I think this recent fix might be
relevant:
http://archives.postgresql.org/pgsql-committers/2006-08/msg00299.php

i have tested again against current HEAD... what i do is to create a
table and then a view against that table... then the rules... you can
use the exact case i posted earlier...

then the insert into view (view + rules defined on it) returning will
not have the same behavior that insert into table...

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jaime Casanova (#3)
Re: insert/update/delete returning and rules

"Jaime Casanova" <systemguards@gmail.com> writes:

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

What are you testing exactly? I think this recent fix might be
relevant:
http://archives.postgresql.org/pgsql-committers/2006-08/msg00299.php

i have tested again against current HEAD... what i do is to create a
table and then a view against that table... then the rules... you can
use the exact case i posted earlier...

Oh, I'm thinking about the wrong problem. What you've got is INSERT
RETURNING on a view, and that's being trapped by an ON INSERT DO INSTEAD
rule, which rewrites it into an INSERT. Without RETURNING.

Right offhand this seems like a can of worms. I think it would probably
be a really bad idea for the rewriter to try to automatically transpose
the RETURNING clause into the rewritten query. In simple cases it might
be able to get it right, but in complicated cases I see no hope.

I'm tempted to suggest that the RETURNING commands might need to be
separate rule events, and that to support this you'd need to write
an additional rule:

CREATE RULE r1 AS ON INSERT RETURNING TO myview DO INSTEAD
INSERT ... RETURNING ...

where the RETURNING clause in the rule body would be expected to produce
exactly the set of columns of the view, and then the rewriter could hack
that up as input to whatever RETURNING list was given in the source
query.

But even this seems like it would fail in complicated cases. What if
the view is a join, and your ON INSERT rule inserts into two different
underlying tables in two commands? If you need fields from both tables
to generate a full RETURNING list then there's no apparent way to make
it work.

Ugh. Any ideas out there?

regards, tom lane

#5Jonah H. Harris
jonah.harris@gmail.com
In reply to: Tom Lane (#4)
Re: insert/update/delete returning and rules

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

But even this seems like it would fail in complicated cases. What if
the view is a join, and your ON INSERT rule inserts into two different
underlying tables in two commands? If you need fields from both tables
to generate a full RETURNING list then there's no apparent way to make
it work.

As both methods have the same issue when dealing with complicated
(multi-table) inserts, updates, or deletes... I'm pondering how to
best combine the tuples in such a way as to generate a full RETURNING
list.

Maybe it's the flu talking, but is there some way to hack the
RETURNINGs into a subselect-like form such that we could combine the
multiple values returned from say 2 insert statements into a complete
RETURNING list in a single tuple?

--
Jonah H. Harris, Software Architect | phone: 732.331.1300
EnterpriseDB Corporation | fax: 732.331.1301
33 Wood Ave S, 2nd Floor | jharris@enterprisedb.com
Iselin, New Jersey 08830 | http://www.enterprisedb.com/

#6Jens-Wolfhard Schicke
j.schicke@asco.de
In reply to: Tom Lane (#4)
Re: insert/update/delete returning and rules

--On Dienstag, August 15, 2006 16:33:27 -0400 Tom Lane <tgl@sss.pgh.pa.us>
wrote:

I'm tempted to suggest that the RETURNING commands might need to be
separate rule events, and that to support this you'd need to write
an additional rule:

CREATE RULE r1 AS ON INSERT RETURNING TO myview DO INSTEAD
INSERT ... RETURNING ...
...

But even this seems like it would fail in complicated cases. What if
the view is a join, and your ON INSERT rule inserts into two different
underlying tables in two commands? If you need fields from both tables
to generate a full RETURNING list then there's no apparent way to make
it work.

Ugh. Any ideas out there?

CREATE RULE r1 AS ON INSERT RETURNING TO myview DO INSTEAD
INSERT ... INTO tbl_1;
INSERT ... INTO tbl_2;
RETURNING SELECT .... FROM tbl_1, tbl_2 WHERE ...;

Just what crossed my mind first, no idea whether this is implementable or
realistic or whatever.

Mit freundlichem Gruß
Jens Schicke
--
Jens Schicke j.schicke@asco.de
asco GmbH http://www.asco.de
Mittelweg 7 Tel 0531/3906-127
38106 Braunschweig Fax 0531/3906-400

#7Jaime Casanova
systemguards@gmail.com
In reply to: Tom Lane (#4)
Re: insert/update/delete returning and rules

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

I'm tempted to suggest that the RETURNING commands might need to be
separate rule events, and that to support this you'd need to write
an additional rule:

CREATE RULE r1 AS ON INSERT RETURNING TO myview DO INSTEAD
INSERT ... RETURNING ...

This is something for 8.3?

--
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."
Richard Cook

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jaime Casanova (#7)
Re: insert/update/delete returning and rules

"Jaime Casanova" <systemguards@gmail.com> writes:

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

I'm tempted to suggest that the RETURNING commands might need to be
separate rule events, and that to support this you'd need to write
an additional rule:

CREATE RULE r1 AS ON INSERT RETURNING TO myview DO INSTEAD
INSERT ... RETURNING ...

This is something for 8.3?

Well, if we put it off till 8.3 we are going to have to write something
pretty lame in the documentation about views not working with RETURNING.

After some further thought, I think we could make it work if we treat
XXX RETURNING as a distinct rule event type and make the following
restrictions (which are exactly analogous to the restrictions for ON
SELECT rules) for ON XXX RETURNING rules:

* there can be only one ON XXX RETURNING rule per relation;

* it has to be an unconditional DO INSTEAD rule;

* it has to have a single action that is the same type of operation
it's replacing (or maybe we could allow any RETURNING command?);

* the RETURNING list has to match the column datatypes of the view.

Perhaps later we could support more stuff, but this particular case
would cover simple needs and it doesn't seem like something we'd
regret supporting later. The main thing we'd be setting in stone
is that the RETURNING commands require a different rule type, which
is a bit tedious but I don't really see a good way around it.
(Hopefully the updatable-views patch will soon save people from
having to write all these rules out by hand, anyway.)

I don't have a patch yet, but preliminary experimentation suggests
that the rewriter will Just Work, and all we'll need is straightforward
boilerplate code to support the additional possible values of
pg_rewrite.ev_type --- so probably less than a day's work.

Thoughts, objections?

regards, tom lane

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#8)
Re: insert/update/delete returning and rules

I wrote:

After some further thought, I think we could make it work if we treat
XXX RETURNING as a distinct rule event type and make the following
restrictions (which are exactly analogous to the restrictions for ON
SELECT rules) for ON XXX RETURNING rules:

After working on this for a bit, I realized that there's a serious,
probably fatal objection to this approach: it's arguably a security
hole. Suppose that you have a regular table on which you've defined
rules that you consider security-critical --- maybe an ON INSERT DO ALSO
that logs the action in a log table, for example. Now you migrate your
database to 8.2. If we treat INSERT RETURNING as a separate rule event
type, then any DB user can bypass your security-critical rules simply
by using INSERT RETURNING instead of INSERT. Yeah, you can fix that by
adding more rules, but it's not comfy-making to think that DB schemas
will be insecure as soon as they are ported to 8.2 until they are fixed.
In any case this thought blows out of the water the assumption that we
can disallow auxiliary rules for RETURNING events --- on a plain table,
that's an important feature to have.

So here's my Plan B: the set of rule event types stays the same,
and we give the rewriter a little bit of smarts about how to handle
RETURNING, while still putting the burden on the rule author to say
exactly what to return. Specifically, I suggest:

* A rule can have a command with a RETURNING clause only if it's an
unconditional DO INSTEAD rule, and there can be only one RETURNING
clause among a table's rules for a particular event type. The clause
must match the datatypes of the relation's columns.

* When rewriting a query that does not have a RETURNING clause, the
rewriter simply throws away any RETURNING clause in the rule.

* When rewriting a query that does have a RETURNING clause, the rewriter
rewrites the rule's RETURNING clause to generate the data required by
the query RETURNING clause (same transformation as we do on a view
SELECT targetlist). If there's no RETURNING in the rules, throw an
error.

With this approach, you still have to update your rules if you want
to support RETURNING on your views --- but if you don't update them,
you don't have a security hole. Basically the standard setup for an
updatable view would use
"ON INSERT DO INSTEAD INSERT INTO ... RETURNING ..."
where today you don't write any RETURNING.

Again, this is something we might want to generalize later, but it
seems to be a reasonable basic capability.

Thoughts? Have I missed something (again)?

regards, tom lane

#10Zeugswetter Andreas DCP SD
ZeugswetterA@spardat.at
In reply to: Tom Lane (#9)
Re: insert/update/delete returning and rules

With this approach, you still have to update your rules if
you want to support RETURNING on your views --- but if you
don't update them, you don't have a security hole. Basically
the standard setup for an updatable view would use
"ON INSERT DO INSTEAD INSERT INTO ... RETURNING ..."
where today you don't write any RETURNING.

I like that approach. And if the sections allow CASE WHEN
it should be possible to cover all use cases efficiently.

Andreas