Re: Declarative partitioning - another take

Hi,

I found that a comment for PartitionRoot in ResultRelInfo is missing.
Although this is trivial, since all other members have comments, I
think it is needed. Attached is the patch to fix it.

Regards,
Yugo Nagata

On Tue, 27 Dec 2016 17:59:05 +0900
Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:

> On 2016/12/26 19:46, Amit Langote wrote:
> > (Perhaps, the following should be its own new thread)
> >
> > I noticed that ExecProcessReturning() doesn't work properly after tuple
> > routing (example shows how returning tableoid currently fails but I
> > mention some other issues below):
> >
> > create table p (a int, b int) partition by range (a);
> > create table p1 partition of p for values from (1) to (10);
> > insert into p values (1) returning tableoid::regclass, *;
> > tableoid | a | b
> > ----------+---+---
> > - | 1 |
> > (1 row)
> >
> > INSERT 0 1
> >
> > I tried to fix that in 0007 to get:
> >
> > insert into p values (1) returning tableoid::regclass, *;
> > tableoid | a | b
> > ----------+---+---
> > p | 1 |
> > (1 row)
> >
> > INSERT 0 1
> >
> > But I think it *may* be wrong to return the root table OID for tuples
> > inserted into leaf partitions, because with select we get partition OIDs:
> >
> > select tableoid::regclass, * from p;
> > tableoid | a | b
> > ----------+---+---
> > p1 | 1 |
> > (1 row)
> >
> > If so, that means we should build the projection info (corresponding to
> > the returning list) for each target partition somehow. ISTM, that's going
> > to have to be done within the planner by appropriate inheritance
> > translation of the original returning targetlist.
>
> Turns out getting the 2nd result may not require planner tweaks after all.
> Unless I'm missing something, translation of varattnos of the RETURNING
> target list can be done as late as ExecInitModifyTable() for the insert
> case, unlike update/delete (which do require planner's attention).
>
> I updated the patch 0007 to implement the same, including the test. While
> doing that, I realized map_partition_varattnos introduced in 0003 is
> rather restrictive in its applicability, because it assumes varno = 1 for
> the expressions it accepts as input for the mapping. Mapping returning
> (target) list required modifying map_partition_varattnos to accept
> target_varno as additional argument. That way, we can map arbitrary
> expressions from the parent attributes numbers to partition attribute
> numbers for expressions not limited to partition constraints.
>
> Patches 0001 to 0006 unchanged.
>
> Thanks,
> Amit

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>