sql,plpgsql: add setting to give cursors default WITH HOLD behavior

[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.

[cockroach-teamcity]

This change is 

[rharding6373]

Nice work! I only have nits.

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)

---

pkg/sql/conn_executor.go line 2009 at r2 (raw file):

	// Close all (non HOLD) cursors.
	var closeReason cursorCloseReason

optional nit: could just initialize to cursorCloseForTxnRollback and eliminate the else statement (or default to curoseCloseForTxnCommit, which seems more intuitive).

---

pkg/sql/planner.go line 208 at r2 (raw file):

	monitor *mon.BytesMonitor

	// sessionMonitor tracks the memory of session-bound objects. It currently

minor nit: s/It currently/It is currently

---

pkg/sql/routine.go line 455 at r2 (raw file):

func (g *routineGenerator) newCursorHelper(
	plan *planComponents, sql string,

Why do we need this new argument? It seems like our only use case is to grab CursorSQL from g.expr.CursorDeclaration, but since we already access the latter in the function, could we just use open.sql instead of passing in a new arg?

---

pkg/sql/sessiondatapb/local_only_session_data.proto line 484 at r2 (raw file):

  // CloseCursorsAtCommit determines whether cursors remain open after their
  // parent transaction closes.
  bool close_cursors_at_commit = 121;

If we do eventually get to inline PL/pgSQL UDFs, will this need to propagate to remote nodes? I think the answer is no, but just double checking before putting it in the local session proto.

[DrewKimball]

TFTR!

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @rharding6373)

---

pkg/sql/conn_executor.go line 2009 at r2 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

optional nit: could just initialize to cursorCloseForTxnRollback and eliminate the else statement (or default to curoseCloseForTxnCommit, which seems more intuitive).

Done.

---

pkg/sql/planner.go line 208 at r2 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

minor nit: s/It currently/It is currently

Done.

---

pkg/sql/routine.go line 455 at r2 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

Why do we need this new argument? It seems like our only use case is to grab CursorSQL from g.expr.CursorDeclaration, but since we already access the latter in the function, could we just use open.sql instead of passing in a new arg?

Good point, Done.

---

pkg/sql/sessiondatapb/local_only_session_data.proto line 484 at r2 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

If we do eventually get to inline PL/pgSQL UDFs, will this need to propagate to remote nodes? I think the answer is no, but just double checking before putting it in the local session proto.

We intentionally don't inline a PL/pgSQL sub-routine that opens a cursor, so that shouldn't be a problem.

[DrewKimball]

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)