Thread: SPI and CommandCounterIncrement, redux

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

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

I wonder why the insert does not do the CommandCounterIncrement,
since it is a statement that did modification and all subsequent 
statements should see the effect ?
In other places this is the usual practice (CommandCounterIncrement
after 
modification), no ?
Or is this a stupid question.

Andreas

"Zeugswetter Andreas SB SD" <ZeugswetterA@spardat.at> writes:
>> 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.

> I wonder why the insert does not do the CommandCounterIncrement,
> since it is a statement that did modification and all subsequent 
> statements should see the effect ?
> In other places this is the usual practice (CommandCounterIncrement
> after modification), no ?

SPI's habit is to do CommandCounterIncrement *before* it starts a query,
rather than after.  I think this makes sense, since otherwise we'd have
to do a CommandCounterIncrement at the start of every function call,
whether the function contained any subqueries or not.
        regards, tom lane