Re: matview patch readability/correctness gripe

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> 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.

> 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.

Will review in light of your comments.

Thanks!

--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company