On Thu, Jan 28, 2016 at 1:01 AM, Masahiko Sawada <sawada.m...@gmail.com> wrote:
> 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.
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.
Regards,
--
Fujii Masao
*** a/src/backend/commands/matview.c
--- b/src/backend/commands/matview.c
***************
*** 217,222 **** ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
--- 217,267 ----
RelationGetRelationName(matviewRel));
/*
+ * Check that there is a unique index with no WHERE clause on
+ * one or more columns of the materialized view if CONCURRENTLY
+ * is specified.
+ */
+ if (concurrent)
+ {
+ List *indexoidlist = RelationGetIndexList(matviewRel);
+ ListCell *indexoidscan;
+ bool hasUniqueIndex = false;
+
+ foreach(indexoidscan, indexoidlist)
+ {
+ Oid indexoid = lfirst_oid(indexoidscan);
+ Relation indexRel;
+ Form_pg_index indexStruct;
+
+ indexRel = index_open(indexoid, AccessShareLock);
+ indexStruct = indexRel->rd_index;
+
+ if (indexStruct->indisunique &&
+ IndexIsValid(indexStruct) &&
+ RelationGetIndexExpressions(indexRel) == NIL &&
+ RelationGetIndexPredicate(indexRel) == NIL &&
+ indexStruct->indnatts > 0)
+ {
+ hasUniqueIndex = true;
+ index_close(indexRel, AccessShareLock);
+ break;
+ }
+
+ index_close(indexRel, AccessShareLock);
+ }
+
+ list_free(indexoidlist);
+
+ if (!hasUniqueIndex)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot refresh materialized view \"%s\" concurrently",
+ quote_qualified_identifier(get_namespace_name(RelationGetNamespace(matviewRel)),
+ RelationGetRelationName(matviewRel))),
+ errhint("Create a unique index with no WHERE clause on one or more columns of the materialized view.")));
+ }
+
+ /*
* The stored query was rewritten at the time of the MV definition, but
* has not been scribbled on by the planner.
*/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
