[PATCH] Lockable views

Hi,

Attached is a patch to enable views to be locked.

Nice.

PostgreSQL has supported automatically updatable views since 9.3, so we can
udpate simply defined views like regular tables. However, currently,
table-level locks on views are not supported. We can not execute LOCK TABLE
for views, while we can get row-level locks by FOR UPDATE/SHARE. In some
situations that we need table-level locks on tables, we may also need
table-level locks on automatically updatable views. Although we can lock
base-relations manually, it would be useful if we can lock views without
knowing the definition of the views.

In the attached patch, only automatically-updatable views that do not have
INSTEAD OF rules or INSTEAD OF triggers are lockable. It is assumed that
those views definition have only one base-relation. When an auto-updatable
view is locked, its base relation is also locked. If the base relation is a
view again, base relations are processed recursively. For locking a view,
the view owner have to have he priviledge to lock the base relation.

* Example

test=# CREATE TABLE tbl (i int);
CREATE TABLE

test=# CREATE VIEW v1 AS SELECT * FROM tbl;
CREATE VIEW
test=# BEGIN;
BEGIN
test=# LOCK TABLE v1;
LOCK TABLE
test=# SELECT relname, locktype, mode FROM pg_locks,pg_class c WHERE c.oid=relation AND relname NOT LIKE 'pg%';
relname | locktype | mode
---------+----------+---------------------
tbl | relation | AccessExclusiveLock
v1 | relation | AccessExclusiveLock
(2 rows)

test=# END;
COMMIT

test=# CREATE VIEW v2 AS SELECT * FROM v1;
CREATE VIEW
test=# BEGIN;
BEGIN
test=# LOCK TABLE v2;
LOCK TABLE
test=# SELECT relname, locktype, mode FROM pg_locks,pg_class c WHERE c.oid=relation AND relname NOT LIKE 'pg%';
relname | locktype | mode
---------+----------+---------------------
v2 | relation | AccessExclusiveLock
tbl | relation | AccessExclusiveLock
v1 | relation | AccessExclusiveLock
(3 rows)

test=# END;
COMMIT

test=# CREATE VIEW v3 AS SELECT count(*) FROM v1;
CREATE VIEW
test=# BEGIN;
BEGIN
test=# LOCK TABLE v3;
ERROR: cannot lock view "v3"
DETAIL: Views that return aggregate functions are not automatically updatable.

It would be nice if the message would be something like:

DETAIL: Views that return aggregate functions are not lockable

test=# END;
ROLLBACK

test=# CREATE FUNCTION fnc() RETURNS trigger AS $$ BEGIN RETURN NEW; END; $$ LANGUAGE plpgsql;
CREATE FUNCTION
test=# CREATE TRIGGER trg INSTEAD OF INSERT ON v1 FOR EACH ROW EXECUTE PROCEDURE fnc();
CREATE TRIGGER
test=# BEGIN;
BEGIN
test=# LOCK TABLE v1;
ERROR: cannot lock view "v1"
DETAIL: views that have an INSTEAD OF trigger are not lockable
test=# END;
ROLLBACK

I wonder if we should lock tables in a subquery as well. For example,

create view v1 as select * from t1 where i in (select i from t2);

In this case should we lock t2 as well?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

test=# CREATE VIEW v3 AS SELECT count(*) FROM v1;
CREATE VIEW
test=# BEGIN;
BEGIN
test=# LOCK TABLE v3;
ERROR: cannot lock view "v3"
DETAIL: Views that return aggregate functions are not automatically updatable.

It would be nice if the message would be something like:

DETAIL: Views that return aggregate functions are not lockable

test=# END;
ROLLBACK

test=# CREATE FUNCTION fnc() RETURNS trigger AS $$ BEGIN RETURN NEW; END; $$ LANGUAGE plpgsql;
CREATE FUNCTION
test=# CREATE TRIGGER trg INSTEAD OF INSERT ON v1 FOR EACH ROW EXECUTE PROCEDURE fnc();
CREATE TRIGGER
test=# BEGIN;
BEGIN
test=# LOCK TABLE v1;
ERROR: cannot lock view "v1"
DETAIL: views that have an INSTEAD OF trigger are not lockable
test=# END;
ROLLBACK

