Re: Rules on table partitions

Hi Dean,

On 2017/06/19 20:19, Dean Rasheed wrote:
> Currently we allow rules to be defined on table partitions, but these
> rules only fire when the partition is accessed directly, not when it
> is accessed via the parent:

Yeah, the same thing as will happen with an inheritance setup, but I guess
you are asking whether that's what we want.

> CREATE TABLE t1(a int, b int) PARTITION BY RANGE(a);
> CREATE TABLE t1_p PARTITION OF t1 FOR VALUES FROM (1) TO (10);
> INSERT INTO t1 VALUES (1,101), (2,201);
>
> CREATE TABLE t1_p_log(a int, b int, d date);
> CREATE RULE t1_p_log_rule AS ON UPDATE TO t1_p
> DO ALSO INSERT INTO t1_p_log VALUES(old.a, old.b, now());
>
> UPDATE t1 SET b=b+1 WHERE a=1;
> UPDATE t1_p SET b=b+1 WHERE a=2;
>
> SELECT * FROM t1_p_log;
>
> a | b | d
> ---+-----+------------
> 2 | 201 | 2017-06-19
> (1 row)
>
>
> I'd regard that as a bug, especially since this kind of thing would
> have worked with old-style user-defined partitioning.

With old-style inheritance based setup, you will see exactly the exact
same result:

create table p (a int, b int);
create table c () inherits (p);
create table c_log (a int, b int, c date);
create rule c_p_log_rule as on update to c
do also insert into c_log values (old.a, old.b, now());
insert into p values (1, 100);
insert into c values (2, 200)

update p set b = b + 1 where a = 1;
select tableoid::regclass, * from p;
tableoid | a | b
----------+---+-----
p | 1 | 101
c | 2 | 200
(2 rows)

select * from c_log;
a | b | c
---+---+---
(0 rows)

update p set b = b + 1 where a = 2;
select tableoid::regclass, * from p;
tableoid | a | b
----------+---+-----
p | 1 | 101
c | 2 | 201
(2 rows)

select * from c_log;
a | b | c
---+---+---
(0 rows)

update c set b = b + 1 where a = 2;
select tableoid::regclass, * from p;
tableoid | a | b
----------+---+-----
p | 1 | 101
c | 2 | 202
(2 rows)

select * from c_log;
a | b | c
---+-----+------------
2 | 201 | 2017-06-20
(1 row)

Note that inheritance is expanded in the planner, not the rewriter, so the
rules of partitions (that are added to the query later than the rewriter)
won't fire, AFAICS. Same will apply to partitions.

> Perhaps we
> should explicitly forbid this for now -- i.e., raise a "not supported"
> error when attempting to add a rule to a partition, or attach a table
> with rules to a partitioned table.

We could do that, but an oft-raised question is how different we should
make new partitions from the old-style inheritance child tables?

Although a slightly different territory, you will also notice that
statement triggers of partitions won't be fired unless they are explicitly
named in the query, which is what happens for inheritance in general and
hence also for partitions.

> Personally, I wouldn't regard adding proper support for rules on
> partitions as a high priority, so I'd be OK with it remaining
> unsupported unless someone cares enough to implement it, but that
> seems preferable to leaving it partially working in this way.

Sure, if consensus turns out to be that we prohibit rules, statement
triggers, etc. that depend on the relation being explicitly named in the
query to be defined on partitions, I could draft up a patch for v10.

> Also, as things stand, it is possible to do the following:
>
> CREATE TABLE t2(a int, b int) PARTITION BY RANGE(a);
> CREATE TABLE t2_p PARTITION OF t2 FOR VALUES FROM (1) TO (10);
> CREATE RULE "_RETURN" AS ON SELECT TO t2_p DO INSTEAD SELECT * FROM t2;
>
> which results in the partition becoming a view that selects from the
> parent, which surely ought to be forbidden.

Hmm, yes. The following exercise convinced me.

create table r (a int) partition by range (a);
create table r1 partition of r for values from (1) to (10);
create rule "_RETURN" as on select to r1 do instead select * from r;

insert into r values (1);
ERROR: cannot insert into view "r1"
HINT: To enable inserting into the view, provide an INSTEAD OF INSERT
trigger or an unconditional ON INSERT DO INSTEAD rule.

The error is emitted by CheckValidResultRel() that is called on individual
leaf partitions when setting up tuple-routing in ExecInitModifyTable.

I agree that we should forbid this case, so please find attached a patch.

Thanks,
Amit