Re: Modify the DECLARE CURSOR command tag depending on the scrollable flag

2013-11-27 19:16 keltezéssel, Tom Lane írta:
> Boszormenyi Zoltan <zb(at)cybertec(dot)at> writes:
>> 2013-11-23 22:01 keltezéssel, Tom Lane írta:
>>> Boszormenyi Zoltan <zb(at)cybertec(dot)at> writes:
>>>> Attached is the patch that modified the command tag returned by
>>>> the DECLARE CURSOR command. It returns "DECLARE SCROLL CURSOR"
>>>> or "DECLARE NO SCROLL CURSOR" depending on the cursor's
>>>> scrollable flag that can be determined internally even if neither is
>>>> asked explicitly.
>>> This does not strike me as an acceptable change. It will break any code
>>> that's expecting the existing command tag, for little or no benefit
>>> to most applications. Even if backwards compatibility were of no concern,
>>> I'm not convinced it's a good thing to expose the backend's internal
>>> choices about query plans used for cursors, which is what this is
>>> basically doing.
>> I saw code in the backend allowing a cursor to be scrollable, although
>> it was not declared as such. How about ripping that out?
> That also fails the unnecessary-backwards-compatibility-break test.

If you read the rest of the mail, it turns out it wasn't a serious question.

Getting the SCROLL / NO SCROLL flags from the preprocessor is no problem.
The only problem is when it's unspecified.

Treating it as NO SCROLL (or adding it to the DECLARE command behind
the application's back) would break apps that want to scroll backward.
(Not ABSOLUTE.)

On the other hand, what problems would one face when adding SCROLL
implicitly if it's unspecified? It's not a workable solution either, see below.

As the documentation suggests, an application that wants to use
UPDATE/DELETE WHERE CURRENT OF ..., it's highly recommended
that the cursor is for a FOR UPDATE query. Watch this scenario:

zozo=> begin;
BEGIN
zozo=> declare mycur cursor for select * from t1 for update;
DECLARE CURSOR
zozo=> fetch from mycur;
id | t
----+---
1 | a
(1 row)

zozo=> fetch from mycur;
id | t
----+---
2 | b
(1 row)

zozo=> update t1 set t=t||'_x' where current of mycur;
UPDATE 1
zozo=> fetch from mycur;
id | t
----+---
3 | c
(1 row)

zozo=> delete from t1 where current of mycur;
DELETE 1
zozo=> move absolute 0 in mycur;
MOVE 0
zozo=> fetch from mycur;
id | t
----+---
1 | a
(1 row)

zozo=> fetch from mycur;
id | t
----+---
(0 rows)

Although the server complains about MOVE BACKWARD, it's not
complaining about MOVE ABSOLUTE, despite it's clearly moving
backward. The cursor position is tracked in the backend in a long
variable and it's not overflowed. This is also legacy behaviour,
changing it would break backward compatibility.

The other problem I see is with the documentation: it says that
the INSENSITIVE keyword is just a placeholder, all cursors are insensitive.
It's clearly false. Moving back to the start, previously existing rows
won't show up again. It's not strictly a sensitive cursor, either,
because the row with id=2 would show up with the new value of "t".
This neither sensitive nor insensitive behaviour is what the SQL
standard calls an "asensitive" cursor. It would worth a doc change.
This is what's written in 9.3:

"
If the cursor's query includes FOR UPDATE or FOR SHARE, then returned rows are locked at
the time they are first fetched, in the same way as for a regular SELECT
<http://www.postgresql.org/docs/9.3/interactive/sql-select.html> command with these
options. In addition, the returned rows will be the most up-to-date versions; therefore
these options provide the equivalent of what the SQL standard calls a "sensitive cursor".
(Specifying INSENSITIVE together with FOR UPDATE or FOR SHARE is an error.)
"
( http://www.postgresql.org/docs/9.3/interactive/sql-declare.html )

However, if the cursor is declared without FOR UPDATE, both
the explicit SCROLL keyword (or implicit, if the query is simple),
scrolling backward and DML with WHERE CURRENT OF are allowed.
In this case, the cursor is really insensitive, FETCH statements
after MOVE ABSOLUTE 0 return all rows with their original data.

This is just to show that adding SCROLL behind the application's
back is also pointless. If the query (which can also be a prepared
statement in ECPG) contains FOR UPDATE, adding SCROLL to the
DECLARE statement would make it fail.

If you consider all these:

- certain combinations of query and DECLARE stmt flags fail;
- adding NO SCROLL is breaking backward compatibility;
- the readahead code has to really know whether the cursor is
scrollable so it can behave just like the server;

then returning the SCROLL / NO SCROLL flag in the command tag is
not a bad solution in my view. In fact, this was the only workable
solution I could come up with to make it work reliably when neither
SCROLL nor NO SCROLL is specified by the application.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/