I have extracted a couple of changes into preparatory patches, as
explained below :
On 20 September 2017 at 21:27, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> On 20 September 2017 at 00:06, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Fri, Sep 15, 2017 at 7:25 AM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>>> [ new patch ]
>>
>> This already fails to apply again. In general, I think it would be a
>> good idea to break this up into a patch series rather than have it as
>> a single patch. That would allow some bits to be applied earlier.
>> The main patch will probably still be pretty big, but at least we can
>> make things a little easier by getting some of the cleanup out of the
>> way first. Specific suggestions on what to break out below.
>>
>> If the changes to rewriteManip.c are a marginal efficiency hack and
>> nothing more, then let's commit this part separately before the main
>> patch. If they're necessary for correctness, then please add a
>> comment explaining why they are necessary.
>
> Ok. Yes, just wanted to avoid two ConvertRowtypeExpr nodes one over
> the other. But that was not causing any correctness issue. Will
> extract these changes into separate patch.
The patch for the above change is :
0002-Prevent-a-redundant-ConvertRowtypeExpr-node.patch
>
>>
>> There appears to be no reason why the definitions of
>> GetInsertedColumns() and GetUpdatedColumns() need to be moved to a
>> header file as a result of this patch. GetUpdatedColumns() was
>> previously defined in trigger.c and execMain.c and, post-patch, is
>> still called from only those files. GetInsertedColumns() was, and
>> remains, called only from execMain.c. If this were needed I'd suggest
>> doing it as a preparatory patch before the main patch, but it seems we
>> don't need it at all.
>
> In earlier versions of the patch, these functions were used in
> nodeModifyTable.c as well. Now that those calls are not there in this
> file, I will revert back the changes done for moving the definitions
> into header file.
Did the above , and included in the attached revised patch
update-partition-key_v19.patch.
>
>>
>> If I understand correctly, the reason for changing mt_partitions from
>> ResultRelInfo * to ResultRelInfo ** is that, currently, all of the
>> ResultRelInfos for a partitioning hierarchy are allocated as a single
>> chunk, but we can't do that and also reuse the ResultRelInfos created
>> during InitPlan. I suggest that we do this as a preparatory patch.
>
> Ok, will prepare a separate patch. Do you mean to include in that
> patch the changes I did in ExecSetupPartitionTupleRouting() that
> re-use the ResultRelInfo structures of per-subplan update result rels
> ?
Above changes are in attached
0001-Re-use-UPDATE-result-rels-created-in-InitPlan.patch.
Patches are to be applied in this order :
0001-Re-use-UPDATE-result-rels-created-in-InitPlan.patch
0002-Prevent-a-redundant-ConvertRowtypeExpr-node.patch
update-partition-key_v19.patch
--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers