On Wed, Feb 10, 2016 at 2:21 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Feb 9, 2016 at 9:11 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Tue, Feb 9, 2016 at 4:27 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>> Thanks for updating the patch!
>>> Attached is the updated version of the patch.
>>> I removed unnecessary assertion check and change of source code
>>> that you added, and improved the source comment.
>>> Barring objection, I'll commit this patch.
>>
>> So, this code basically duplicates what is already in
>> refresh_by_match_merge to check if there is a UNIQUE index defined. If
>> we are sure that an error is detected earlier in the code as done in
>> this patch, wouldn't it be better to replace the error message in
>> refresh_by_match_merge() by an assertion?
>
> I'm OK with an assertion if we add source comment about why
> refresh_by_match_merge() can always guarantee that there is
> a unique index on the matview. Probably it's because the matview
> is locked with exclusive lock at the start of ExecRefreshMatView(),
> i.e., it's guaranteed that we cannot drop any indexes on the matview
> after the first check is passed. Also it might be better to add
> another comment about that the caller of refresh_by_match_merge()
> must always check that there is a unique index on the matview before
> calling that function, just in the case where it's called elsewhere
> in the future.
>
> OTOH, I don't think it's not so bad idea to just emit an error, instead.
Sorry, s/it's not/it's
Regards,
--
Fujii Masao
