Thanks a lot, Dean, to look into this and also sorry for the late reply.
On Sun, Jan 5, 2020 at 12:08 AM Dean Rasheed <dean.a.rash...@gmail.com>
wrote:
> Tracing it through, this seems to be a result of
> cab5dc5daf2f6f5da0ce79deb399633b4bb443b5 which added support for
> updatable views with a mix of updatable and non-updatable columns.
> That included a change to rewriteTargetListIU() to prevent it from
> adding dummy targetlist entries for unassigned-to attributes for
> auto-updatable views, in case they are no longer simple references to
> the underlying relation. Instead, that is left to expand_targetlist(),
> as for a normal table. However, in this case (an UPDATE on a view with
> a conditional rule), the target relation of the original query isn't
> rewritten (we leave it to the executor to report the error), and so
> expand_targetlist() ends up adding a new targetlist entry that
> references the target relation, which is still the original view. But
> when the planner bulds the simple_rel_array, it only adds entries for
> relations referenced in the query's jointree, which only includes the
> base table by this point, not the view. Thus the new targetlist entry
> added by expand_targetlist() refers to a NULL slot in the
> simple_rel_array, and it blows up.
>
> That's a great analysis of this issue.
> Given that this is a query that's going to fail anyway, I'm inclined
> to think that the right thing to do is to throw the error sooner, in
> rewriteQuery(), rather than attempting to plan a query that cannot be
> executed.
>
I am wondering whether a simple auto-updatable view can have a conditional
update instead rule.
For the test case I added, does bellow plan looks reasonable?
gpadmin=# explain UPDATE v1 SET b = 2 WHERE a = 1;
QUERY PLAN
-------------------------------------------------------------------
Insert on t2 (cost=0.00..49.55 rows=1 width=8)
-> Seq Scan on t1 (cost=0.00..49.55 rows=1 width=8)
Filter: ((b > 100) AND (a > 2) AND (a = 1))
Update on t1 (cost=0.00..49.55 rows=3 width=14)
-> Seq Scan on t1 (cost=0.00..49.55 rows=3 width=14)
Filter: (((a > 2) IS NOT TRUE) AND (b > 100) AND (a = 1))
(7 rows)
gpadmin=# UPDATE v1 SET b = 2 WHERE a = 1;
UPDATE 1
The document also says that:
"There is a catch if you try to use conditional rules for *complex view*
updates: there must be an unconditional
INSTEAD rule for each action you wish to allow on the view" which makes me
think a simple view can have a
conditional INSTEAD rule. And the document is explicitly changed in commit
a99c42f291421572aef2:
- There is a catch if you try to use conditional rules for view
+ There is a catch if you try to use conditional rules for complex view
Does that mean we should support conditional rules for a simple view?
Regards,
Pengzhou Tang
