Thread: [HACKERS] [PATCH] Lockable views

[HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
Hi,

Attached is a patch to enable views to be locked.

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.
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



Regards,

-- 
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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> 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

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> 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

Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> >> 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

Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> 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

Re: [HACKERS] [PATCH] Lockable views

From
Robert Haas
Date:
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 http://postgr.es/m/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

Re: [HACKERS] [PATCH] Lockable views

From
Michael Paquier
Date:
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 http://postgr.es/m/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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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>


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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 http://postgr.es/m/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>


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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 http://postgr.es/m/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>


Re: [HACKERS] [PATCH] Lockable views

From
Michael Paquier
Date:
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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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>


Re: [HACKERS] [PATCH] Lockable views

From
Michael Paquier
Date:
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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Alvaro Herrera
Date:
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 http://postgr.es/m/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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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>


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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>

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> 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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
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>

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> >> > 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.

The patch produces a warning.

/home/t-ishii/lock_view-v3.patch:542: trailing whitespace.
-- Verify that we  can lock a auto-updatable views 
warning: 1 line adds whitespace errors.

Your addition to the doc:
+   Automatically updatable views (see <xref linkend="sql-createview">)
+   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>
+   rules are also lockable. When a view is locked, its base relations are
+   also locked recursively with the same lock mode.

does not mention about the point:

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

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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
Hi,

The updated patch is attached.

On Fri, 29 Dec 2017 23:39:39 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

 
> The patch produces a warning.
> 
> /home/t-ishii/lock_view-v3.patch:542: trailing whitespace.
> -- Verify that we  can lock a auto-updatable views 
> warning: 1 line adds whitespace errors.

Fixed.

> 
> Your addition to the doc:
> +   Automatically updatable views (see <xref linkend="sql-createview">)
> +   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>
> +   rules are also lockable. When a view is locked, its base relations are
> +   also locked recursively with the same lock mode.
> 
> does not mention about the point:
> 
> >> >> > 1) Leave as it is (ignore tables appearing in a subquery)

I added this point to the documentation.


Regards,

> 
> 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>

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Thomas Munro
Date:
On Sun, Dec 31, 2017 at 11:57 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> On Fri, 29 Dec 2017 23:39:39 +0900 (JST)
> Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
>> Your addition to the doc:
>> +   Automatically updatable views (see <xref linkend="sql-createview">)
>> +   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>
>> +   rules are also lockable. When a view is locked, its base relations are
>> +   also locked recursively with the same lock mode.
>
> I added this point to the documentation.

+   Automatically updatable views (see <xref linkend="sql-createview">)
+   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>

Tthe documentation doesn't build: you now need to say </literal>
instead of </>, and you need to say <xref ... />.

-- 
Thomas Munro
http://www.enterprisedb.com


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Thu, 25 Jan 2018 20:51:41 +1300
Thomas Munro <thomas.munro@enterprisedb.com> wrote:

> On Sun, Dec 31, 2017 at 11:57 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> > On Fri, 29 Dec 2017 23:39:39 +0900 (JST)
> > Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> >> Your addition to the doc:
> >> +   Automatically updatable views (see <xref linkend="sql-createview">)
> >> +   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>
> >> +   rules are also lockable. When a view is locked, its base relations are
> >> +   also locked recursively with the same lock mode.
> >
> > I added this point to the documentation.
> 
> +   Automatically updatable views (see <xref linkend="sql-createview">)
> +   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>

Thank you for pointing out this.

Also, I've noticed this patch cannot be applied to the HEAD, so I'll fix
them together and update the patch.

> 
> Tthe documentation doesn't build: you now need to say </literal>
> instead of </>, and you need to say <xref ... />.
> 
> -- 
> Thomas Munro
> http://www.enterprisedb.com
> 


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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Fri, 26 Jan 2018 21:30:49 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

> On Thu, 25 Jan 2018 20:51:41 +1300
> Thomas Munro <thomas.munro@enterprisedb.com> wrote:
> 
> > On Sun, Dec 31, 2017 at 11:57 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> > > On Fri, 29 Dec 2017 23:39:39 +0900 (JST)
> > > Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> > >> Your addition to the doc:
> > >> +   Automatically updatable views (see <xref linkend="sql-createview">)
> > >> +   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>
> > >> +   rules are also lockable. When a view is locked, its base relations are
> > >> +   also locked recursively with the same lock mode.
> > >
> > > I added this point to the documentation.
> > 
> > +   Automatically updatable views (see <xref linkend="sql-createview">)
> > +   that do not have <literal>INSTEAD OF</> triggers or <literal>INSTEAD</>
> 
> Thank you for pointing out this.
> 
> Also, I've noticed this patch cannot be applied to the HEAD, so I'll fix
> them together and update the patch.

Attached is the updated patch v5 including fixing SGML and rebase to HEAD.

Regards,

> 
> > 
> > Tthe documentation doesn't build: you now need to say </literal>
> > instead of </>, and you need to say <xref ... />.
> > 
> > -- 
> > Thomas Munro
> > http://www.enterprisedb.com
> > 
> 
> 
> -- 
> Yugo Nagata <nagata@sraoss.co.jp>
> 


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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> Attached is the updated patch v5 including fixing SGML and rebase to HEAD.

You need to DROP VIEW lock_view4 and lock_view5 in the regression
test 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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Tue, 30 Jan 2018 13:58:30 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

> > Attached is the updated patch v5 including fixing SGML and rebase to HEAD.
> 
> 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.

Regards,

> 
> 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>

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> 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".

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


Re: [HACKERS] [PATCH] Lockable views

From
Thomas Munro
Date:
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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Tue, 30 Jan 2018 19:21:04 +1300
Thomas Munro <thomas.munro@enterprisedb.com> wrote:

> 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.

I think the use case is almost similar to that of auto-updatable views.
There are some purpose to use views, for example 1) preventing from
modifying application due to schema changes, 2) protecting the underlying
table from users without proper privileges, 3) making a shorthand of a
query with complex WHERE condition. When these are updatable views and
users need transaction isolation during updating them, I think the lockable
views feature is benefitical because users don't need to refer to the
underlying table. Users might don't know the underlying table, or even
might not have the privilege to lock this.

> 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?

Whether the underlying relation is actually locked or not is confirmed
in the regression test using pg_locks, so I don't believe that we need
to add an isolation test.
 
> 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/ ?

Thank you for pointing out these. I attached the fixed patch.

Regards,
 
> -- 
> Thomas Munro
> http://www.enterprisedb.com


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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Thomas Munro
Date:
On Tue, Jan 30, 2018 at 6:48 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> Looks good to me. If there's no objection, especially from Thomas
> Munro, I will mark this as "ready for committer".

No objection from me.

-- 
Thomas Munro
http://www.enterprisedb.com


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> On Tue, Jan 30, 2018 at 6:48 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
>> Looks good to me. If there's no objection, especially from Thomas
>> Munro, I will mark this as "ready for committer".
> 
> No objection from me.

I marked this as "Ready for Committer".

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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Thu, 01 Feb 2018 09:48:49 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

> > On Tue, Jan 30, 2018 at 6:48 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> >> Looks good to me. If there's no objection, especially from Thomas
> >> Munro, I will mark this as "ready for committer".
> > 
> > No objection from me.
> 
> I marked this as "Ready for Committer".

Thank you for reviewing the patch!

Regards,

> 
> 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>


Re: [HACKERS] [PATCH] Lockable views

From
Robert Haas
Date:
On Tue, Jan 30, 2018 at 1:21 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> 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."

Yeah, that seems like a pretty bad idea.  It's exposing what is
basically an implementation detail to users.  I think that if we
change the rules for which subqueries get flattened in a future
release, then the behavior will also change.  That seems bad.

I also think that this is a bad idea for another reason, which is that
it leaves us with no syntax to say that you want to lock the view
itself, and pg_dump wants do that if only we had syntax for it.

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


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> On Tue, Jan 30, 2018 at 1:21 AM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
>> 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."
> 
> Yeah, that seems like a pretty bad idea.  It's exposing what is
> basically an implementation detail to users.

Initially I thought all base tables including ones in a subquery also
should be locked like you. But after some discussions with Yugo, I
agree that locking table in a subquery is less valuable for users (and
I am afraid it may introduce more deadlock chances). See upthead
discussions.

> I think that if we
> change the rules for which subqueries get flattened in a future
> release, then the behavior will also change.  That seems bad.

I doubt it could happen in the future but if that happend we should
disallow locking on such views.

> I also think that this is a bad idea for another reason, which is that
> it leaves us with no syntax to say that you want to lock the view
> itself, and pg_dump wants do that if only we had syntax for it.

I agree with Yugo and Alvaro. It's better to have a separate syntax
for locking views itself.

https://www.postgresql.org/message-id/20171226143407.6wjzjn42pt54qskm@alvherre.pgsql

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


Re: [HACKERS] [PATCH] Lockable views

From
Robert Haas
Date:
On Thu, Feb 1, 2018 at 8:09 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> Initially I thought all base tables including ones in a subquery also
> should be locked like you. But after some discussions with Yugo, I
> agree that locking table in a subquery is less valuable for users (and
> I am afraid it may introduce more deadlock chances). See upthead
> discussions.

I just reread those discussions but I don't see that they really make
any argument for the behavior the patch implements.  I see no
explanation on the thread for why locking a table inside of a subquery
is more or less likely to cause deadlock than locking one outside of a
subquery.

>> I think that if we
>> change the rules for which subqueries get flattened in a future
>> release, then the behavior will also change.  That seems bad.
>
> I doubt it could happen in the future but if that happend we should
> disallow locking on such views.

That doesn't make any sense to me.  When someone migrates from
PostgreSQL 11 to, say, PostgreSQL 14, the view definition is going to
be recreated from an SQL query.  Neither the user nor the database
will know whether the query was optimized the same way on both
databases, so how could we disallow locking only those views where
there was a difference on the two releases?  Even if we could, how
does that help anything?  Throwing an error is just as much a
backward-incompatibility in the command as silently changing what gets
locked.

But my complaint may have been a little off base all the same -- I
guess we're doing this based on the rewriter output, rather than the
optimizer output, which makes it a lot less likely that we would
decide to change anything here.

>> I also think that this is a bad idea for another reason, which is that
>> it leaves us with no syntax to say that you want to lock the view
>> itself, and pg_dump wants do that if only we had syntax for it.
>
> I agree with Yugo and Alvaro. It's better to have a separate syntax
> for locking views itself.
>
> https://www.postgresql.org/message-id/20171226143407.6wjzjn42pt54qskm@alvherre.pgsql

Hmm, Alvaro's argument makes sense, I guess.  I withdraw this complaint.

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


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
Robert,

> I just reread those discussions but I don't see that they really make
> any argument for the behavior the patch implements.  I see no
> explanation on the thread for why locking a table inside of a subquery
> is more or less likely to cause deadlock than locking one outside of a
> subquery.

