Обсуждение: INSERT ... SELECT ... FOR SHARED?

Поиск
Список
Период
Сортировка

INSERT ... SELECT ... FOR SHARED?

От
Mark Mielke
Дата:
This is similar to a previous question I had asked about INSERT ... 
DELETE ...

To be "safe", to archive an existing row, and replace with a new row, I 
believe on must do:
   begin;   row := select ... from XXX where ... for update;   insert into XXX_archived values (row);   ... update or
delete/inserton XXX ...   commit;
 

I am trying to lock the row for update to prevent a concurrent process 
from trying archive the row at the same time.

I tried the following and received an odd error:
   begin;   insert into XXX_archived select ... from XXX where ... for update;   ... update or delete/insert on XXX ...
 commit;  
 

First, if the table doesn't match any rows:

# insert into product_image_archived select * from product_image where 
itemno = 'XXXXXX' for update;
INSERT 0 0

Second, if the table does match a row:

# insert into product_image values ('XXXXXX', 'somepath');
INSERT 0 1
# insert into product_image_archived select * from product_image where 
itemno = 'XXXXXX' for update;
ERROR:  cannot extract system attribute from virtual tuple

Is this supposed to work? Is it an easy thing to fix?

The only difference between the product_image and product_image_archived 
tables, is that product_image has a primary key constraint on the 
product identifier.

I can do it the original way - it just seemed "odd".

Cheers,
mark

-- 
Mark Mielke <mark@mielke.cc>



Re: INSERT ... SELECT ... FOR SHARED?

От
Tom Lane
Дата:
Mark Mielke <mark@mark.mielke.cc> writes:
> # insert into product_image_archived select * from product_image where 
> itemno = 'XXXXXX' for update;
> ERROR:  cannot extract system attribute from virtual tuple

Hm, on an assert-enabled build this actually crashes :-(.  It looks like
I broke the specific case here:

http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/executor/execMain.c.diff?r1=1.280;r2=1.281;f=h

as a result of thinking (in a moment of brain fade) that FOR UPDATE
could only occur within a SELECT.  It works in 8.2.

That's fairly easily fixed --- just move the loop that initializes
ctidAttNo out of the operation == CMD_SELECT case --- but looking around
execMain.c I notice that this isn't the only such assumption.
ExecutePlan() thinks that UPDATE and DELETE can't have any FOR UPDATE
clauses.  That hasn't been true for awhile; consider

update t1 set ... from (select ... from t2 for update) ss where ...;

CVS HEAD will take this, and silently not do the requested locks :-(

While there are a lot of related cases that we can't currently handle,
this one would work if execMain weren't just blindly ignoring the
possibility.  I think we need to rejigger the tests in execMain so
that it either honors the rowmarks or throws error if it can't.

The lack of regression tests covering this area is a bit annoying
at this point.  However, it's hard to see how to test FOR UPDATE
until we get some concurrent-sessions support in psql.
        regards, tom lane


Re: INSERT ... SELECT ... FOR SHARED?

От
Gregory Stark
Дата:
"Tom Lane" <tgl@sss.pgh.pa.us> writes:

> The lack of regression tests covering this area is a bit annoying
> at this point.  However, it's hard to see how to test FOR UPDATE
> until we get some concurrent-sessions support in psql.

Well, end-to-end testing would e better but we can test that the locks are
at least being recorded by white-box inspection:

postgres=# begin;
BEGIN
postgres=# select * from tellers for share;tid | bid | tbalance | filler 
-----+-----+----------+--------  1 |   1 |        0 |   2 |   1 |        0 |   3 |   1 |        0 |   4 |   1 |
0|   5 |   1 |        0 |   6 |   1 |        0 |   7 |   1 |        0 |   8 |   1 |        0 |   9 |   1 |        0 |
10|   1 |        0 | 
 
(10 rows)

postgres=# select distinct age(xmax) from tellers;age 
-----  0
(1 row)

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services!


Re: INSERT ... SELECT ... FOR SHARED?

От
Tom Lane
Дата:
Gregory Stark <stark@enterprisedb.com> writes:
> "Tom Lane" <tgl@sss.pgh.pa.us> writes:
>> The lack of regression tests covering this area is a bit annoying
>> at this point.  However, it's hard to see how to test FOR UPDATE
>> until we get some concurrent-sessions support in psql.

> Well, end-to-end testing would e better but we can test that the locks are
> at least being recorded by white-box inspection:

> postgres=# select distinct age(xmax) from tellers;

Huh, that's a cute trick ... and it would answer the immediate problem,
which is to check that the statement did take the row locks it was
supposed to.
        regards, tom lane