I wonder if we should lock tables in a subquery as well. For example,

create view v1 as select * from t1 where i in (select i from t2);

In this case should we lock t2 as well?

Current the patch ignores t2 in the case above.

So we have options below:

- Leave as it is (ignore tables appearing in a subquery)

- Lock all tables including in a subquery

- Check subquery in the view definition. If there are some tables
involved, emit an error and abort.

The first one might be different from what users expect. There may be
a risk that the second one could cause deadlock. So it seems the third
one seems to be the safest IMO.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, 12 Oct 2017 13:11:45 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

test=# CREATE VIEW v3 AS SELECT count(*) FROM v1;
CREATE VIEW
test=# BEGIN;
BEGIN
test=# LOCK TABLE v3;
ERROR: cannot lock view "v3"
DETAIL: Views that return aggregate functions are not automatically updatable.

It would be nice if the message would be something like:

DETAIL: Views that return aggregate functions are not lockable

This uses messages from view_query_is_auto_updatable() of the rewrite system directly.
Although we can modify the messages, I think it is not necessary for now
since we can lock only automatically updatable views.

test=# END;
ROLLBACK

test=# CREATE FUNCTION fnc() RETURNS trigger AS $$ BEGIN RETURN NEW; END; $$ LANGUAGE plpgsql;
CREATE FUNCTION
test=# CREATE TRIGGER trg INSTEAD OF INSERT ON v1 FOR EACH ROW EXECUTE PROCEDURE fnc();
CREATE TRIGGER
test=# BEGIN;
BEGIN
test=# LOCK TABLE v1;
ERROR: cannot lock view "v1"
DETAIL: views that have an INSTEAD OF trigger are not lockable
test=# END;
ROLLBACK

I wonder if we should lock tables in a subquery as well. For example,

create view v1 as select * from t1 where i in (select i from t2);

In this case should we lock t2 as well?

Current the patch ignores t2 in the case above.

So we have options below:

- Leave as it is (ignore tables appearing in a subquery)

- Lock all tables including in a subquery

- Check subquery in the view definition. If there are some tables
involved, emit an error and abort.

The first one might be different from what users expect. There may be
a risk that the second one could cause deadlock. So it seems the third
one seems to be the safest IMO.

Make sense. Even if the view is locked, when tables in a subquery is
modified, the contents of view can change. To avoid it, we have to
lock tables, or give up to lock such views.

We can say the same thing for functions in a subquery. If the definition
of the functions are changed, the result of the view can change.
We cannot lock functions, but should we abtain row-level lock on pg_proc
in such cases? (of cause, or give up to lock such views....)

BTW, though you mentioned the risk of deadlocks, even when there
are no subquery, deadlock can occur in the current patch.

For example, lock a table T in Session1, and then lock a view V
whose base relation is T in Session2. Session2 will wait for
Session1 to release the lock on T. After this, when Session1 try to
lock view V, the deadlock occurs and the query is canceled.

Is this unacceptable behavior?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Yugo Nagata <nagata@sraoss.co.jp>

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

test=# CREATE VIEW v3 AS SELECT count(*) FROM v1;
CREATE VIEW
test=# BEGIN;
BEGIN
test=# LOCK TABLE v3;
ERROR: cannot lock view "v3"
DETAIL: Views that return aggregate functions are not automatically updatable.

It would be nice if the message would be something like:

DETAIL: Views that return aggregate functions are not lockable

This uses messages from view_query_is_auto_updatable() of the rewrite system directly.
Although we can modify the messages, I think it is not necessary for now
since we can lock only automatically updatable views.

You could add a flag to view_query_is_auto_updatable() to switch the
message between

DETAIL: Views that return aggregate functions are not automatically updatable.

and

