Michael Paquier <michael.paqu...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers