On 29 January 2014 11:27, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 29 January 2014 06:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Actually though, isn't this issue mostly about inheritance of a query
>> *target* table?
>
> Exactly. IMHO updateable views on inheritance sets will have multiple
> other performance problems, so trying to support them here will not
> make their usage seamless.
>
> We allowed updateable views to work with restrictions in earlier
> releases, so I can't see why continuing with a much reduced
> restriction would be a problem in this release. We don't need to
> remove the remaining restriction all in one release, so we?
>
>> Moving that expansion to the rewriter is a totally
>> different and perhaps more tractable change. It's certainly horribly ugly
>> as it is; hard to see how doing it at the rewriter could be worse.
>
> I see the patch adding some changes to inheritance_planner that might
> well get moved to rewriter.
> As long as the ugliness all stays in one place, does it matter where
> that is -- for this patch -- ? Just asking whether moving it will
> reduce the net ugliness of our inheritance support.
>
> @Craig: I don't think this would have much effect on partitioning
> performance. The main cost of that is constraint exclusion, which we
> wuld still perform only once. All other copying and re-jigging still
> gets performed even for excluded relations, so doing it earlier
> wouldn't hurt as much as you might think.
>
> @Dean: If we moved that to rewriter, would expand_security_quals() and
> preprocess_rowmarks() also move?
>
Actually I tend to think that expand_security_quals() should remain
where it is, regardless of where inheritance expansion happens. One of
the things that simplifies the job that expand_security_quals() has to
do is that it takes place after preprocess_targetlist(), so the
targetlist for the security barrier subqueries that it constructs is
known. Calling expand_security_quals() earlier would require
additional surgery on preprocess_targetlist() and expand_targetlist(),
and would probably also mean that the Query would need to record the
sourceRelation subquery RTE, as discussed upthread.
Regards,
Dean