DETAIL: Views that return aggregate functions are not lockable

I wonder if we should lock tables in a subquery as well. For example,

create view v1 as select * from t1 where i in (select i from t2);

In this case should we lock t2 as well?

Current the patch ignores t2 in the case above.

So we have options below:

- Leave as it is (ignore tables appearing in a subquery)

- Lock all tables including in a subquery

- Check subquery in the view definition. If there are some tables
involved, emit an error and abort.

The first one might be different from what users expect. There may be
a risk that the second one could cause deadlock. So it seems the third
one seems to be the safest IMO.

Make sense. Even if the view is locked, when tables in a subquery is
modified, the contents of view can change. To avoid it, we have to
lock tables, or give up to lock such views.

We can say the same thing for functions in a subquery. If the definition
of the functions are changed, the result of the view can change.
We cannot lock functions, but should we abtain row-level lock on pg_proc
in such cases? (of cause, or give up to lock such views....)

I think we don't need to care about function definition changes used
in where clauses in views. None view queries using functions do not
care about the definition changes of functions while executing the
query. So why updatable views need to care them?

BTW, though you mentioned the risk of deadlocks, even when there
are no subquery, deadlock can occur in the current patch.

For example, lock a table T in Session1, and then lock a view V
whose base relation is T in Session2. Session2 will wait for
Session1 to release the lock on T. After this, when Session1 try to
lock view V, the deadlock occurs and the query is canceled.

You are right. Dealocks could occur in any case.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, 16 Oct 2017 10:07:48 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

test=# CREATE VIEW v3 AS SELECT count(*) FROM v1;
CREATE VIEW
test=# BEGIN;
BEGIN
test=# LOCK TABLE v3;
ERROR: cannot lock view "v3"
DETAIL: Views that return aggregate functions are not automatically updatable.

It would be nice if the message would be something like:

DETAIL: Views that return aggregate functions are not lockable

This uses messages from view_query_is_auto_updatable() of the rewrite system directly.
Although we can modify the messages, I think it is not necessary for now
since we can lock only automatically updatable views.

You could add a flag to view_query_is_auto_updatable() to switch the
message between

DETAIL: Views that return aggregate functions are not automatically updatable.

and

DETAIL: Views that return aggregate functions are not lockable

OK. I'll change view_query_is_auto_updatable() so.

I wonder if we should lock tables in a subquery as well. For example,

create view v1 as select * from t1 where i in (select i from t2);

In this case should we lock t2 as well?

Current the patch ignores t2 in the case above.

So we have options below:

- Leave as it is (ignore tables appearing in a subquery)

- Lock all tables including in a subquery

- Check subquery in the view definition. If there are some tables
involved, emit an error and abort.

The first one might be different from what users expect. There may be
a risk that the second one could cause deadlock. So it seems the third
one seems to be the safest IMO.

Make sense. Even if the view is locked, when tables in a subquery is
modified, the contents of view can change. To avoid it, we have to
lock tables, or give up to lock such views.

We can say the same thing for functions in a subquery. If the definition
of the functions are changed, the result of the view can change.
We cannot lock functions, but should we abtain row-level lock on pg_proc
in such cases? (of cause, or give up to lock such views....)

I think we don't need to care about function definition changes used
in where clauses in views. None view queries using functions do not
care about the definition changes of functions while executing the
query. So why updatable views need to care them?

I'm a bit confused. What is difference between tables and functions
in a subquery with regard to view locking? I think also none view queries
using a subquery do not care about the changes of tables in the
subquery while executing the query. I might be misnderstanding
the problem you mentioned.

BTW, I found that if we have to handle subqueries in where clause, we would
also have to care about subqueries in target list... The view defined as
below is also updatable.

=# create view v7 as select (select * from tbl2 limit 1) from tbl;

BTW, though you mentioned the risk of deadlocks, even when there
are no subquery, deadlock can occur in the current patch.

