On Tue, Jan 30, 2018 at 6:48 PM, Tatsuo Ishii <is...@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
