On 2 January 2018 at 10:56, David Rowley <david.row...@2ndquadrant.com> wrote: > On 23 December 2017 at 04:00, Amit Khandekar <amitdkhan...@gmail.com> wrote: >> On 15 December 2017 at 18:28, Robert Haas <robertmh...@gmail.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
