Thanks Amit for your review comments. I am yet to handle all of your
comments, but meanwhile , attached is an updated patch, that handles
RETURNING.
Earlier it was not working because ExecInsert() did not return any
RETURNING clause. This is because the setup needed to create RETURNIG
projection info for leaf partitions is done in ExecInitModifyTable()
only in case of INSERT. But because it is an UPDATE operation, we have
to do this explicitly as a one-time operation when it is determined
that row-movement is required. This is similar to how we do one-time
setup of mt_partition_dispatch_info. So in the patch, I have moved
this code into a new function ExecInitPartitionReturningProjection(),
and now this is called in ExecInitModifyTable() as well as during row
movement for ExecInsert() processing the returning clause.
Basically we need to do all that is done in ExecInitModifyTable() for
INSERT. There are a couple of other things that I suspect that might
need to be done as part of the missing initialization for Execinsert()
during row-movement :
1. Junk filter handling
2. WITH CHECK OPTION
Yet, ExecDelete() during row-movement is still returning the RETURNING
result redundantly, which I am yet to handle this.
On 23 March 2017 at 07:04, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Hi Amit,
>
> Thanks for the updated patch.
>
> On 2017/03/23 3:09, Amit Khandekar wrote:
>> Attached is v2 patch which implements the above optimization.
>
> Would it be better to have at least some new tests? Also, there are a few
> places in the documentation mentioning that such updates cause error,
> which will need to be updated. Perhaps also add some explanatory notes
> about the mechanism (delete+insert), trigger behavior, caveats, etc.
> There were some points discussed upthread that could be mentioned in the
> documentation.
Yeah, agreed. Will do this in the subsequent patch.
>
> @@ -633,6 +634,9 @@ ExecDelete(ItemPointer tupleid,
> HeapUpdateFailureData hufd;
> TupleTableSlot *slot = NULL;
>
> + if (already_deleted)
> + *already_deleted = false;
> +
>
> concurrently_deleted?
Done.
>
> @@ -962,7 +969,7 @@ ExecUpdate(ItemPointer tupleid,
> }
> else
> {
> - LockTupleMode lockmode;
> + LockTupleMode lockmode;
>
> Useless hunk.
Removed.
I am yet to handle your other comments , still working on them, but
till then , attached is the updated patch.
