sql: add support for holdable cursors#141943
Conversation
This commit refactors `plpgsqlCursorHelper` so that the common logic needed for a cursor stored in a row container now lives on the embedded `persistedCursorHelper`. This will be useful for the following commit that will implement holdable cursors. Informs cockroachdb#77101 Release note: None
|
This change is |
yuzefovich
left a comment
There was a problem hiding this comment.
Nice!
Reviewed 2 of 2 files at r1, 5 of 5 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mw5h)
pkg/sql/sql_cursor.go line 380 at r2 (raw file):
// * The following rules apply only to cursors opened in the current txn: // * txnCommit: close non-holdable cursors, persist holdable cursors. // * txnRollback: close all cursors.
Is this correct? Holdable cursors opened outside of (i.e. before) the current txn shouldn't be closed when the current txn rolls back, right?
This commit adds support for holdable cursors with the following syntax: ``` DECLARE <name> CURSOR WITH HOLD FOR <query>; ``` A holdable cursor is read into a row container upon txn commit, and remains active on the session until it or the session is closed. A transaction rollback drops any cursors opened within that transaction, holdable or not. Rollback does not drop holdable cursors from previous transactions. Prepared transactions cannot create holdable cursors. Fixes cockroachdb#77101 Release note (sql change): Holdable cursors declared using `CURSOR WITH HOLD` are now supported. A holdable cursor fully executes a query upon txn commit and stores the result in a row container, which is maintained by the session.
DrewKimball
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/sql/sql_cursor.go line 380 at r2 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
Is this correct? Holdable cursors opened outside of (i.e. before) the current txn shouldn't be closed when the current txn rolls back, right?
That's right. It's a confusing comment - I'll reword it.
7558729 to
a31c203
Compare
|
I also found that I had to remove the |
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status:
a31c203 to
9716d4e
Compare
|
I'm going to move the third commit to another PR. It's logically distinct, and the changes are going to be more extensive than I'd thought. |
|
bors r=yuzefovich |
|
Build failed: |
|
bors retry |
|
Build failed: |
|
bors retry |
|
Build succeeded: |
sql: refactor persistent cursor helper
This commit refactors
plpgsqlCursorHelperso that the common logicneeded for a cursor stored in a row container now lives on the
embedded
persistedCursorHelper. This will be useful for the followingcommit that will implement holdable cursors.
Informs #77101
Release note: None
sql: support holdable cursors
This commit adds support for holdable cursors with the following syntax:
A holdable cursor is read into a row container upon txn commit, and
remains active on the session until it or the session is closed.
A transaction rollback drops any cursors opened within that transaction,
holdable or not. Rollback does not drop holdable cursors from previous
transactions. Prepared transactions cannot create holdable cursors.
Fixes #77101
Release note (sql change): Holdable cursors declared using
CURSOR WITH HOLDare now supported. A holdable cursor fully executes a query upon txn commit
and stores the result in a row container, which is maintained by the
session.