On Wed, Jan 27, 2016 at 4:42 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
> On Tue, Jan 26, 2016 at 9:33 PM, Masahiko Sawada <sawada.m...@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

Attachment: matview_concurrently_refresh_check_index_v2.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to