For example, lock a table T in Session1, and then lock a view V
whose base relation is T in Session2. Session2 will wait for
Session1 to release the lock on T. After this, when Session1 try to
lock view V, the deadlock occurs and the query is canceled.

You are right. Dealocks could occur in any case.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Yugo Nagata <nagata@sraoss.co.jp>

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I'm a bit confused. What is difference between tables and functions
in a subquery with regard to view locking? I think also none view queries
using a subquery do not care about the changes of tables in the
subquery while executing the query. I might be misnderstanding
the problem you mentioned.

The difference is in the function cases we concern the function
definition. While the table cases need to care about table
definitions *and* contents of the table.

If we are changing the table definition, AccessExclusiveLock will be
held for the table and the updation will be blocked anyway. So we
don't need to care about the table definition changes.

On the other hand, table contents changes need to be cared because no
automatic locking are held in some cases. I think whether tables in
the subquery need locking or not is depending on use cases.

So I dug into the previous candidates a little bit more:

1) Leave as it is (ignore tables appearing in a subquery)

2) Lock all tables including in a subquery

3) Check subquery in the view definition. If there are some tables
involved, emit an error and abort.

I think one of the problems with #2 is, we will lock tables involved
by the view in random order, which could cause unwanted dead
locks. This is not good and I cannot see any easy way to avoid
this. Also some tables may not need to be locked.

Problem with #3 is, it does not help a user who wants to control
lockings by himself/herself.

So it seem #1 is the most reasonable way to deal with the problem
assuming that it's user's responsibility to take appropriate locks on
the tables in the subquery.

BTW, I found that if we have to handle subqueries in where clause, we would
also have to care about subqueries in target list... The view defined as
below is also updatable.

=# create view v7 as select (select * from tbl2 limit 1) from tbl;

The view is not updatable. You will get something like if you try to update v7:

DETAIL: Views that have no updatable columns are not automatically updatable.

On the other hand this:

create view v7 as select i, (select j from tbl2 limit 1) from tbl;

will be updatable. In this case column j of v7 will never be
updatable. And you should do something like:

insert into v7(i) values...

In short, you don't need to care about a subquery appearing in the TLE
as far as the view locking concerns.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Oct 11, 2017 at 11:36 AM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

In the attached patch, only automatically-updatable views that do not have
INSTEAD OF rules or INSTEAD OF triggers are lockable. It is assumed that
those views definition have only one base-relation. When an auto-updatable
view is locked, its base relation is also locked. If the base relation is a
view again, base relations are processed recursively. For locking a view,
the view owner have to have he priviledge to lock the base relation.

Why is this the right behavior?

I would have expected LOCK TABLE v to lock the view and nothing else.

See /messages/by-id/AANLkTi=KupesJHRdEvGfbT30aU_iYRO6zwK+fwwY_sGd@mail.gmail.com
for previous discussion of this topic.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 27, 2017 at 2:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Oct 11, 2017 at 11:36 AM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

In the attached patch, only automatically-updatable views that do not have
INSTEAD OF rules or INSTEAD OF triggers are lockable. It is assumed that
those views definition have only one base-relation. When an auto-updatable
view is locked, its base relation is also locked. If the base relation is a
view again, base relations are processed recursively. For locking a view,
the view owner have to have he priviledge to lock the base relation.

Why is this the right behavior?

I would have expected LOCK TABLE v to lock the view and nothing else.

See /messages/by-id/AANLkTi=KupesJHRdEvGfbT30aU_iYRO6zwK+fwwY_sGd@mail.gmail.com
for previous discussion of this topic.

That's what I would expect as well.. But I may be missing something. I
am marking the patch as returned with feedback as this has not been
replied in one month.
--
Michael

On Tue, 17 Oct 2017 11:59:05 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

I'm a bit confused. What is difference between tables and functions
in a subquery with regard to view locking? I think also none view queries
using a subquery do not care about the changes of tables in the
subquery while executing the query. I might be misnderstanding
the problem you mentioned.

The difference is in the function cases we concern the function
definition. While the table cases need to care about table
definitions *and* contents of the table.

