Re: Existence check for suitable index in advance when concurrently refreshing. - Mailing list pgsql-hackers

On Wed, Jan 27, 2016 at 4:42 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Jan 26, 2016 at 9:33 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> Hi all,
>>
>> In concurrently refreshing materialized view, we check whether that
>> materialized view has suitable index(unique and not having WHERE
>> condition), after filling data to new snapshot
>> (refresh_matview_datafill()).
>> This logic leads to taking a lot of time until postgres returns ERROR
>> log if that table doesn't has suitable index and table is large. it
>> wastes time.
>> I think we should check whether that materialized view can use
>> concurrently refreshing or not in advance.
>
> +1
>
>> The patch is attached.
>>
>> Please give me feedbacks.

Thank you for having look at this patch.

> +            indexRel = index_open(indexoid, RowExclusiveLock);
>
> Can we use AccessShareLock here, instead?

Yeah, I think we can use it. Fixed.

> +            if (indexStruct->indisunique &&
> +                IndexIsValid(indexStruct) &&
> +                RelationGetIndexExpressions(indexRel) == NIL &&
> +                RelationGetIndexPredicate(indexRel) == NIL)
> +                hasUniqueIndex = true;
> +
> +            index_close(indexRel, RowExclusiveLock);
>
> In the case where hasUniqueIndex = true, ISTM that we can get out of
> the loop immediately just after calling index_close(). No?

Fixed.

> +    /* Must have at least one unique index */
> +    Assert(foundUniqueIndex);
>
> Can we guarantee that there is at least one valid unique index here?
> If yes, it's better to write the comment about that.
>

Added.

Attached latest patch. Please review it.

Regards,

--
Masahiko Sawada