If we allow to lock a table in a subquery, following could happen:

We have: CREATE VIEW v1 AS SELECT * FROM t1 WHERE i = (SELECT i FROM t2);

1. Session A tries to lock v1 (I suppose it tries to acquire lock in
the order of t1, then t2). A acquires lock on t1 but yet on t2.

2. Another session B acquires lock on t2.

3. A continues to try to acquire lock on t2 (blocked).

4. B tries to acquire lock on t1. Deadlock occurs.

So if a user mixes locking a view and a underlying base table, there's
a possibility of deadlocks. If we regard that it is user's
responsibility not to mix them or to be careful about the consequence
the mixing of locks so that dealocks do not happen, then I would agree
that we should lock a table inside a subquery.

What do you think?

>>> I think that if we
>>> change the rules for which subqueries get flattened in a future
>>> release, then the behavior will also change.  That seems bad.
>>
>> I doubt it could happen in the future but if that happend we should
>> disallow locking on such views.
> 
> That doesn't make any sense to me.  When someone migrates from
> PostgreSQL 11 to, say, PostgreSQL 14, the view definition is going to
> be recreated from an SQL query.  Neither the user nor the database
> will know whether the query was optimized the same way on both
> databases, so how could we disallow locking only those views where
> there was a difference on the two releases?  Even if we could, how
> does that help anything?  Throwing an error is just as much a
> backward-incompatibility in the command as silently changing what gets
> locked.
> 
> But my complaint may have been a little off base all the same -- I
> guess we're doing this based on the rewriter output, rather than the
> optimizer output, which makes it a lot less likely that we would
> decide to change anything here.

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


Re: [HACKERS] [PATCH] Lockable views

From
Robert Haas
Date:
On Mon, Feb 5, 2018 at 8:18 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> We have: CREATE VIEW v1 AS SELECT * FROM t1 WHERE i = (SELECT i FROM t2);
>
> 1. Session A tries to lock v1 (I suppose it tries to acquire lock in
> the order of t1, then t2). A acquires lock on t1 but yet on t2.
>
> 2. Another session B acquires lock on t2.
>
> 3. A continues to try to acquire lock on t2 (blocked).
>
> 4. B tries to acquire lock on t1. Deadlock occurs.

True.  But the same exact analysis also applies to this definition,
which contains no subquery:

CREATE VIEW v1 AS SELECT t1.* FROM t1, t2 WHERE t1.i = t2.i;

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


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> True.  But the same exact analysis also applies to this definition,
> which contains no subquery:
> 
> CREATE VIEW v1 AS SELECT t1.* FROM t1, t2 WHERE t1.i = t2.i;

That's not an updatable view, thus cannot be locked according to the
proposed implementation.

Anyway do you want to allow to lock all base tables in a view
definition if the view is updatable?

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


Re: [HACKERS] [PATCH] Lockable views

From
Robert Haas
Date:
On Mon, Feb 5, 2018 at 10:26 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
>> True.  But the same exact analysis also applies to this definition,
>> which contains no subquery:
>>
>> CREATE VIEW v1 AS SELECT t1.* FROM t1, t2 WHERE t1.i = t2.i;
>
> That's not an updatable view, thus cannot be locked according to the
> proposed implementation.

Hmm, true.  Why exactly are we imposing the restriction to updateable
views, anyway?

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


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> Hmm, true.  Why exactly are we imposing the restriction to updateable
> views, anyway?

In my understanding, because of ambiguity to determine which rows in
which base tables needs to be modified by just looking at the DML
against a view. There could be multiple ways to modify the base
tables.

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


Re: [HACKERS] [PATCH] Lockable views

From
Robert Haas
Date:
On Mon, Feb 5, 2018 at 10:49 PM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
>> Hmm, true.  Why exactly are we imposing the restriction to updateable
>> views, anyway?
>
> In my understanding, because of ambiguity to determine which rows in
> which base tables needs to be modified by just looking at the DML
> against a view. There could be multiple ways to modify the base
> tables.

But what does that have to do with locking?

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


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> But what does that have to do with locking?

Well, if the view is not updatable, I think there will be less point
to allow to lock the base tables in the view because locking is
typically used in a case when updates are required.

Of course we could add special triggers to allow to update views that
are not automatically updatable but that kind of views are tend to
complex and IMO there's less need the automatic view locking feature.

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


Re: [HACKERS] [PATCH] Lockable views

From
Robert Haas
Date:
On Tue, Feb 6, 2018 at 1:28 AM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
>> But what does that have to do with locking?
>
> Well, if the view is not updatable, I think there will be less point
> to allow to lock the base tables in the view because locking is
> typically used in a case when updates are required.
>
> Of course we could add special triggers to allow to update views that
> are not automatically updatable but that kind of views are tend to
> complex and IMO there's less need the automatic view locking feature.

Hmm.  Well, I see now why you've designed the feature in the way that
you have, but I guess it still seems somewhat arbitrary to me.  If you
ignore the deadlock consideration, then there's no reason not to
define the feature as locking every table mentioned anywhere in the
query, including subqueries, and it can work for all views whether
updatable or not.  If the deadlock consideration is controlling, then
I guess we can't do better than what you have, but I'm not sure how
future-proof it is.  If in the future somebody makes views updateable
that involve a join, say from the primary key of one table to a unique
key of another so that no duplicate rows can be introduced, then
they'll either have to write code to make this feature identify and
lock the "main" table, which I'm not sure would be strong enough in
all cases, or lock them all, which reintroduces the deadlock problem.

