Re: proposal and patch : support INSERT INTO...RETURNING with partitioned table using rule

-----------------------------------
Problem I'm trying to solve:
    For partitioned tables,  make it possible to use RETURNING clause on INSERT INTO
           together with DO INSTEAD rule

[  Note  -  wherever I say INSERT I also mean UPDATE and DELETE ]

-----------------------------------
Current behaviour :

    An INSERT which has a RETURNING clause and which is to be rewritten based on
    a rule will be accepted if the rule is an "unconditional DO INSTEAD".
    In general I believe "unconditional" means "no WHERE clause", but in practice
    if the rule is of the form
       CREATE RULE insert_part_history as ON INSERT to history \
         DO INSTEAD SELECT history_insert_partitioned(NEW) returning NEW.id
    this is treated as conditional and the query is rejected.   

    Testcase:
    A table T is partitioned and has two or more columns,  one of which
    is an id column declared as
         id bigint DEFAULT nextval('history_id_seq'::regclass) NOT NULL
    and the application issues
      "INSERT into history  (column-list which excludes id)  values (....) RETURNING id"

    I can get the re-direction of the INSERT *without* RETURNING to work using
    either trigger or rule, in which the trigger/rule invokes a procedure, but
    whichever way I do it, I could not get this RETURNING clause to work.

    For a trigger,
      the INSERT ... RETURNING was accepted but returned no rows, (as I would
      expect), and for the RULE, the INSERT ... RETURNING was rejected with :
 
      ERROR:  cannot perform INSERT RETURNING on relation "history"
      HINT:  You need an unconditional ON INSERT DO INSTEAD rule with a RETURNING clause.

      but this hint was not much help,  since :

   For a rule,
     CREATE RULE insert_part_history as ON INSERT to history \
         DO INSTEAD SELECT history_insert_partitioned(NEW) returning NEW.id
    ERROR:  syntax error at or near "returning"
    LINE 1: ...DO INSTEAD SELECT history_insert_partitioned(NEW) returning ...

    Here the function history_insert_partitioned is something like
        CREATE FUNCTION history_insert_partitioned( NEW public.history) RETURNS BIGINT AS $$
            DECLARE
               ...
            BEGIN
               ...
                 < acccess NEW fields e.g. timestamp>
                 <  construct partitioned table name>
                  < EXECUTE 'INSERT INTO ' partitioned table
               ...
                RETURN history_id;
            END;
            $$
            LANGUAGE plpgsql

-----------------------------------
Some references to discussion of this requirement :
  .  http://wiki.postgresql.org/wiki/Todo
     item "Make it possible to use RETURNING together with conditional DO INSTEAD rules,
           such as for partitioning setups"
  .  http://archives.postgresql.org/pgsql-general/2012-06/msg00377.php
  .  http://archives.postgresql.org/pgsql-general/2010-12/msg00542.php
  .  http://acodapella.blogspot.it/2011/06/hibernate-postgresql-table-partitioning.html

-----------------------------------
Proposal:
  .  I propose to extend the rule system to recognize cases where the INSERT query specifies
     RETURNING and the rule promises to return a row,  and to then permit this query to run
     and return the expected row.   In effect,  to widen the definition of "unconditional"
     to handle cases such as my testcase.
  .  One comment is that all the infrastructure for returning one row from the re-written query
     is already present in the code,  and the non-trivial question is how to ensure the
     new design is safe in preventing any rewrite that actually would not return a row.
  .  In this patch,   I have chosen to make use of the LIMIT clause  -
     I add a side-effect implication to a LIMIT clause when it occurs in a rewrite of an INSERT
     to mean "this rule will return a row".
     So,  with my patch, same testcase,  same function history_insert_partitioned and new rule
        CREATE RULE insert_part_history as ON INSERT to history \
               DO INSTEAD SELECT history_insert_partitioned(NEW) LIMIT 1
     the INSERT is accepted and returns the id.
     This use of LIMIT clause is probably contentious but I wished to avoid introducing new
     SQL syntax,  and the LIMIT clause does have a connotation of returning rows.

-----------------------------------
I attach patch based on clone of postgresql.git as of yesterday (120619-145751 EST)
I have tested the patch with INSERT and UPDATE  (not tested with DELETE but should work).
The patch is not expected to be final but just to show how I did it.

John

John Lumby <johnlumby(at)hotmail(dot)com> wrote:

> I attach patch based on clone of postgresql.git as of yesterday

Please read about the CommitFest process:

http://wiki.postgresql.org/wiki/CommitFest

and add your patch to the open CF:

https://commitfest.postgresql.org/action/commitfest_view/open

This will ensure that the patch doesn't get lost before the next review
cycle starts.

-Kevin

On Wed, Jun 20, 2012 at 12:24 PM, John Lumby <johnlumby(at)hotmail(dot)com> wrote:
>     An INSERT which has a RETURNING clause and which is to be rewritten based on
>     a rule will be accepted if the rule is an "unconditional DO INSTEAD".
>     In general I believe "unconditional" means "no WHERE clause", but in practice
>     if the rule is of the form
>        CREATE RULE insert_part_history as ON INSERT to history \
>          DO INSTEAD SELECT history_insert_partitioned(NEW) returning NEW.id
>     this is treated as conditional and the query is rejected.

This isn't rejected because the query is treated as condition; it's
rejected because it's not valid syntax. A SELECT query can't have a
RETURNING clause, because the target list (i.e. the part that
immediately follows the SELECT) already serves that purpose. The fact
that it's in a CREATE RULE statement is irrelevant:

rhaas=# select 4 returning 3;
ERROR: syntax error at or near "returning"
LINE 1: select 4 returning 3;
^

>   .  I propose to extend the rule system to recognize cases where the INSERT query specifies
>      RETURNING and the rule promises to return a row,  and to then permit this query to run
>      and return the expected row.   In effect,  to widen the definition of "unconditional"
>      to handle cases such as my testcase.

That already (kind of) works:

rhaas=# create table history (id bigserial, name text);NOTICE: CREATE
TABLE will create implicit sequence "history_id_seq" for serial column
"history.id"
CREATE TABLE
rhaas=# create table history1 () inherits (history);
CREATE TABLE
rhaas=# create rule history_insert as on insert to history do instead
insert into history1 (id, name) values (NEW.id, NEW.name || ' is
awesome!') returning 17::bigint, 'cheeze whiz'::text;
CREATE RULE
rhaas=# insert into history (name) values ('Linus') returning id,
name; id | name
----+-------------
17 | cheeze whiz
(1 row)

INSERT 0 1
rhaas=# select * from history;
id | name
----+-------------------
1 | Linus is awesome!
(1 row)

I do notice that the RETURNING clause of the INSERT can't reference
NEW, which seems like a restriction that we probably ought to lift,
but it doesn't seem to have much to do with your patch.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company