On Mon, Aug 3, 2020 at 2:54 PM Amit Langote <amitlangot...@gmail.com> wrote:
> However, I noticed that having system columns like ctid, xmin, etc. in
> the RETURNING list is now broken and maybe irrepairably due to the
> approach we are taking in the patch. Let me show an example:
>
> drop table foo;
> create table foo (a int, b int) partition by list (a);
> create table foo1 (c int, b int, a int);
> alter table foo1 drop c;
> alter table foo attach partition foo1 for values in (1);
> create table foo2 partition of foo for values in (2);
> create table foo3 partition of foo for values in (3);
> create or replace function trigfunc () returns trigger language
> plpgsql as $$ begin new.b := 2; return new; end; $$;
> create trigger trig before insert on foo2 for each row execute
> function trigfunc();
> insert into foo values (1, 1), (2, 2), (3, 3);
> update foo set a = 2 from (values (1), (2), (3)) s(x) where a = s.x
> returning tableoid::regclass, ctid, xmin, xmax, *;
> tableoid | ctid | xmin | xmax | a | b | x
> ----------+----------------+------+------------+---+---+---
> foo2 | (4294967295,0) | 128 | 4294967295 | 2 | 2 | 1
> foo2 | (0,3) | 782 | 0 | 2 | 2 | 2
> foo2 | (0,4) | 782 | 0 | 2 | 2 | 3
> (3 rows)
>
> During foo1's update, it appears that we are losing the system
> information in the physical tuple initialized during ExecInsert on
> foo2 during its conversion back to foo1's reltype using the new code.
> I haven't been able to figure out how to preserve the system
> information in HeapTuple contained in the destination slot across the
> conversion. Note we want to use the latter to project the RETURNING
> list.
>
> By the way, you'll need two adjustments to even get this example
> working, one of which is a reported problem that system columns in
> RETURNING list during an operation on partitioned table stopped
> working in PG 12 [1] for which I've proposed a workaround (attached).
> Another is that we forgot in our patch to "materialize" the virtual
> tuple after conversion, which means slot_getsysattr() can't find it to
> access system columns like xmin, etc.
The only solution I could think of for this so far is this:
+ if (map)
+ {
+ orig_slot = execute_attr_map_slot(map,
+ res_slot,
+ orig_slot);
+
+ /*
+ * A HACK to install system information into the just
+ * converted tuple so that RETURNING computes any
+ * system columns correctly. This would be the same
+ * information that would be present in the HeapTuple
+ * version of the tuple in res_slot.
+ */
+ tuple = ExecFetchSlotHeapTuple(orig_slot, true,
+ &should_free);
+ tuple->t_data->t_infomask &= ~(HEAP_XACT_MASK);
+ tuple->t_data->t_infomask2 &= ~(HEAP2_XACT_MASK);
+ tuple->t_data->t_infomask |= HEAP_XMAX_INVALID;
+ HeapTupleHeaderSetXmin(tuple->t_data,
+ GetCurrentTransactionId());
+ HeapTupleHeaderSetCmin(tuple->t_data,
+ estate->es_output_cid);
+ HeapTupleHeaderSetXmax(tuple->t_data, 0); /*
for cleanliness */
+ }
+ /*
+ * Override tts_tableOid with the OID of the destination
+ * partition.
+ */
+ orig_slot->tts_tableOid =
RelationGetRelid(destRel->ri_RelationDesc);
+ /* Also the TID. */
+ orig_slot->tts_tid = res_slot->tts_tid;
..but it might be too ugly :(.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