Personally, I would be inclined to view the deadlock problem as not
very important.  I just don't see how that is going to come up very
often.  What I do think will be an issue is that if you start locking
lots of tables, you might prevent the system from getting much work
done, whether or not you also cause any deadlocks.  But I don't see
what we can do about that, really.  If users want full control over
which tables get locked, then they have to name them explicitly.  Or
alternatively, maybe they should avoid the need for full-table locks
by using SSI, gaining the benefits of (1) optimistic rather than
pessimistic concurrency control, (2) finer-grained locking, and (3)
not needing to issue explicit LOCK commands.

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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Tue, 6 Feb 2018 11:12:37 -0500
Robert Haas <robertmhaas@gmail.com> wrote:

> On Tue, Feb 6, 2018 at 1:28 AM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> >> But what does that have to do with locking?
> >
> > Well, if the view is not updatable, I think there will be less point
> > to allow to lock the base tables in the view because locking is
> > typically used in a case when updates are required.
> >
> > Of course we could add special triggers to allow to update views that
> > are not automatically updatable but that kind of views are tend to
> > complex and IMO there's less need the automatic view locking feature.
> 
> Hmm.  Well, I see now why you've designed the feature in the way that
> you have, but I guess it still seems somewhat arbitrary to me.  If you
> ignore the deadlock consideration, then there's no reason not to
> define the feature as locking every table mentioned anywhere in the
> query, including subqueries, and it can work for all views whether
> updatable or not.  If the deadlock consideration is controlling, then
> I guess we can't do better than what you have, but I'm not sure how
> future-proof it is.  If in the future somebody makes views updateable
> that involve a join, say from the primary key of one table to a unique
> key of another so that no duplicate rows can be introduced, then
> they'll either have to write code to make this feature identify and
> lock the "main" table, which I'm not sure would be strong enough in
> all cases, or lock them all, which reintroduces the deadlock problem.
> 
> Personally, I would be inclined to view the deadlock problem as not
> very important.  I just don't see how that is going to come up very

I agree that the deadlock won't occur very often and this is not
so important.

I have updated the lockable-view patch to v8.

This new version doen't consider the deadlock problem, and all tables
or views appearing in the view definition query are locked recursively.
Also, this allows all kinds of views to be locked even if it is not
auto-updatable view.


> often.  What I do think will be an issue is that if you start locking
> lots of tables, you might prevent the system from getting much work
> done, whether or not you also cause any deadlocks.  But I don't see
> what we can do about that, really.  If users want full control over
> which tables get locked, then they have to name them explicitly.  Or
> alternatively, maybe they should avoid the need for full-table locks
> by using SSI, gaining the benefits of (1) optimistic rather than
> pessimistic concurrency control, (2) finer-grained locking, and (3)
> not needing to issue explicit LOCK commands.



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


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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Tue, 27 Mar 2018 23:28:04 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

I found the previous patch was broken and this can't handle
views that has subqueries as bellow;

 CREATE VIEW lock_view6 AS SELECT * from (select * from lock_tbl1) sub;

I fixed this and attached the updated version including additional tests.

Regards,

> On Tue, 6 Feb 2018 11:12:37 -0500
> Robert Haas <robertmhaas@gmail.com> wrote:
> 
> > On Tue, Feb 6, 2018 at 1:28 AM, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> > >> But what does that have to do with locking?
> > >
> > > Well, if the view is not updatable, I think there will be less point
> > > to allow to lock the base tables in the view because locking is
> > > typically used in a case when updates are required.
> > >
> > > Of course we could add special triggers to allow to update views that
> > > are not automatically updatable but that kind of views are tend to
> > > complex and IMO there's less need the automatic view locking feature.
> > 
> > Hmm.  Well, I see now why you've designed the feature in the way that
> > you have, but I guess it still seems somewhat arbitrary to me.  If you
> > ignore the deadlock consideration, then there's no reason not to
> > define the feature as locking every table mentioned anywhere in the
> > query, including subqueries, and it can work for all views whether
> > updatable or not.  If the deadlock consideration is controlling, then
> > I guess we can't do better than what you have, but I'm not sure how
> > future-proof it is.  If in the future somebody makes views updateable
> > that involve a join, say from the primary key of one table to a unique
> > key of another so that no duplicate rows can be introduced, then
> > they'll either have to write code to make this feature identify and
> > lock the "main" table, which I'm not sure would be strong enough in
> > all cases, or lock them all, which reintroduces the deadlock problem.
> > 
> > Personally, I would be inclined to view the deadlock problem as not
> > very important.  I just don't see how that is going to come up very
> 
> I agree that the deadlock won't occur very often and this is not
> so important.
> 
> I have updated the lockable-view patch to v8.
> 
> This new version doen't consider the deadlock problem, and all tables
> or views appearing in the view definition query are locked recursively.
> Also, this allows all kinds of views to be locked even if it is not
> auto-updatable view.
> 
> 
> > often.  What I do think will be an issue is that if you start locking
> > lots of tables, you might prevent the system from getting much work
> > done, whether or not you also cause any deadlocks.  But I don't see
> > what we can do about that, really.  If users want full control over
> > which tables get locked, then they have to name them explicitly.  Or
> > alternatively, maybe they should avoid the need for full-table locks
> > by using SSI, gaining the benefits of (1) optimistic rather than
> > pessimistic concurrency control, (2) finer-grained locking, and (3)
> > not needing to issue explicit LOCK commands.
> 
> 
> 
> > -- 
> > Robert Haas
> > EnterpriseDB: http://www.enterprisedb.com
> > The Enterprise PostgreSQL Company
> 
> 
> -- 
> Yugo Nagata <nagata@sraoss.co.jp>


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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> On Tue, 27 Mar 2018 23:28:04 +0900
> Yugo Nagata <nagata@sraoss.co.jp> wrote:
> 
> I found the previous patch was broken and this can't handle
> views that has subqueries as bellow;
> 
>  CREATE VIEW lock_view6 AS SELECT * from (select * from lock_tbl1) sub;
> 
> I fixed this and attached the updated version including additional tests.

