Re: UPDATE of partition key

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: UPDATE of partition key
Date: 2017-06-06 18:24:42
Message-ID: CA+TgmoZPLQ+0E3BSwPbed-OjHsoPpmsoZVnHPTY7BRDUAgsRvQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 5, 2017 at 2:51 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> Greg/Amit's idea of using the CTID field rather than an infomask bit
>> seems like a possibly promising approach. Not everything that needs
>> bit-space can use the CTID field, so using it is a little less likely
>> to conflict with something else we want to do in the future than using
>> a precious infomask bit. However, I'm worried about this:
>>
>> /* Make sure there is no forward chain link in t_ctid */
>> tp.t_data->t_ctid = tp.t_self;
>>
>> The comment does not say *why* we need to make sure that there is no
>> forward chain link, but it implies that some code somewhere in the
>> system does or at one time did depend on no forward link existing.
>
> I think it is to ensure that EvalPlanQual mechanism gets invoked in
> the right case. The visibility routine will return HeapTupleUpdated
> both when the tuple is deleted or updated (updated - has a newer
> version of the tuple), so we use ctid to decide if we need to follow
> the tuple chain for a newer version of the tuple.

That would explain why need to make sure that there *is* a forward
chain link in t_ctid for an update, but it doesn't explain why we need
to make sure that there *isn't* a forward link for delete.

> The proposed change in WARM tuple patch uses ip_posid field of CTID
> and we are planning to use ip_blkid field. Here is the relevant text
> and code from WARM tuple patch:
>
> "Store the root line pointer of the WARM chain in the t_ctid.ip_posid
> field of the last tuple in the chain and mark the tuple header with
> HEAP_TUPLE_LATEST flag to record that fact."
>
> +#define HeapTupleHeaderSetHeapLatest(tup, offnum) \
> +do { \
> + AssertMacro(OffsetNumberIsValid(offnum)); \
> + (tup)->t_infomask2 |= HEAP_LATEST_TUPLE; \
> + ItemPointerSetOffsetNumber(&(tup)->t_ctid, (offnum)); \
> +} while (0)
>
> For further details, refer patch 0001-Track-root-line-pointer-v23_v26
> in the below e-mail:
> https://www.postgresql.org/message-id/CABOikdOTstHK2y0rDk%2BY3Wx9HRe%2BbZtj3zuYGU%3DVngneiHo5KQ%40mail.gmail.com

OK.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-06-06 18:36:35 Re: Should we standardize on a type for signal handler flags?
Previous Message Andres Freund 2017-06-06 18:24:35 Re: Should we standardize on a type for signal handler flags?