If we are changing the table definition, AccessExclusiveLock will be
held for the table and the updation will be blocked anyway. So we
don't need to care about the table definition changes.

On the other hand, table contents changes need to be cared because no
automatic locking are held in some cases. I think whether tables in
the subquery need locking or not is depending on use cases.

So I dug into the previous candidates a little bit more:

1) Leave as it is (ignore tables appearing in a subquery)

2) Lock all tables including in a subquery

3) Check subquery in the view definition. If there are some tables
involved, emit an error and abort.

I think one of the problems with #2 is, we will lock tables involved
by the view in random order, which could cause unwanted dead
locks. This is not good and I cannot see any easy way to avoid
this. Also some tables may not need to be locked.

Problem with #3 is, it does not help a user who wants to control
lockings by himself/herself.

So it seem #1 is the most reasonable way to deal with the problem
assuming that it's user's responsibility to take appropriate locks on
the tables in the subquery.

Thank you for your response. I agree to adopt #1.

BTW, I found that if we have to handle subqueries in where clause, we would
also have to care about subqueries in target list... The view defined as
below is also updatable.

=# create view v7 as select (select * from tbl2 limit 1) from tbl;

The view is not updatable. You will get something like if you try to update v7:

DETAIL: Views that have no updatable columns are not automatically updatable.

Although you can not insert into or update v7, you can delete tuples from v7
since it just delete tuples from table tbl regardless with any column.
However, as disussed above, if it is user's responsibility to take appropriate
locks on the tables in subqueries in the target list, we don't need to
care about these.

On the other hand this:

create view v7 as select i, (select j from tbl2 limit 1) from tbl;

will be updatable. In this case column j of v7 will never be
updatable. And you should do something like:

insert into v7(i) values...

In short, you don't need to care about a subquery appearing in the TLE
as far as the view locking concerns.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Yugo Nagata <nagata@sraoss.co.jp>

On Fri, 27 Oct 2017 07:11:14 +0200
Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Oct 11, 2017 at 11:36 AM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

In the attached patch, only automatically-updatable views that do not have
INSTEAD OF rules or INSTEAD OF triggers are lockable. It is assumed that
those views definition have only one base-relation. When an auto-updatable
view is locked, its base relation is also locked. If the base relation is a
view again, base relations are processed recursively. For locking a view,
the view owner have to have he priviledge to lock the base relation.

Why is this the right behavior?

I would have expected LOCK TABLE v to lock the view and nothing else.

See /messages/by-id/AANLkTi=KupesJHRdEvGfbT30aU_iYRO6zwK+fwwY_sGd@mail.gmail.com
for previous discussion of this topic.

This discussion is one about 7 years ago when automatically-updatable views
are not supported. Since 9.3, simple views can be updated as well as tables,
so now I think it is reasonable that LOCK TABLE for views locks their base
tables.

If we want to lock only the view, it seems to me that LOCK VIEW syntax is good.
However, to realize this, changing the syntax to avoid a shift/reduce
conflict will be needed as disucussed in the "LOCK for non-tables" thread.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Yugo Nagata <nagata@sraoss.co.jp>

On Wed, 29 Nov 2017 11:29:36 +0900
Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Oct 27, 2017 at 2:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Oct 11, 2017 at 11:36 AM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

In the attached patch, only automatically-updatable views that do not have
INSTEAD OF rules or INSTEAD OF triggers are lockable. It is assumed that
those views definition have only one base-relation. When an auto-updatable
view is locked, its base relation is also locked. If the base relation is a
view again, base relations are processed recursively. For locking a view,
the view owner have to have he priviledge to lock the base relation.

Why is this the right behavior?

I would have expected LOCK TABLE v to lock the view and nothing else.

See /messages/by-id/AANLkTi=KupesJHRdEvGfbT30aU_iYRO6zwK+fwwY_sGd@mail.gmail.com
for previous discussion of this topic.

