SPI and CommandCounterIncrement, redux - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | SPI and CommandCounterIncrement, redux |
Date | |
Msg-id | 11330.1006296063@sss.pgh.pa.us Whole thread Raw |
List | pgsql-hackers |
Back in August we had a discussion about whether SPI isn't broken in its handling of CommandCounterIncrement:http://fts.postgresql.org/db/mw/msg.html?mid=1029236 That discussion tailed off without any agreement what to do, but I've just been reminded of the problem, and I now understand that there's a separate and (I think) easily-fixable bug besides the larger issue. Moreover, this bug represents a nasty regression from 7.1, so I think we *must* do something about it now. The example I've just been looking at (courtesy of Patrick MacDonald) is create table table1 (name text); create or replace function failing () returns integer as ' declare counter integer; thisRow record; tmpName1 text; tmpName2 text; begin select count(*) into counter from table1; raise notice ''First count is: %'', counter; tmpName1 = ''name'' || counter + 1; tmpName2 = ''name'' || counter + 2; -- insert two values insert into table1 values (tmpName1); insert into table1 values (tmpName2); -- loop through the values and display name for thisRow in select * from table1 loop raise notice ''Name:: %'', thisRow.name; end loop; select count(*) into counter from table1; raise notice ''New count is: %'', counter; -- the last name displayed should be the last -- name inserted raisenotice ''Last name should be: %'', tmpName2; return counter; end; ' language 'plpgsql'; In current sources, this function behaves correctly the first time you invoke it (in a given backend), and incorrectly on subsequent uses: regression=# select failing(); NOTICE: First count is: 0 NOTICE: Name :: name1 NOTICE: Name :: name2 NOTICE: New count is: 2 NOTICE: Last name should be: name2failing --------- 2 (1 row) regression=# select failing(); NOTICE: First count is: 2 NOTICE: Name :: name1 NOTICE: Name :: name2 NOTICE: Name :: name3 NOTICE: New count is: 4 NOTICE: Last name should be: name4failing --------- 4 (1 row) regression=# Note the failure to display "Name :: name4". The problem is that no CommandCounterIncrement happens between the second INSERT and the FOR ... SELECT, so the inserted row is considered not yet visible. This worked correctly in 7.1. It fails in current sources because plpgsql's FOR is now based on SPI cursor support, and there is no CommandCounterIncrement in SPI_cursor_open, whereas there is one in SPI_exec and SPI_execp. (But the first time through, a CommandCounterIncrement occurs as a side effect of query planning for the SELECT, so the problem is masked.) I believe an appropriate fix is to add a CommandCounterIncrement call near the start of SPI_cursor_open; this will make it behave similarly to the other SPI query-execution calls. I have verified that this change fixes Patrick's example, as well as the first-call- vs-later-calls discrepancies in the examples I posted in August. And it doesn't break the regression tests. However, this quick fix does not address the larger issue of whether we need to try to make the CommandCounterIncrement-ing behavior consistent between the cases where plpgsql has to generate a query plan and the cases where it's already got one cached. I think we're going to have to come back and revisit that in a future release cycle. Since this isn't a complete fix, I thought I'd better run it by pgsql-hackers to see if anyone has a problem with it or sees a better quick fix. Any comments out there? regards, tom lane
pgsql-hackers by date: