cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30k stars 3.79k forks source link

sql: support DECLARE cursor WITH HOLD #77101

Open rafiss opened 2 years ago

rafiss commented 2 years ago

Split off from https://github.com/cockroachdb/cockroach/issues/41412

WITH HOLD support allows keeping a cursor open for longer than a transaction by writing its results into a buffer.

postgres docs on declare.

Unless WITH HOLD is specified, the cursor created by this command can only be used within the current transaction. Thus, DECLARE without WITH HOLD is useless outside a transaction block: the cursor would survive only to the completion of the statement. Therefore PostgreSQL reports an error if such a command is used outside a transaction block. Use BEGIN and COMMIT (or ROLLBACK) to define a transaction block.

If WITH HOLD is specified and the transaction that created the cursor successfully commits, the cursor can continue to be accessed by subsequent transactions in the same session. (But if the creating transaction is aborted, the cursor is removed.) A cursor created with WITH HOLD is closed when an explicit CLOSE command is issued on it, or the session ends. In the current implementation, the rows represented by a held cursor are copied into a temporary file or memory area so that they remain available for subsequent transactions.

WITH HOLD may not be specified when the query includes FOR UPDATE or FOR SHARE.

Epic CRDB-11397

Jira issue: CRDB-13412

vy-ton commented 2 years ago

User reported running into this unsupported feature with Tibco Spotfire. Looks like it also blocks CRDB as AWS DMS source thread

@rafiss Whats the level of effort here since I see it was separated from original DECLARE issue?

rafiss commented 2 years ago

@jordanlewis has the best understanding at the moment. But my impression is that this shouldn't be too much extra work. Unlike Postgres, which requires an extra buffer (as mentioned in the issue description), I think this will be easier in CRDB since we implemented this using internal executors.

rafiss commented 2 years ago

I just checked with Jordan, who pointed out that it's actually a more significant effort.

Implementation notes: The challenge is that the internal executor is still bound to the transaction. So we can't just close the transaction and keep using the internal executor. We actually would need to create an extra buffer that lives outside the transaction, and also rework the execution logic for fetching from the cursor.

I bet if we read through Postgres source code, we could get some good ideas.

otan commented 1 year ago

~I wonder if we can no-op if we detect this outside a transaction, or error only if a WITH HOLD cursor is held outside the transaction?~ edit: nvm, doesn't help me

DMS seems to declare WITH HOLD all the time, but always use their cursor within the transaction. i wonder if we could have a session var that allows WITH HOLD inside the transaction and on commit/rollback error if we detect any WITH HOLD cursors.

jordanlewis commented 1 year ago

WITH HOLD can be ignored outside of transactions, yes.

That session variable is an interesting idea, though it might not always work well - it's common to leave a cursor not completely closed. Do you know if DMS always completely consumes and closes their cursors?

otan commented 1 year ago

Do you know if DMS always completely consumes and closes their cursors?

it appears to

rafiss commented 1 year ago

i wonder if we could have a session var that allows WITH HOLD inside the transaction and on commit/rollback error if we detect any WITH HOLD cursors.

do we actually need a session variable for this? what about just supporting WITH HOLD, and always returning an error if we detect any WITH HOLD cursors when the txn is closed?

jordanlewis commented 1 year ago

That works for me, Rafi.

DrewKimball commented 9 months ago

It might be nice to also introduce a session setting to control the default behavior of cursor declarations, since oracle has the opposite default from postgres: https://docs.oracle.com/javadb/10.8.3.0/devguide/rdevconceptsholdablecursors.html