While looking for the cause of Erikjan Rijkers' recent report, my
attention was drawn to this hunk of the matview patch:
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index a1a9808e5d94959218b415ed34c46579c478c177..896326615753f2344b466eb180080174ddeda31d 100644
*** a/src/backend/rewrite/rewriteDefine.c
--- b/src/backend/rewrite/rewriteDefine.c
*************** DefineQueryRewrite(char *rulename,
*** 356,362 **** */ checkRuleResultList(query->targetList,
RelationGetDescr(event_relation),
! true); /* * ... there must not be another ON SELECT rule already ...
--- 357,364 ---- */ checkRuleResultList(query->targetList,
RelationGetDescr(event_relation),
! event_relation->rd_rel->relkind !=
! RELKIND_MATVIEW); /* * ... there must not be another ON SELECT rule
already...
IMO this is either flat-out wrong, or an unmaintainable crock, or both.
The third argument of checkRuleResultList is "bool isSelect", defined
thus:
* The targetList might be either a SELECT targetlist, or a RETURNING list;* isSelect tells which. (This is mostly used
forchoosing error messages,* but also we don't enforce column name matching for RETURNING.)
So the above hunk has at the very least rendered the documentation of
checkRuleResultList a lie, but I suspect it's broken the logic too.
I see no very good reason why matviews should work differently from
regular views at all here. But even if there is some aspect of
checkRuleResultList that should work differently for matviews, I really
doubt that we want error messages related to them to be saying
"RETURNING list" when the error is about the SELECT list.
So this either needs to be reverted, or refactored into two arguments
not one, with checkRuleResultList being updated to account honestly
and readably for whatever it's supposed to be doing here.
regards, tom lane