That's what I would expect as well.. But I may be missing something. I
am marking the patch as returned with feedback as this has not been
replied in one month.

I was busy for and I could not work on this patch. After reading the
previous discussion, I still think the behavior of this patch would
be right. So, I would like to reregister to CF 2018-1. Do I need to
create a new entry on CF? or should I change the status to
"Moved to next CF"?

--
Michael

--
Yugo Nagata <nagata@sraoss.co.jp>

On Fri, Dec 22, 2017 at 04:19:46PM +0900, Yugo Nagata wrote:

I was busy for and I could not work on this patch. After reading the
previous discussion, I still think the behavior of this patch would
be right. So, I would like to reregister to CF 2018-1. Do I need to
create a new entry on CF? or should I change the status to
"Moved to next CF"?

This is entirely up to you. It may make sense as well to spawn a new thread
and mark the new patch set as v2, based on the feedback received for v1, as
well as it could make sense to continue with this thread. If the move involves
a complete patch rewrite with a rather different concept, a new thread is more
adapted in my opinion.
--
Michael

On Sat, 23 Dec 2017 09:44:30 +0900
Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Dec 22, 2017 at 04:19:46PM +0900, Yugo Nagata wrote:

I was busy for and I could not work on this patch. After reading the
previous discussion, I still think the behavior of this patch would
be right. So, I would like to reregister to CF 2018-1. Do I need to
create a new entry on CF? or should I change the status to
"Moved to next CF"?

This is entirely up to you. It may make sense as well to spawn a new thread
and mark the new patch set as v2, based on the feedback received for v1, as
well as it could make sense to continue with this thread. If the move involves
a complete patch rewrite with a rather different concept, a new thread is more
adapted in my opinion.

Thank you for your comment.

I have created a new entry in CF-2017-1 and registered this thread again.

--
Michael

--
Yugo Nagata <nagata@sraoss.co.jp>

On Tue, Dec 26, 2017 at 06:37:06PM +0900, Yugo Nagata wrote:

I have created a new entry in CF-2017-1 and registered this thread again.

Fine for me. Thanks for the update. And I guess that you are planning to
send a new version before the beginning of the next commit fest using
the feedback provided?
--
Michael

Yugo Nagata wrote:

On Fri, 27 Oct 2017 07:11:14 +0200
Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Oct 11, 2017 at 11:36 AM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

In the attached patch, only automatically-updatable views that do not have
INSTEAD OF rules or INSTEAD OF triggers are lockable. It is assumed that
those views definition have only one base-relation. When an auto-updatable
view is locked, its base relation is also locked. If the base relation is a
view again, base relations are processed recursively. For locking a view,
the view owner have to have he priviledge to lock the base relation.

Why is this the right behavior?

I would have expected LOCK TABLE v to lock the view and nothing else.

This discussion is one about 7 years ago when automatically-updatable views
are not supported. Since 9.3, simple views can be updated as well as tables,
so now I think it is reasonable that LOCK TABLE for views locks their base
tables.

I agree with Yugo Nagata -- LOCK TABLE is in some cases necessary to
provide the right isolation so that an operation can be carried out
without interference from other processes that want to process the same
data -- and if a view is provided on top of existing tables, preventing
concurrent changes to the data returned by the view is done by locking
the view and recursively the tables that the view are built on, as if
the view were a table. This is why LOCK TABLE is the right command to
do it.

Also, if an application is designed using a table and concurrent changes
are prevented via LOCK TABLE, then when the underlying schema is changed
and the table is replaced by a view, the application continues to work
unchanged; not only syntactically (no error because of table-locking a
view) but also semantically because new application code that modifies
data in underlying tables from paths other than the view will need to
compete with those through the view, which is correct.

See /messages/by-id/AANLkTi=KupesJHRdEvGfbT30aU_iYRO6zwK+fwwY_sGd@mail.gmail.com
for previous discussion of this topic.

If we want to lock only the view, it seems to me that LOCK VIEW syntax is good.
However, to realize this, changing the syntax to avoid a shift/reduce
conflict will be needed as disucussed in the "LOCK for non-tables" thread.

