Michael Paquier <michael.paquier@gmail.com> wrote:
> I am not sure that adding a boolean flag introducing a concept
> related to matview inside checkRuleResultList is the best
> approach to solve that. checkRuleResultList is something related
> only to rules, and has nothing related to matviews in it yet.
Well, I was tempted to keep that concept in DefineQueryRewrite()
where the call is made, by calling the new bool something like
requireColumnNameMatch and not having checkRuleResultList()
continue to use isSelect for that purpose at all.
DefineQueryRewrite() already references RELKIND_RELATION once,
RELKIND_MATVIEW twice, and RELKIND_VIEW three times, so it would
hardly be introducing a new concept there. I did it the way I did
in the posted patch because it seemed to more nearly match what Tom
was suggesting.
> I have been pondering myself about this issue, and wouldn't it be
> better to directly enforce targetList in checkRuleResultList call
> at an upper level with the column name list given by users if
> there is any instead of the list of columns of the SELECT query?
>
> After looking closely at the code this looks to be a better
> approach from a general viewpoint. I didn't got the time to write
> a patch but this is the conclusion I reached after some analysis
> at least...
That doesn't sound like something we could back-patch.
Anyway, I'm not sure why that would be better. We have rewrite
rules on three kinds of relations -- tables, views, and
materialized views. In general, a materialized view tends to
support the properties of both a view and a table, with a few
points in the rewrite code that need to pay attention to which kind
of relation the rule applies to. Among the various validations for
the rewrite, this one validation (matching column names) does not
apply to a non-SELECT rule with a RETURNING clause or to a SELECT
rule which populates a materialized view. If you were to move the
code to exclude this validation for the latter case, wouldn't you
also need to move the validation for the former case? Where would
you put that?
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
