On Tue, Jan 30, 2018 at 6:48 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
>>> You need to DROP VIEW lock_view4 and lock_view5 in the regression
>>> test as well.
>>
>> Thank you for reviewing the patch.
>>
>> I fixed this and attached a updated patch v6.
>
> Looks good to me. If there's no objection, especially from Thomas
> Munro, I will mark this as "ready for committer".
About the idea: it makes some kind of sense to me that we should lock
the underlying table, in all the same cases that you could do DML on
the view automatically. I wonder if this is a problem for the
soundness: "Tables appearing in a subquery are ignored and not
locked." I can imagine using this for making backwards-compatible
schema changes, in which case the LOCK-based transaction isolation
techniques might benefit from this behaviour. I'd be interested to
hear about the ideal use case you have in mind.
About the patch: I didn't study it in detail. It builds, has
documentation and passes all tests. Would it be a good idea to add an
isolation test to show that the underlying relation is actually
locked?
Typo:
+ /* Check permissions with the view owner's priviledge. */
s/priviledge/privilege/
Grammar:
+/*
+ * Check whether the view is lockable.
+ *
+ * Currently, only auto-updatable views can be locked, that is,
+ * views whose definition are simple and that doesn't have
+ * instead of rules or triggers are lockable.
s/definition are simple and that doesn't/definition is simple and that don't/
s/instead of/INSTEAD OF/ ?
--
Thomas Munro
http://www.enterprisedb.com