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...@lab.ntt.co.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 <nag...@sraoss.co.jp>
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 6332ea0..845bdf2 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -326,6 +326,7 @@ typedef struct JunkFilter * onConflictSetWhere list of ON CONFLICT DO UPDATE exprs (qual) * PartitionCheck partition check expression * PartitionCheckExpr partition check expression state + * PartitionRoot relation descriptor for parent relation * ---------------- */ typedef struct ResultRelInfo
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
