On Wed, Sep 23, 2020 at 10:12 PM Amit Langote <amitlangot...@gmail.com> wrote: > On Sun, Sep 20, 2020 at 11:41 PM Etsuro Fujita <etsuro.fuj...@gmail.com> > wrote: > > On Mon, Sep 14, 2020 at 10:45 PM Amit Langote <amitlangot...@gmail.com> > > wrote: > > > On Mon, Sep 14, 2020 at 5:56 PM Etsuro Fujita <etsuro.fuj...@gmail.com> > > > wrote: > > > > IIUC, I think two issues are discussed in the thread [1]: (a) there is > > > > currently no way to define the set of meaningful system columns for a > > > > partitioned table that contains pluggable storages other than standard > > > > heap, and (b) even in the case where the set is defined as before, > > > > like partitioned tables that don’t contain any pluggable storages, > > > > system column values are not obtained from a tuple inserted into a > > > > partitioned table in cases as reported in [1], because virtual slots > > > > are assigned for partitioned tables [2][3]. (I think the latter is > > > > the original issue in the thread, though.)
> > > > I think we could fix this update-tuple-routing-vs-RETURNING issue > > > > without the fix for (a). But to transfer system column values in a > > > > cleaner way, I think we need to fix (b) first so that we can always > > > > obtain/copy them from the new tuple moved to another partition by > > > > INSERT following DELETE. > > > > > > Yes, I think you are right. Although, I am still not sure how to > > > "copy" system columns from one partition's slot to another's, that is, > > > without assuming what they are. > > > > I just thought we assume that partitions support all the existing > > system attributes until we have the fix for (a), i.e., the slot > > assigned for a partition must have the getsysattr callback routine > > from which we can fetch each existing system attribute of a underlying > > tuple in the slot, regardless of whether that system attribute is used > > for the partition or not. > > Hmm, to copy one slot's system attributes into another, we will also > need a way to "set" the system attributes in the destination slot. > But maybe I didn't fully understand what you said. Sorry, my thought was vague. To store xmin/xmax/cmin/cmax into a given slot, we need to extend the TupleTableSlot struct to contain these attributes as well? Or we need to introduce a new callback routine for that (say, setsysattr)? These would not be back-patchable, though. > > I also created a patch for PG11, which I am attaching > > as well. > > In the patch for PG 11: > > + new_tuple->t_self = new_tuple->t_data->t_ctid = > + old_tuple->t_self; > ... > > Should we add a one-line comment above this block of code to transfer > system attributes? Maybe: /* Also transfer the system attributes. */? Will add that comment. Thanks for reviewing! > BTW, do you think we should alter the test in PG 11 branch to test > that system attributes are returned correctly? Yeah, I think so. I didn’t come up with any good test cases for that, though. > Once we settle the > other issue, we can adjust the HEAD's test to do likewise? Yeah, but for the other issue, I started thinking that we should just forbid referencing xmin/xmax/cmin/cmax in 12, 13, and HEAD... Best regards, Etsuro Fujita
