Re: UPDATE of partition key

Hi Amit,

Thanks for updating the patch.  Since ddl.sgml got updated on Saturday,
patch needs a rebase.

> On 31 March 2017 at 16:54, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>> On 31 March 2017 at 14:04, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> On 2017/03/28 19:12, Amit Khandekar wrote:
>>>> In the section 5.11 "Partitioning" => subsection 5 "Caveats", I have
>>>> removed the caveat of not being able to update partition key. And it
>>>> is now replaced by the caveat where an update/delete operations can
>>>> silently miss a row when there is a concurrent UPDATE of partition-key
>>>> happening.
>>>
>>> Hmm, how about just removing the "partition-changing updates are
>>> disallowed" caveat from the list on the 5.11 Partitioning page and explain
>>> the concurrency-related caveats on the UPDATE reference page?
>>
>> IMHO this caveat is better placed in Partitioning chapter to emphasize
>> that it is a drawback specifically in presence of partitioning.

I mean we fixed things for declarative partitioning so it's no longer a
caveat like it is for partitioning implemented using inheritance (in that
the former doesn't require user-defined triggers to implement
row-movement).  Seeing the first sentence, that is:

An <command>UPDATE</> causes a row to move from one partition to another
if the new value of the row fails to satisfy the implicit partition
constraint of the original partition but there is another partition which
can fit this row.

which clearly seems to suggest that row-movement, if required, is handled
by the system.  So it's not clear why it's in this list.  If we want to
describe the limitations of the current implementation, we'll need to
rephrase it a bit.  How about something like:

For an <command>UPDATE</> that causes a row to move from one partition to
another due the partition key being updated, the following caveats exist:
<a brief description of the possibility of surprising results in the
presence of concurrent manipulation of the row in question>

>>> +    If an <command>UPDATE</command> on a partitioned table causes a row to
>>> +    move to another partition, it is possible that all row-level
>>> +    <literal>BEFORE</> <command>UPDATE</command> triggers and all row-level
>>> +    <literal>BEFORE</> <command>DELETE</command>/<command>INSERT</command>
>>> +    triggers are applied on the respective partitions in a way that is
>>> apparent
>>> +    from the final state of the updated row.
>>>
>>> How about dropping "it is possible that" from this sentence?
>>
>> What the statement means is : "It is true that all triggers are
>> applied on the respective partitions; but it is possible that they are
>> applied in a way that is apparent from final state of the updated
>> row". So "possible" applies to "in a way that is apparent..". It
>> means, the user should be aware that all the triggers can change the
>> row and so the final row will be affected by all those triggers.
>> Actually, we have a similar statement for UPSERT involved with
>> triggers in the preceding section. I have taken the statement from
>> there.

I think where it appears in that sentence made me think it could be
confusing to some.  How about reordering sentences in that paragraph so
that the whole paragraphs reads as follows:

If an UPDATE on a partitioned table causes a row to move to another
partition, it will be performed as a DELETE from the original partition
followed by INSERT into the new partition. In this case, all row-level
BEFORE UPDATE triggers and all row-level BEFORE DELETE triggers are fired
on the original partition. Then all row-level BEFORE INSERT triggers are
fired on the destination partition. The possibility of surprising outcomes
should be considered when all these triggers affect the row being moved.
As far as AFTER ROW triggers are concerned, AFTER DELETE and AFTER INSERT
triggers are applied; but AFTER UPDATE triggers are not applied because
the UPDATE has been converted to a DELETE and INSERT. None of the DELETE
and INSERT statement-level triggers are fired, even if row movement
occurs; only the UPDATE triggers of the target table used in the UPDATE
statement will be fired.

Finally, I forgot to mention during the last review that the new parameter
'returning' to ExecDelete() could be called 'process_returning'.

Thanks,
Amit