Re: [HACKERS] UPDATE of partition key

On 2 January 2018 at 10:56, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 23 December 2017 at 04:00, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
>> On 15 December 2017 at 18:28, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> - PartitionDispatch **pd,
>>> - ResultRelInfo ***partitions,
>>> - TupleConversionMap ***tup_conv_maps,
>>> - TupleTableSlot **partition_tuple_slot,
>>> - int *num_parted, int *num_partitions)
>>> + PartitionTupleRouting **partition_tuple_routing)
>>>
>>> Since we're consolidating all of ExecSetupPartitionTupleRouting's
>>> output parameters into a single structure, I think it might make more
>>> sense to have it just return that value. I think it's only done with
>>> output parameter today because there are so many different things
>>> being produced, and we can't return them all.
>>
>> You mean ExecSetupPartitionTupleRouting() will return the structure
>> (not pointer to structure), and the caller will get the copy of the
>> structure like this ? :
>>
>> mtstate->mt_partition_tuple_routing =
>> ExecSetupPartitionTupleRouting(mtstate, rel, node->nominalRelation, estate);
>>
>> I am ok with that, but just wanted to confirm if that is what you are
>> saying. I don't recall seeing a structure return value in PG code, so
>> not sure if it is conventional in PG to do that. Hence, I am somewhat
>> inclined to keep it as output param. It also avoids a structure copy.
>>
>> Another way is for ExecSetupPartitionTupleRouting() to palloc this
>> structure, and return its pointer, but then caller would have to
>> anyway do a structure copy, so that's not convenient, and I don't
>> think you are suggesting this way either.
>
> I'm pretty sure Robert is suggesting that
> ExecSetupPartitionTupleRouting pallocs the memory for the structure,
> sets it up then returns a pointer to the new struct. That's not very
> unusual. It seems unusual for a function to return void and modify a
> single parameter pointer to get the value to the caller rather than
> just to return that value.

Sorry, my mistake. Earlier I somehow was under the impression that the
callers of ExecSetupPartitionTupleRouting() already have this
structure palloc'ed, and that they pass address of this structure. I
now can see that both CopyStateData->partition_tuple_routing and
ModifyTableState->mt_partition_tuple_routing are pointers, not
structures. So it make perfect sense for
ExecSetupPartitionTupleRouting() to palloc and return a pointer. Sorry
for the noise. Will share the change in an upcoming patch version.
Thanks !

--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company