WIP: SetQuerySnapshot redesign - Mailing list pgsql-patches

The attached patch implements my proposal of 6-Sep to force updating of
the snapshot before every SQL query executed by SQL and SPI functions,
but to provide an "escape hatch" by defining a read-only mode in which
we do neither SetQuerySnapshot nor CommandCounterIncrement, and instead
always use the snapshot that is in force in the calling query.  (Most of
the bulk of the patch has to do with providing a suitably maintained
global ActiveSnapshot, which functions can consult in order to do that.)

Since no one objected, I've made the read-only mode apply automatically
in all non-volatile functions.  We could still talk about driving it off
some other function property, though.

Note that SetQuerySnapshot itself is gone; I took the opportunity to
make the tqual.c API a bit more orthogonal.

I have not yet finished updating the documentation, but this addition to
the regression tests may prove illuminating:

*** src/test/regress/expected/transactions.out.orig    Mon Sep  6 13:46:34 2004
--- src/test/regress/expected/transactions.out    Sun Sep 12 17:27:18 2004
***************
*** 374,379 ****
--- 374,457 ----
      FETCH 10 FROM c;
  ERROR:  portal "c" cannot be run
  COMMIT;
+ --
+ -- Check that "stable" functions are really stable.  They should not be
+ -- able to see the partial results of the calling query.  (Ideally we would
+ -- also check that they don't see commits of concurrent transactions, but
+ -- that's a mite hard to do within the limitations of pg_regress.)
+ --
+ select * from xacttest;
+   a  |    b
+ -----+---------
+   56 |     7.8
+  100 |  99.097
+    0 | 0.09561
+   42 |  324.78
+  777 | 777.777
+ (5 rows)
+
+ create or replace function max_xacttest() returns smallint language sql as
+ 'select max(a) from xacttest' stable;
+ begin;
+ update xacttest set a = max_xacttest() + 10 where a > 0;
+ select * from xacttest;
+   a  |    b
+ -----+---------
+    0 | 0.09561
+  787 |     7.8
+  787 |  99.097
+  787 |  324.78
+  787 | 777.777
+ (5 rows)
+
+ rollback;
+ -- But a volatile function can see the partial results of the calling query
+ create or replace function max_xacttest() returns smallint language sql as
+ 'select max(a) from xacttest' volatile;
+ begin;
+ update xacttest set a = max_xacttest() + 10 where a > 0;
+ select * from xacttest;
+   a  |    b
+ -----+---------
+    0 | 0.09561
+  787 |     7.8
+  797 |  99.097
+  807 |  324.78
+  817 | 777.777
+ (5 rows)
+
+ rollback;
+ -- Now the same test with plpgsql (since it depends on SPI which is different)
+ create or replace function max_xacttest() returns smallint language plpgsql as
+ 'begin return max(a) from xacttest; end' stable;
+ begin;
+ update xacttest set a = max_xacttest() + 10 where a > 0;
+ select * from xacttest;
+   a  |    b
+ -----+---------
+    0 | 0.09561
+  787 |     7.8
+  787 |  99.097
+  787 |  324.78
+  787 | 777.777
+ (5 rows)
+
+ rollback;
+ create or replace function max_xacttest() returns smallint language plpgsql as
+ 'begin return max(a) from xacttest; end' volatile;
+ begin;
+ update xacttest set a = max_xacttest() + 10 where a > 0;
+ select * from xacttest;
+   a  |    b
+ -----+---------
+    0 | 0.09561
+  787 |     7.8
+  797 |  99.097
+  807 |  324.78
+  817 | 777.777
+ (5 rows)
+
+ rollback;
  -- test case for problems with dropping an open relation during abort
  BEGIN;
      savepoint x;

It may be worth noting that in 7.4, the SQL version of the function acts
stable (ie, the output matches the first case above) while the plpgsql
version acts volatile.  This is because of random differences in the
placement of CommandCounterIncrement calls, in particular the fact that
functions.c didn't do a CCI after the last query of a SQL function.  If
you change the function definition to

create or replace function max_xacttest() returns smallint language sql as
'select 1; select max(a) from xacttest' stable;

then it starts acting volatile in 7.4, because CCIs are happening.

Comments?

            regards, tom lane