Re: WIP patch (v2) for updatable security barrier views

On 01/29/2014 03:34 PM, Kouhei Kaigai wrote:
>> Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
>>> Let me ask an elemental question. What is the reason why inheritance
>>> expansion logic should be located on the planner stage, not on the
>>> tail of rewriter?
>>
>> I think it's mostly historical. You would however have to think of a way
>> to preserve the inheritance relationships in the parsetree representation.
>> In the current code, expand_inherited_tables() adds AppendRelInfo nodes
>> to the planner's data structures as it does the expansion; but I don't think
>> AppendRelInfo is a suitable structure for the rewriter to work with, and
>> in any case there's no place to put it in the Query representation.
>>
>> Actually though, isn't this issue mostly about inheritance of a query
>> *target* table? 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.
>>
> It's just an idea, so might not be a deep consideration.
>
> Isn't ii available to describe a parse tree as if some UPDATE/DELETE statements
> are combined with UNION ALL? Of course, even if it is only internal form.
>
> UPDATE parent SET x = 2*x, y = y || '_update' WHERE x % 10 = 5
> UNION ALL
> UPDATE child_1 SET x = 2*x, y = y || '_update' WHERE x % 10 = 5
> :
>
> Right now, only SELECT statement is allowed being placed under set-operations.
> If rewriter can expand inherited relations into multiple individual selects
> with UNION ALL, it may be a reasonable internal representation.
>
> In similar way, both of UPDATE/DELETE takes a scan on relation once, then
> it modifies the target relation. Probably, here is no significant different
> on the earlier half; that performs as if multiple SELECTs with UNION ALL are
> running, except for it fetches ctid system column as junk attribute and
> acquires row-level locks.
>
> On the other hand, it may need to adjust planner code to construct
> ModifyTable node running on multiple relations. Existing code pulls
> resultRelations from AppendRelInfo, doesn't it? It needs to take the list
> of result relations using recursion of set operations, if not flatten.
> Once we can construct ModifyTable with multiple relations on behalf of
> multiple relation's scan, here is no difference from what we're doing now.
>
> How about your opinion?

My worry here is that a fair bit of work gets done before inheritance
expansion. Partitioning already performs pretty poorly for anything but
small numbers of partitions.

If we're expanding inheritance in the rewriter, won't that further
increase the already large amount of duplicate work involved in planning
a query that involves inheritance?

Or am I misunderstanding you?

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services