+1 on making TABLE mandatory in LOCK [TABLE], since that will support
this new LOCK VIEW thing as well as locking other object types.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, 26 Dec 2017 22:22:33 +0900
Michael Paquier <michael.paquier@gmail.com> wrote:

On Tue, Dec 26, 2017 at 06:37:06PM +0900, Yugo Nagata wrote:

I have created a new entry in CF-2017-1 and registered this thread again.

Fine for me. Thanks for the update. And I guess that you are planning to
send a new version before the beginning of the next commit fest using
the feedback provided?

Yes. I'll update the patch according to Ishii-san's comments.

--
Michael

--
Yugo Nagata <nagata@sraoss.co.jp>

Hi,

Attached is the updated patch.

On Mon, 16 Oct 2017 10:07:48 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

It would be nice if the message would be something like:

DETAIL: Views that return aggregate functions are not lockable

You could add a flag to view_query_is_auto_updatable() to switch the
message between

DETAIL: Views that return aggregate functions are not automatically updatable.

and

DETAIL: Views that return aggregate functions are not lockable

I didn't want to change the interface of view_query_is_auto_updatable()
because this might be called from other third-patry software, so I renamed
this function to view_query_is_auto_updatable_or_lockable() and added the flag
to this. I created view_query_is_auto_updatable() as a wrapper of this function.
I also made view_query_is_lockable() that returns a other message than
view_query_is_auto_updatable().

On Tue, 17 Oct 2017 11:59:05 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

1) Leave as it is (ignore tables appearing in a subquery)

2) Lock all tables including in a subquery

3) Check subquery in the view

So it seem #1 is the most reasonable way to deal with the problem
assuming that it's user's responsibility to take appropriate locks on
the tables in the subquery.

I adopted #1 and I didn't change anything about this.

Regards,

--
Yugo Nagata <nagata@sraoss.co.jp>

I didn't want to change the interface of view_query_is_auto_updatable()
because this might be called from other third-patry software, so I renamed
this function to view_query_is_auto_updatable_or_lockable() and added the flag
to this. I created view_query_is_auto_updatable() as a wrapper of this function.
I also made view_query_is_lockable() that returns a other message than
view_query_is_auto_updatable().

On Tue, 17 Oct 2017 11:59:05 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

1) Leave as it is (ignore tables appearing in a subquery)

2) Lock all tables including in a subquery

3) Check subquery in the view

So it seem #1 is the most reasonable way to deal with the problem
assuming that it's user's responsibility to take appropriate locks on
the tables in the subquery.

I adopted #1 and I didn't change anything about this.

Looks good to me except that the patch lacks documents and the
regression test needs more cases. For example, it needs a test for the
case #1 above: probably using pg_locks to make sure that the tables
appearing in the subquery do not hold locks.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

On Thu, 28 Dec 2017 09:29:11 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

I didn't want to change the interface of view_query_is_auto_updatable()
because this might be called from other third-patry software, so I renamed
this function to view_query_is_auto_updatable_or_lockable() and added the flag
to this. I created view_query_is_auto_updatable() as a wrapper of this function.
I also made view_query_is_lockable() that returns a other message than
view_query_is_auto_updatable().

On Tue, 17 Oct 2017 11:59:05 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

1) Leave as it is (ignore tables appearing in a subquery)

2) Lock all tables including in a subquery

3) Check subquery in the view

So it seem #1 is the most reasonable way to deal with the problem
assuming that it's user's responsibility to take appropriate locks on
the tables in the subquery.

I adopted #1 and I didn't change anything about this.

Looks good to me except that the patch lacks documents and the
regression test needs more cases. For example, it needs a test for the
case #1 above: probably using pg_locks to make sure that the tables
appearing in the subquery do not hold locks.

Attached is the update patch, v3. I add some regression test and
the documentation.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Yugo Nagata <nagata@sraoss.co.jp>