On Thu, Dec 22, 2016 at 9:49 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova
> <lubennikovaav@gmail.com> wrote:
>> The following review has been posted through the commitfest application:
>> make installcheck-world: tested, passed
>> Implements feature: tested, passed
>> Spec compliant: tested, passed
>> Documentation: tested, passed
>>
>> Hi, thank you for the patch.
>> Results are very promising. Do you see any drawbacks of this feature or something that requires more testing?
>>
>
> I think you can focus on the handling of array scan keys for testing.
> In general, one of my colleagues has shown interest in testing this
> patch and I think he has tested as well but never posted his findings.
> I will request him to share his findings and what kind of tests he has
> done, if any.
>
>> I'm willing to oo a review.
>
> Thanks, that will be helpful.
>
>
>> I saw the discussion about parameters in the thread above. And I agree that we'd better concentrate
>> on the patch itself and add them later if necessary.
>>
>> 1. Can't we simply use "if (scan->parallel_scan != NULL)" instead of xs_temp_snap flag?
>>
>> + if (scan->xs_temp_snap)
>> + UnregisterSnapshot(scan->xs_snapshot);
>>
>
> I agree with what Rober has told in his reply.
>
Typo.
/Rober/Robert Haas
Thanks to Michael Paquier for noticing it and informing me offline.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com