On Wed, Dec 13, 2017 at 3:26 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Sat, Dec 9, 2017 at 5:30 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Sat, Dec 9, 2017 at 1:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Fri, Dec 8, 2017 at 5:11 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>>>> Okay, I have adjusted the patch accordingly. I have also added a
>>>>> regression test which should produce the same result across different
>>>>> runs, see if that looks okay to you, then it is better to add such a
>>>>> test as well.
>>>>
>>>> The regression test added by patch needs cleanup at the end which I
>>>> have added in the attached patch.
>>>
>>> Hmm. If we're going this way, then shouldn't we revert the changes
>>> commit 2c09a5c12a66087218c7f8cba269cd3de51b9b82 made to
>>> ExecParallelRetrieveInstrumentation?
>>>
>>
>> Yeah, it is better to revert it as ideally that is not required after
>> this patch and that is what I have tried to convey above ("Ideally, it
>> would have obviated the need for my previous patch which
>> got committed as 778e78ae." (The commit id is for branch 10,
>> otherwise, it is same as what you mention.)). I have locally reverted
>> that patch and then rebased it on top of that.
>
> Uh, should I just revert that commit entirely first, and then we can
> commit the new fix afterward?
>
Yes. I have already extracted the test case of that commit to the new
patch which is what we need from that commit.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com