sql: fix minor bug around disabling portal pausability

I've been reviewing the pausable portal code, and I think I found
a minor bug that doesn't actually appear to have any consequences. What
happens is that if we created a pausable portal, but then it's executed
without a limit (i.e. `MaxRows` is not set on `Execute`), then we
disable pausability since the portal will run to completion. We do that
by modifying a _copy_ of `PreparedPortal` struct that is used throughout
the execution, yet we don't update the map that stores all portals. Note
that other nested things (like `portalPauseInfo`) are stored by
reference, so modifications to them are reflected in the copy stored in
the map, so I think things just happen to work out. Still, it seems
prudent to update the copy that we store in the map with the fact that
pausability was disabled.

A similar issue is present when we revoke the portal pausability - we
modify the copy of PreparedPortal and don't update the map. Additionally,
here we explicitly reset `planner.pausablePortal` to `nil` when revoking
pausability. I audited all cleanup functions, and they should behave
correctly: namely, we'll have nil'ed out `pauseInfo`, so the portal won't
be considered pausable, and the cleanup will be performed right away,
avoiding the pausauble portal cleanup stacks.

Release note: None

Author: yuzefovich

pkg/sql/conn_executor.go

@@ -2455,7 +2455,7 @@ func (ex *connExecutor) execCmd() (retErr error) {
 			// If this is the first-time execution of a portal without a limit set,
 			// it means all rows will be exhausted, so no need to pause this portal.
 			if tcmd.Limit == 0 && portal.pauseInfo != nil && portal.pauseInfo.curRes == nil {
-				portal.pauseInfo = nil
+				ex.disablePortalPausability(&portal)
 			}
 
 			stmtRes := ex.clientComm.CreateStatementResult(

pkg/sql/conn_executor_exec.go

@@ -2765,7 +2765,8 @@ func (ex *connExecutor) dispatchToExecutionEngine(
 					}
 					// This stmt is not supported via the pausable portals model
 					// - set it back to an un-pausable (normal) portal.
-					planner.pausablePortal.pauseInfo = nil
+					ex.disablePortalPausability(planner.pausablePortal)
+					planner.pausablePortal = nil
 					err = res.RevokePortalPausability()
 					// If this plan is a transaction control statement, we don't
 					// even execute it but just early exit.

pkg/sql/prepared_stmt.go

@@ -82,13 +82,17 @@ const (
 // PreparedPortal is a PreparedStatement that has been bound with query
 // arguments.
 type PreparedPortal struct {
+	// Fields below are initialized when creating the PreparedPortal and aren't
+	// modified later.
 	Name  string
 	Stmt  *prep.Statement
 	Qargs tree.QueryArguments
 
 	// OutFormats contains the requested formats for the output columns.
 	OutFormats []pgwirebase.FormatCode
 
+	// Fields below might be updated throughout the PreparedPortal's lifecycle.
+
 	// exhausted tracks whether this portal has already been fully exhausted,
 	// meaning that any additional attempts to execute it should return no
 	// rows.
@@ -141,6 +145,16 @@ func (ex *connExecutor) makePreparedPortal(
 	return portal, portal.accountForCopy(ctx, &ex.extraTxnState.prepStmtsNamespaceMemAcc, name)
 }
 
+func (ex *connExecutor) disablePortalPausability(portal *PreparedPortal) {
+	portal.portalPausablity = PortalPausabilityDisabled
+	portal.pauseInfo = nil
+	// Since the PreparedPortal is stored by value in the map, we need to
+	// explicitly update it. (Note that PreparedPortal.pauseInfo is stored by
+	// pointer, so modifications to portalPauseInfo will be reflected in the map
+	// automatically.)
+	ex.extraTxnState.prepStmtsNamespace.portals[portal.Name] = *portal
+}
+
 // accountForCopy updates the state to account for the copy of the
 // PreparedPortal (p is the copy).
 func (p *PreparedPortal) accountForCopy(