This patch gives a warning while compiling:

lockcmds.c:186:1: warning: no semicolon at end of struct or union
 } LockViewRecurse_context;
 ^

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


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> I found the previous patch was broken and this can't handle
>> views that has subqueries as bellow;
>> 
>>  CREATE VIEW lock_view6 AS SELECT * from (select * from lock_tbl1) sub;
>> 
>> I fixed this and attached the updated version including additional tests.
> 
> This patch gives a warning while compiling:
> 
> lockcmds.c:186:1: warning: no semicolon at end of struct or union
>  } LockViewRecurse_context;
>  ^

Also I get a regression test failure:

*** /usr/local/src/pgsql/current/postgresql/src/test/regress/expected/lock.out    2018-03-28 15:24:13.805314577 +0900
--- /usr/local/src/pgsql/current/postgresql/src/test/regress/results/lock.out    2018-03-28 15:42:07.975592594 +0900
***************
*** 118,124 ****
  ------------
   lock_tbl1
   lock_view6
! (2 rows)
  
  ROLLBACK;
  -- Verify that we can lock a table with inheritance children.
--- 118,125 ----
  ------------
   lock_tbl1
   lock_view6
!  mvtest_tm
! (3 rows)

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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Wed, 28 Mar 2018 15:45:09 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

> >> I found the previous patch was broken and this can't handle
> >> views that has subqueries as bellow;
> >> 
> >>  CREATE VIEW lock_view6 AS SELECT * from (select * from lock_tbl1) sub;
> >> 
> >> I fixed this and attached the updated version including additional tests.
> > 
> > This patch gives a warning while compiling:
> > 
> > lockcmds.c:186:1: warning: no semicolon at end of struct or union
> >  } LockViewRecurse_context;
> >  ^
> 
> Also I get a regression test failure:

Thank you for your reviewing my patch.
I attached the updated patch, v10.

Regards,

> 
> *** /usr/local/src/pgsql/current/postgresql/src/test/regress/expected/lock.out    2018-03-28 15:24:13.805314577
+0900
> --- /usr/local/src/pgsql/current/postgresql/src/test/regress/results/lock.out    2018-03-28 15:42:07.975592594 +0900
> ***************
> *** 118,124 ****
>   ------------
>    lock_tbl1
>    lock_view6
> ! (2 rows)
>   
>   ROLLBACK;
>   -- Verify that we can lock a table with inheritance children.
> --- 118,125 ----
>   ------------
>    lock_tbl1
>    lock_view6
> !  mvtest_tm
> ! (3 rows)
> 
> 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>

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
> On Wed, 28 Mar 2018 15:45:09 +0900 (JST)
> Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> 
>> >> I found the previous patch was broken and this can't handle
>> >> views that has subqueries as bellow;
>> >> 
>> >>  CREATE VIEW lock_view6 AS SELECT * from (select * from lock_tbl1) sub;
>> >> 
>> >> I fixed this and attached the updated version including additional tests.
>> > 
>> > This patch gives a warning while compiling:
>> > 
>> > lockcmds.c:186:1: warning: no semicolon at end of struct or union
>> >  } LockViewRecurse_context;
>> >  ^
>> 
>> Also I get a regression test failure:
> 
> Thank you for your reviewing my patch.
> I attached the updated patch, v10.

Thanks. Looks good to me. I marked the patch as "Ready for Committer".
Unless there's an objection, especially from Robert or Thomas Munro, I
am going to commit/push it.

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


Re: [HACKERS] [PATCH] Lockable views

From
Andres Freund
Date:
Hi,

On 2018-03-28 20:26:48 +0900, Yugo Nagata wrote:
> diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
> index b2c7203..96d477c 100644
> --- a/doc/src/sgml/ref/lock.sgml
> +++ b/doc/src/sgml/ref/lock.sgml
> @@ -46,6 +46,11 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
>    </para>
>  
>    <para>
> +   When a view is specified to be locked, all relations appearing in the view
> +   definition query are also locked recursively with the same lock mode. 
> +  </para>

Trailing space added. I'd remove "specified to be" from the sentence.

I think mentioning how this interacts with permissions would be a good
idea too. Given how operations use the view's owner to recurse, that's
not obvious. Should also explain what permissions are required to do the
operation on the view.


> @@ -86,15 +92,17 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
>          return;                    /* woops, concurrently dropped; no permissions
>                                   * check */
>  
> -    /* Currently, we only allow plain tables to be locked */
> -    if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
> +

This newline looks spurious to me.


>  /*
> + * Apply LOCK TABLE recursively over a view
> + *
> + * All tables and views appearing in the view definition query are locked
> + * recursively with the same lock mode.
> + */
> +
> +typedef struct
> +{
> +    Oid root_reloid;
> +    LOCKMODE lockmode;
> +    bool nowait;
> +    Oid viewowner;
> +    Oid viewoid;
> +} LockViewRecurse_context;

Probably wouldn't hurt to pgindent the larger changes in the patch.


