-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sql: support DECLARE cursor WITH HOLD #77101
Comments
@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. |
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. |
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. |
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? |
it appears to |
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? |
That works for me, Rafi. |
94127: sql: allow cursor WITH HOLD inside of txn as long as it gets closed r=otan a=rafiss informs #77101 The `DECLARE foo CURSOR WITH HOLD ...` syntax is now supported in a limited fashion. Unlike PG's `WITH HOLD`, this version can only be executed in a transaction block still. If there are any open cursors that have `WITH HOLD` open at the time a transaction is commited, the COMMIT fails. No release note, since there's not actually any new functionality. Release note: None 94244: sql: add comment for TableIsHydrated and ColumnIsHydrated r=chengxiong-ruan a=chengxiong-ruan Epic: None Release note: None Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com> Co-authored-by: Chengxiong Ruan <chengxiongruan@gmail.com>
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 |
This patch introduces a new setting, `close_cursors_at_commit`, which causes cursors opened using a PL/pgSQL OPEN statement to remain open once the calling transaction commits. This is similar to oracle behavior, and will be useful for enabling migrations. As part of this change, the `sqlCursors.closeAll` method has been expanded to take in the reason for closing as a parameter. It now uses the following rules: * If the reason for closing is txn commit, non-HOLD cursors are closed. * If the reason for closing is txn rollback, all cursors created by the current transaction are closed. * If the reason for closing is an explicit CLOSE ALL or the session closing, all cursors are closed unconditionally. Note that the behavior has not changed for SQL cursors - if a SQL cursor is declared using `WITH HOLD` and is open at txn commit, an error will result. Informs cockroachdb#77101 Release note (sql change): Introduced a new setting, `close_cursors_at_commit`, which causes a cursor to remain open even after its calling transaction commits. Note that transaction rollback still closes any cursor created in that transaction.
This patch introduces a new setting, `close_cursors_at_commit`, which causes cursors opened using a PL/pgSQL OPEN statement to remain open once the calling transaction commits. This is similar to oracle behavior, and will be useful for enabling migrations. As part of this change, the `sqlCursors.closeAll` method has been expanded to take in the reason for closing as a parameter. It now uses the following rules: * If the reason for closing is txn commit, non-HOLD cursors are closed. * If the reason for closing is txn rollback, all cursors created by the current transaction are closed. * If the reason for closing is an explicit CLOSE ALL or the session closing, all cursors are closed unconditionally. Note that the behavior has not changed for SQL cursors - if a SQL cursor is declared using `WITH HOLD` and is open at txn commit, an error will result. Informs cockroachdb#77101 Release note (sql change): Introduced a new setting, `close_cursors_at_commit`, which causes a cursor to remain open even after its calling transaction commits. Note that transaction rollback still closes any cursor created in that transaction.
This patch introduces a new setting, `close_cursors_at_commit`, which causes cursors opened using a PL/pgSQL OPEN statement to remain open once the calling transaction commits. This is similar to oracle behavior, and will be useful for enabling migrations. As part of this change, the `sqlCursors.closeAll` method has been expanded to take in the reason for closing as a parameter. It now uses the following rules: * If the reason for closing is txn commit, non-HOLD cursors are closed. * If the reason for closing is txn rollback, all cursors created by the current transaction are closed. * If the reason for closing is an explicit CLOSE ALL or the session closing, all cursors are closed unconditionally. Note that the behavior has not changed for SQL cursors - if a SQL cursor is declared using `WITH HOLD` and is open at txn commit, an error will result. Informs cockroachdb#77101 Release note (sql change): Introduced a new setting, `close_cursors_at_commit`, which causes a cursor to remain open even after its calling transaction commits. Note that transaction rollback still closes any cursor created in that transaction.
117910: sql,plpgsql: add setting to give cursors default WITH HOLD behavior r=DrewKimball a=DrewKimball This patch introduces a new setting, `close_cursors_at_commit`, which causes cursors opened using a PL/pgSQL OPEN statement to remain open once the calling transaction commits. This is similar to oracle behavior, and will be useful for enabling migrations. As part of this change, the `sqlCursors.closeAll` method has been expanded to take in the reason for closing as a parameter. It now uses the following rules: * If the reason for closing is txn commit, non-HOLD cursors are closed. * If the reason for closing is txn rollback, all cursors created by the current transaction are closed. * If the reason for closing is an explicit CLOSE ALL or the session closing, all cursors are closed unconditionally. Note that the behavior has not changed for SQL cursors - if a SQL cursor is declared using `WITH HOLD` and is open at txn commit, an error will result. Informs #77101 Release note (sql change): Introduced a new setting, `close_cursors_at_commit`, which causes a cursor to remain open even after its calling transaction commits. Note that transaction rollback still closes any cursor created in that transaction. Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
This patch introduces a new setting, `close_cursors_at_commit`, which causes cursors opened using a PL/pgSQL OPEN statement to remain open once the calling transaction commits. This is similar to oracle behavior, and will be useful for enabling migrations. As part of this change, the `sqlCursors.closeAll` method has been expanded to take in the reason for closing as a parameter. It now uses the following rules: * If the reason for closing is txn commit, non-HOLD cursors are closed. * If the reason for closing is txn rollback, all cursors created by the current transaction are closed. * If the reason for closing is an explicit CLOSE ALL or the session closing, all cursors are closed unconditionally. Note that the behavior has not changed for SQL cursors - if a SQL cursor is declared using `WITH HOLD` and is open at txn commit, an error will result. Informs cockroachdb#77101 Release note (sql change): Introduced a new setting, `close_cursors_at_commit`, which causes a cursor to remain open even after its calling transaction commits. Note that transaction rollback still closes any cursor created in that transaction.
Split off from #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.
Epic CRDB-11397
Jira issue: CRDB-13412
The text was updated successfully, but these errors were encountered: