On Tue, Jun 26, 2018 at 7:02 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> On Tue, Jun 26, 2018 at 6:18 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Tue, Jun 26, 2018 at 4:33 PM, Ashutosh Bapat
>> <ashutosh.bapat@enterprisedb.com> wrote:
>>> Looks like we need similar adjustment in ExecSimpleRelationUpdate() as
>>> well. Updated the patch.
>>>
>>
>> - /* Store the slot into tuple that we can write. */
>> + /* Materialize slot into a tuple that we can inspect. */
>> tuple = ExecMaterializeSlot(slot);
>>
>> I think it is better to keep the last word of the sentence as "write"
>> instead of "inspect" as was in the original sentence.
>
> A copy-pasto while correcting a typo :)
>
>> It makes more
>> sense as we are materializing the tuple to write it. Similarly, in
>> the other change in the patch can use "write".
>
> Are you suggesting that we should use "write" in the modified comment
> instead of "inspect" in original comment.
>
Yes.
Another variant used in few other similar places is:
/*
* get the heap tuple out of the tuple table slot, making sure we have a
* writable copy
*/
> Ok, I have now corrected grammar as well. Here's updated patch.
>
Your new comment is also correct. I like the comment already used in
code as that makes the code consistent, what do you think?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com