> +static bool
> +LockViewRecurse_walker(Node *node, LockViewRecurse_context *context)
> +{
> +    if (node == NULL)
> +        return false;
> +
> +    if (IsA(node, Query))
> +    {
> +        Query        *query = (Query *) node;
> +        ListCell    *rtable;
> +
> +        foreach(rtable, query->rtable)
> +        {
> +            RangeTblEntry    *rte = lfirst(rtable);
> +            AclResult         aclresult;
> +
> +            Oid relid = rte->relid;
> +            char relkind = rte->relkind;
> +            char *relname = get_rel_name(relid);
> +
> +            /* The OLD and NEW placeholder entries in the view's rtable are skipped. */
> +            if (relid == context->viewoid &&
> +                (!strcmp(rte->eref->aliasname, "old") || !strcmp(rte->eref->aliasname, "new")))
> +                continue;
> +
> +            /* Currently, we only allow plain tables or views to be locked. */
> +            if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE &&
> +                relkind != RELKIND_VIEW)
> +                continue;
> +
> +            /* Check infinite recursion in the view definition. */
> +            if (relid == context->root_reloid)
> +                ereport(ERROR,
> +                        (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> +                        errmsg("infinite recursion detected in rules for relation \"%s\"",
> +                                get_rel_name(context->root_reloid))));

Hm, how can that happen? And if it can happen, why can it only happen
with the root relation?

Greetings,

Andres Freund


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
Andres,

I have just pushed the v10 patch. Yugo will reply back to your point
but I will look into your review as well.

Thanks.

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

> Hi,
> 
> On 2018-03-28 20:26:48 +0900, Yugo Nagata wrote:
>> diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
>> index b2c7203..96d477c 100644
>> --- a/doc/src/sgml/ref/lock.sgml
>> +++ b/doc/src/sgml/ref/lock.sgml
>> @@ -46,6 +46,11 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
>>    </para>
>>  
>>    <para>
>> +   When a view is specified to be locked, all relations appearing in the view
>> +   definition query are also locked recursively with the same lock mode. 
>> +  </para>
> 
> Trailing space added. I'd remove "specified to be" from the sentence.
> 
> I think mentioning how this interacts with permissions would be a good
> idea too. Given how operations use the view's owner to recurse, that's
> not obvious. Should also explain what permissions are required to do the
> operation on the view.
> 
> 
>> @@ -86,15 +92,17 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
>>          return;                    /* woops, concurrently dropped; no permissions
>>                                   * check */
>>  
>> -    /* Currently, we only allow plain tables to be locked */
>> -    if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
>> +
> 
> This newline looks spurious to me.
> 
> 
>>  /*
>> + * Apply LOCK TABLE recursively over a view
>> + *
>> + * All tables and views appearing in the view definition query are locked
>> + * recursively with the same lock mode.
>> + */
>> +
>> +typedef struct
>> +{
>> +    Oid root_reloid;
>> +    LOCKMODE lockmode;
>> +    bool nowait;
>> +    Oid viewowner;
>> +    Oid viewoid;
>> +} LockViewRecurse_context;
> 
> Probably wouldn't hurt to pgindent the larger changes in the patch.
> 
> 
>> +static bool
>> +LockViewRecurse_walker(Node *node, LockViewRecurse_context *context)
>> +{
>> +    if (node == NULL)
>> +        return false;
>> +
>> +    if (IsA(node, Query))
>> +    {
>> +        Query        *query = (Query *) node;
>> +        ListCell    *rtable;
>> +
>> +        foreach(rtable, query->rtable)
>> +        {
>> +            RangeTblEntry    *rte = lfirst(rtable);
>> +            AclResult         aclresult;
>> +
>> +            Oid relid = rte->relid;
>> +            char relkind = rte->relkind;
>> +            char *relname = get_rel_name(relid);
>> +
>> +            /* The OLD and NEW placeholder entries in the view's rtable are skipped. */
>> +            if (relid == context->viewoid &&
>> +                (!strcmp(rte->eref->aliasname, "old") || !strcmp(rte->eref->aliasname, "new")))
>> +                continue;
>> +
>> +            /* Currently, we only allow plain tables or views to be locked. */
>> +            if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE &&
>> +                relkind != RELKIND_VIEW)
>> +                continue;
>> +
>> +            /* Check infinite recursion in the view definition. */
>> +            if (relid == context->root_reloid)
>> +                ereport(ERROR,
>> +                        (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
>> +                        errmsg("infinite recursion detected in rules for relation \"%s\"",
>> +                                get_rel_name(context->root_reloid))));
> 
> Hm, how can that happen? And if it can happen, why can it only happen
> with the root relation?
> 
> Greetings,
> 
> Andres Freund


Re: [HACKERS] [PATCH] Lockable views

From
Tom Lane
Date:
Tatsuo Ishii <ishii@sraoss.co.jp> writes:
> I have just pushed the v10 patch.

The buildfarm is fairly unhappy, and I think it's because of this patch.

            regards, tom lane


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> I have just pushed the v10 patch.
> 
> The buildfarm is fairly unhappy, and I think it's because of this patch.

Thanks for the info. Yes, at least prion is unhappy because of the
patch. I will look into this.

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


Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> The buildfarm is fairly unhappy, and I think it's because of this patch.
> 
> Thanks for the info. Yes, at least prion is unhappy because of the
> patch. I will look into this.

Done. See if the buildarm becomes happy.

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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Thu, 29 Mar 2018 17:26:36 -0700
Andres Freund <andres@anarazel.de> wrote:

Thank you for your comments. I attach a patch to fix issues
you've pointed out.

> Hi,
> 
> On 2018-03-28 20:26:48 +0900, Yugo Nagata wrote:
> > diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
> > index b2c7203..96d477c 100644
> > --- a/doc/src/sgml/ref/lock.sgml
> > +++ b/doc/src/sgml/ref/lock.sgml
> > @@ -46,6 +46,11 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
> >    </para>
> >  
> >    <para>
> > +   When a view is specified to be locked, all relations appearing in the view
> > +   definition query are also locked recursively with the same lock mode. 
> > +  </para>
> 
> Trailing space added. I'd remove "specified to be" from the sentence.

Fixed.

> 
> I think mentioning how this interacts with permissions would be a good
> idea too. Given how operations use the view's owner to recurse, that's
> not obvious. Should also explain what permissions are required to do the
> operation on the view.

Added a description about permissions into the documentation.

> 
> 
> > @@ -86,15 +92,17 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
> >          return;                    /* woops, concurrently dropped; no permissions
> >                                   * check */
> >  
> > -    /* Currently, we only allow plain tables to be locked */
> > -    if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
> > +
> 
> This newline looks spurious to me.

Removed.

> 
> 
> >  /*
> > + * Apply LOCK TABLE recursively over a view
> > + *
> > + * All tables and views appearing in the view definition query are locked
> > + * recursively with the same lock mode.
> > + */
> > +
> > +typedef struct
> > +{
> > +    Oid root_reloid;
> > +    LOCKMODE lockmode;
> > +    bool nowait;
> > +    Oid viewowner;
> > +    Oid viewoid;
> > +} LockViewRecurse_context;
> 
> Probably wouldn't hurt to pgindent the larger changes in the patch.
> 
> 
> > +static bool
> > +LockViewRecurse_walker(Node *node, LockViewRecurse_context *context)
> > +{
> > +    if (node == NULL)
> > +        return false;
> > +
> > +    if (IsA(node, Query))
> > +    {
> > +        Query        *query = (Query *) node;
> > +        ListCell    *rtable;
> > +
> > +        foreach(rtable, query->rtable)
> > +        {
> > +            RangeTblEntry    *rte = lfirst(rtable);
> > +            AclResult         aclresult;
> > +
> > +            Oid relid = rte->relid;
> > +            char relkind = rte->relkind;
> > +            char *relname = get_rel_name(relid);
> > +
> > +            /* The OLD and NEW placeholder entries in the view's rtable are skipped. */
> > +            if (relid == context->viewoid &&
> > +                (!strcmp(rte->eref->aliasname, "old") || !strcmp(rte->eref->aliasname, "new")))
> > +                continue;
> > +
> > +            /* Currently, we only allow plain tables or views to be locked. */
> > +            if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE &&
> > +                relkind != RELKIND_VIEW)
> > +                continue;
> > +
> > +            /* Check infinite recursion in the view definition. */
> > +            if (relid == context->root_reloid)
> > +                ereport(ERROR,
> > +                        (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> > +                        errmsg("infinite recursion detected in rules for relation \"%s\"",
> > +                                get_rel_name(context->root_reloid))));
> 
> Hm, how can that happen? And if it can happen, why can it only happen
> with the root relation?

For example, the following queries cause the infinite recursion of views. 
This is detected and the error is raised.

 create table t (i int);
 create view v1 as select 1;
 create view v2 as select * from v1;
 create or replace view v1 as select * from v2;
 begin;
 lock v1;
 abort;

However, I found that the previous patch could not handle the following
situation in which the root relation itself doesn't have infinite recursion.

 create view v3 as select * from v1;
 begin;
 lock v3;
 abort;

This fixed in the attached patch.

Regards,

> 
> Greetings,
> 
> Andres Freund


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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Mon, 2 Apr 2018 18:32:53 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

> On Thu, 29 Mar 2018 17:26:36 -0700
> Andres Freund <andres@anarazel.de> wrote:
> 
> Thank you for your comments. I attach a patch to fix issues
> you've pointed out.

I found a typo in the documentation and attach the updated patch.

Regards,

> 
> > Hi,
> > 
> > On 2018-03-28 20:26:48 +0900, Yugo Nagata wrote:
> > > diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
> > > index b2c7203..96d477c 100644
> > > --- a/doc/src/sgml/ref/lock.sgml
> > > +++ b/doc/src/sgml/ref/lock.sgml
> > > @@ -46,6 +46,11 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
> > >    </para>
> > >  
> > >    <para>
> > > +   When a view is specified to be locked, all relations appearing in the view
> > > +   definition query are also locked recursively with the same lock mode. 
> > > +  </para>
> > 
> > Trailing space added. I'd remove "specified to be" from the sentence.
> 
> Fixed.
> 
> > 
> > I think mentioning how this interacts with permissions would be a good
> > idea too. Given how operations use the view's owner to recurse, that's
> > not obvious. Should also explain what permissions are required to do the
> > operation on the view.
> 
> Added a description about permissions into the documentation.
> 
> > 
> > 
> > > @@ -86,15 +92,17 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
> > >          return;                    /* woops, concurrently dropped; no permissions
> > >                                   * check */
> > >  
> > > -    /* Currently, we only allow plain tables to be locked */
> > > -    if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
> > > +
> > 
> > This newline looks spurious to me.
> 
> Removed.
> 
> > 
> > 
> > >  /*
> > > + * Apply LOCK TABLE recursively over a view
> > > + *
> > > + * All tables and views appearing in the view definition query are locked
> > > + * recursively with the same lock mode.
> > > + */
> > > +
> > > +typedef struct
> > > +{
> > > +    Oid root_reloid;
> > > +    LOCKMODE lockmode;
> > > +    bool nowait;
> > > +    Oid viewowner;
> > > +    Oid viewoid;
> > > +} LockViewRecurse_context;
> > 
> > Probably wouldn't hurt to pgindent the larger changes in the patch.
> > 
> > 
> > > +static bool
> > > +LockViewRecurse_walker(Node *node, LockViewRecurse_context *context)
> > > +{
> > > +    if (node == NULL)
> > > +        return false;
> > > +
> > > +    if (IsA(node, Query))
> > > +    {
> > > +        Query        *query = (Query *) node;
> > > +        ListCell    *rtable;
> > > +
> > > +        foreach(rtable, query->rtable)
> > > +        {
> > > +            RangeTblEntry    *rte = lfirst(rtable);
> > > +            AclResult         aclresult;
> > > +
> > > +            Oid relid = rte->relid;
> > > +            char relkind = rte->relkind;
> > > +            char *relname = get_rel_name(relid);
> > > +
> > > +            /* The OLD and NEW placeholder entries in the view's rtable are skipped. */
> > > +            if (relid == context->viewoid &&
> > > +                (!strcmp(rte->eref->aliasname, "old") || !strcmp(rte->eref->aliasname, "new")))
> > > +                continue;
> > > +
> > > +            /* Currently, we only allow plain tables or views to be locked. */
> > > +            if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE &&
> > > +                relkind != RELKIND_VIEW)
> > > +                continue;
> > > +
> > > +            /* Check infinite recursion in the view definition. */
> > > +            if (relid == context->root_reloid)
> > > +                ereport(ERROR,
> > > +                        (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> > > +                        errmsg("infinite recursion detected in rules for relation \"%s\"",
> > > +                                get_rel_name(context->root_reloid))));
> > 
> > Hm, how can that happen? And if it can happen, why can it only happen
> > with the root relation?
> 
> For example, the following queries cause the infinite recursion of views. 
> This is detected and the error is raised.
> 
>  create table t (i int);
>  create view v1 as select 1;
>  create view v2 as select * from v1;
>  create or replace view v1 as select * from v2;
>  begin;
>  lock v1;
>  abort;
> 
> However, I found that the previous patch could not handle the following
> situation in which the root relation itself doesn't have infinite recursion.
> 
>  create view v3 as select * from v1;
>  begin;
>  lock v3;
>  abort;
> 
> This fixed in the attached patch.
> 
> Regards,
> 
> > 
> > Greetings,
> > 
> > Andres Freund
> 
> 
> -- 
> Yugo Nagata <nagata@sraoss.co.jp>


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

Attachment

Re: [HACKERS] [PATCH] Lockable views

From
Tatsuo Ishii
Date:
>> > > +typedef struct
>> > > +{
>> > > +    Oid root_reloid;
>> > > +    LOCKMODE lockmode;
>> > > +    bool nowait;
>> > > +    Oid viewowner;
>> > > +    Oid viewoid;
>> > > +} LockViewRecurse_context;
>> > 
>> > Probably wouldn't hurt to pgindent the larger changes in the patch.

Yeah. Also, each struct member needs a comment.

>> > Hm, how can that happen? And if it can happen, why can it only happen
>> > with the root relation?
>> 
>> For example, the following queries cause the infinite recursion of views. 
>> This is detected and the error is raised.
>> 
>>  create table t (i int);
>>  create view v1 as select 1;
>>  create view v2 as select * from v1;
>>  create or replace view v1 as select * from v2;
>>  begin;
>>  lock v1;
>>  abort;
>> 
>> However, I found that the previous patch could not handle the following
>> situation in which the root relation itself doesn't have infinite recursion.
>> 
>>  create view v3 as select * from v1;
>>  begin;
>>  lock v3;
>>  abort;

Shouldn't they be in the regression test?

It's shame that create_view test does not include the cases, but it's
another story.

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


Re: [HACKERS] [PATCH] Lockable views

From
Yugo Nagata
Date:
On Thu, 05 Apr 2018 07:53:42 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

I update the patch to fix the lockable view issues.

> >> > > +typedef struct
> >> > > +{
> >> > > +    Oid root_reloid;
> >> > > +    LOCKMODE lockmode;
> >> > > +    bool nowait;
> >> > > +    Oid viewowner;
> >> > > +    Oid viewoid;
> >> > > +} LockViewRecurse_context;
> >> > 
> >> > Probably wouldn't hurt to pgindent the larger changes in the patch.
> 
> Yeah. Also, each struct member needs a comment.

I applied pgindent and added comments to struct members.

> 
> >> > Hm, how can that happen? And if it can happen, why can it only happen
> >> > with the root relation?
> >> 
> >> For example, the following queries cause the infinite recursion of views. 
> >> This is detected and the error is raised.
> >> 
> >>  create table t (i int);
> >>  create view v1 as select 1;
> >>  create view v2 as select * from v1;
> >>  create or replace view v1 as select * from v2;
> >>  begin;
> >>  lock v1;
> >>  abort;
> >> 
> >> However, I found that the previous patch could not handle the following
> >> situation in which the root relation itself doesn't have infinite recursion.
> >> 
> >>  create view v3 as select * from v1;
> >>  begin;
> >>  lock v3;
> >>  abort;
> 
> Shouldn't they be in the regression test?

Added tests for the infinite recursion detection.

Regards,

> 
> It's shame that create_view test does not include the cases, but it's
> another story.
> 
> 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>

Attachment