fix(oracle): recover object column panics on pinned connections

agents/drivers/oracle-go/dbms_output.go

@@ -550,6 +550,9 @@ func (s *server) withStatementDBMSOutput(sqlText string, run func() error) []que
 	s.statementConn = conn
 	defer func() {
 		s.statementConn = previous
+		if repinned := s.takeRepinnedStatementConn(); repinned != nil {
+			_ = repinned.Close()
+		}
 		_ = conn.Close()
 	}()
 	session := s.statementDBMSOutputSession(conn)
@@ -604,6 +607,9 @@ func (s *server) withPagedStatementDBMSOutput(sqlText string, run func() error)
 			// so this function owns it. The statement's own cleanup has already closed
 			// its rows (runPagedOracleSelect defers that), which is what lets the close
 			// below return instead of waiting for a cursor that no one will close.
+			if repinned := s.takeRepinnedStatementConn(); repinned != nil {
+				_ = repinned.Close()
+			}
 			_ = conn.Close()
 			panic(recovered)
 		}
@@ -612,6 +618,12 @@ func (s *server) withPagedStatementDBMSOutput(sqlText string, run func() error)
 	capture := s.ensureDBMSOutput(ctx, session)
 	_ = run()
 	messages := s.finishStatementDBMSOutput(ctx, session, capture, sqlText)
+	if repinned := s.takeRepinnedStatementConn(); repinned != nil {
+		// A driver panic poisoned the pinned connection and the statement's
+		// fallback ran on a replacement, so the page's cursor lives there.
+		_ = conn.Close()
+		conn = repinned
+	}
 	return conn, messages
 }
 

agents/drivers/oracle-go/main.go

@@ -660,6 +660,8 @@ type server struct {
 	statementCtx           context.Context
 	statementCancel        context.CancelFunc
 	statementConn          *sql.Conn
+	statementSchema        string
+	repinnedStatementConn  *sql.Conn
 	plDebugMu              sync.Mutex
 	plDebugSessions        map[string]*plDebugSession
 	plAllocation           *plDebugAllocation
@@ -2028,6 +2030,17 @@ func (s *server) currentSchema() (string, error) {
 		}
 		return strings.ToUpper(schema), nil
 	}
+	// The connection pinned for a statement's DBMS_OUTPUT capture carries the
+	// statement's CURRENT_SCHEMA the same way.
+	if s.statementConn != nil {
+		if err := s.repinPoisonedStatementConn(); err != nil {
+			return "", err
+		}
+		if err := s.statementConn.QueryRowContext(s.statementContext(), sqlText).Scan(&schema); err != nil {
+			return "", err
+		}
+		return strings.ToUpper(schema), nil
+	}
 	db, err := s.requireDB()
 	if err != nil {
 		return "", err
@@ -5133,8 +5146,10 @@ func (s *server) queryRowsWithOracleValueRewriteIfNeeded(sqlText string, timeout
 	// A statement that keeps row locks while it runs is rewritten before the
 	// first attempt: when such a statement panics after the rows were locked,
 	// the locks stay behind and the retry below blocks on them until the query
-	// times out (issues #9344 / #9180).
-	if oracleSQLLocksRows(sqlText) {
+	// times out (issues #9344 / #9180). A manual transaction cannot retry at
+	// all: the panic leaves its session unusable and the transaction is rolled
+	// back (issue #13455).
+	if oracleSQLLocksRows(sqlText) || s.hasManualTransaction() {
 		rewritten, err := rewriteOracleSelectSQL(sqlText, s.loadOracleColumnMeta, deferLOBs)
 		if err == nil && rewritten != sqlText {
 			return s.queryRowsWithTimeout(rewritten, nil, timeoutSecs)
@@ -6131,6 +6146,9 @@ func (s *server) setSchema(schema string) error {
 	}
 	if s.statementConn != nil {
 		_, err := s.statementConn.ExecContext(s.statementContext(), sqlText)
+		if err == nil {
+			s.statementSchema = schema
+		}
 		return err
 	}
 	db, err := s.requireDB()
@@ -6141,6 +6159,46 @@ func (s *server) setSchema(schema string) error {
 	return err
 }
 
+// repinPoisonedStatementConn replaces the statement's pinned connection once a
+// driver panic has poisoned it, so the placeholder fallbacks of the same
+// statement do not fail on the dead session (issue #13455). The fresh
+// connection gets the statement's CURRENT_SCHEMA back; the statement's owner
+// closes it together with the original one.
+func (s *server) repinPoisonedStatementConn() error {
+	if s.manualTx != nil || s.statementConn == nil || !oracleConnPoisoned(s.statementConn) {
+		return nil
+	}
+	db, err := s.requireDB()
+	if err != nil {
+		return err
+	}
+	conn, err := db.Conn(s.statementContext())
+	if err != nil {
+		return err
+	}
+	if s.statementSchema != "" {
+		if _, err := conn.ExecContext(s.statementContext(), "ALTER SESSION SET CURRENT_SCHEMA = "+quoteIdentifier(s.statementSchema)); err != nil {
+			_ = conn.Close()
+			return err
+		}
+	}
+	if s.repinnedStatementConn != nil {
+		_ = s.repinnedStatementConn.Close()
+	}
+	s.repinnedStatementConn = conn
+	s.statementConn = conn
+	return nil
+}
+
+// takeRepinnedStatementConn hands a connection opened by
+// repinPoisonedStatementConn to the statement's owner.
+func (s *server) takeRepinnedStatementConn() *sql.Conn {
+	conn := s.repinnedStatementConn
+	s.repinnedStatementConn = nil
+	s.statementSchema = ""
+	return conn
+}
+
 func (s *server) queryRows(sqlText string, args []any) (*sql.Rows, error) {
 	return s.queryRowsWithTimeout(sqlText, args, 0)
 }
@@ -6149,6 +6207,9 @@ func (s *server) queryRowsWithTimeout(sqlText string, args []any, timeoutSecs in
 	if _, err := s.requireDB(); err != nil {
 		return nil, err
 	}
+	if err := s.repinPoisonedStatementConn(); err != nil {
+		return nil, err
+	}
 	ctx, cancel := context.WithCancel(s.statementContext())
 	var timer *time.Timer
 	if timeoutSecs > 0 {

agents/drivers/oracle-go/main_test.go

@@ -4329,6 +4329,134 @@ func TestOracleQueryRowsKeepsSTAsTextErrorWhenPlaceholderAlsoFails(t *testing.T)
 	}
 }
 
+// oracleViewSourceTxConn lets the scripted driver open the manual transaction
+// the query guard wraps.
+type oracleViewSourceTxConn struct{ *oracleViewSourceConn }
+
+func (c oracleViewSourceTxConn) BeginTx(context.Context, driver.TxOptions) (driver.Tx, error) {
+	return oracleViewSourceTx{}, nil
+}
+
+type oracleViewSourceTx struct{}
+
+func (oracleViewSourceTx) Commit() error   { return nil }
+func (oracleViewSourceTx) Rollback() error { return nil }
+
+type oracleViewSourceTxDriver struct{ scripted *oracleViewSourceDriver }
+
+func (d oracleViewSourceTxDriver) Open(string) (driver.Conn, error) {
+	return oracleViewSourceTxConn{&oracleViewSourceConn{driver: d.scripted}}, nil
+}
+
+func openGuardedOracleViewSourceTestDB(t *testing.T, steps []oracleViewSourceQueryStep) (*sql.DB, *oracleViewSourceDriver) {
+	t.Helper()
+	scripted := &oracleViewSourceDriver{steps: steps}
+	db := sql.OpenDB(oracleQueryConnector{oracleGuardTestConnector{oracleViewSourceTxDriver{scripted}}})
+	t.Cleanup(func() { _ = db.Close() })
+	return db, scripted
+}
+
+func udtPanicFallbackSteps(rows [][]driver.Value) []oracleViewSourceQueryStep {
+	return []oracleViewSourceQueryStep{
+		{exec: true, queryContains: `ALTER SESSION SET CURRENT_SCHEMA = "DBXT"`},
+		{queryContains: "SELECT ID, G FROM T_UDT", args: []driver.Value{}, panicText: "simulated go-ora panic on object column"},
+		// The poisoned pinned connection is replaced and gets the editor schema back.
+		{exec: true, queryContains: `ALTER SESSION SET CURRENT_SCHEMA = "DBXT"`},
+		{queryContains: "CURRENT_SCHEMA", args: []driver.Value{}, columns: []string{"SCHEMA"}, rows: [][]driver.Value{{"DBXT"}}},
+		{
+			queryContains: "ALL_TAB_COLUMNS",
+			args:          []driver.Value{"DBXT", "T_UDT"},
+			columns:       []string{"COLUMN_NAME", "DATA_TYPE", "DATA_TYPE_OWNER"},
+			rows:          [][]driver.Value{{"ID", "NUMBER", nil}, {"G", "GEOM_T", "DBXT"}},
+		},
+		{queryContains: "'<GEOM_T>'", args: []driver.Value{}, columns: []string{"ID", "G"}, rows: rows},
+	}
+}
+
+func TestExecuteQueryRetriesObjectColumnPanicOnFreshPinnedConnection(t *testing.T) {
+	steps := udtPanicFallbackSteps([][]driver.Value{{int64(1), "<GEOM_T>"}})
+	db, scripted := openGuardedOracleViewSourceTestDB(t, steps)
+	s := newServer()
+	s.db = db
+	result, err := s.executeQuery(queryOptions{SQL: "SELECT ID, G FROM T_UDT", Schema: "DBXT", MaxRows: 10})
+	if err != nil {
+		t.Fatalf("expected the placeholder fallback to recover the panic, got %v", err)
+	}
+	if !reflect.DeepEqual(result.Rows, [][]any{{int64(1), "<GEOM_T>"}}) {
+		t.Fatalf("unexpected rows: %#v", result.Rows)
+	}
+	if scripted.next != len(steps) {
+		t.Fatalf("expected all %d scripted steps, ran %d", len(steps), scripted.next)
+	}
+	if s.statementConn != nil || s.repinnedStatementConn != nil || s.statementSchema != "" || db.Stats().InUse != 0 {
+		t.Fatalf("statement leaked its pinned connections: %+v", db.Stats())
+	}
+}
+
+func TestExecuteQueryPageKeepsCursorOnReplacementPinnedConnection(t *testing.T) {
+	steps := udtPanicFallbackSteps([][]driver.Value{{int64(1), "<GEOM_T>"}, {int64(2), "<GEOM_T>"}})
+	db, scripted := openGuardedOracleViewSourceTestDB(t, steps)
+	s := newServer()
+	s.db = db
+	result, err := s.executeQueryPage(queryOptions{SQL: "SELECT ID, G FROM T_UDT", Schema: "DBXT", MaxRows: 10}, 1)
+	if err != nil {
+		t.Fatalf("expected the placeholder fallback to recover the panic, got %v", err)
+	}
+	if !result.HasMore || result.SessionID == nil || !reflect.DeepEqual(result.Rows, [][]any{{int64(1), "<GEOM_T>"}}) {
+		t.Fatalf("unexpected first page: %+v", result)
+	}
+	if scripted.next != len(steps) {
+		t.Fatalf("expected all %d scripted steps, ran %d", len(steps), scripted.next)
+	}
+	next, err := s.fetchQueryPage(*result.SessionID, 1)
+	if err != nil || !reflect.DeepEqual(next.Rows, [][]any{{int64(2), "<GEOM_T>"}}) {
+		t.Fatalf("unexpected second page: %+v, %v", next, err)
+	}
+	s.closeQuerySession(*result.SessionID)
+	if s.statementConn != nil || s.repinnedStatementConn != nil || db.Stats().InUse != 0 {
+		t.Fatalf("paging session leaked its pinned connections: %+v", db.Stats())
+	}
+}
+
+func TestManualTransactionRewritesObjectColumnsBeforeTheFirstAttempt(t *testing.T) {
+	steps := []oracleViewSourceQueryStep{
+		{exec: true, queryContains: `ALTER SESSION SET CURRENT_SCHEMA = "DBXT"`},
+		{queryContains: "CURRENT_SCHEMA", args: []driver.Value{}, columns: []string{"SCHEMA"}, rows: [][]driver.Value{{"DBXT"}}},
+		{
+			queryContains: "ALL_TAB_COLUMNS",
+			args:          []driver.Value{"DBXT", "T_UDT"},
+			columns:       []string{"COLUMN_NAME", "DATA_TYPE", "DATA_TYPE_OWNER"},
+			rows:          [][]driver.Value{{"ID", "NUMBER", nil}, {"G", "GEOM_T", "DBXT"}},
+		},
+		{queryContains: "'<GEOM_T>'", args: []driver.Value{}, columns: []string{"ID", "G"}, rows: [][]driver.Value{{int64(1), "<GEOM_T>"}}},
+		{queryContains: "SELECT 1 FROM DUAL", args: []driver.Value{}, columns: []string{"1"}, rows: [][]driver.Value{{int64(1)}}},
+	}
+	db, scripted := openGuardedOracleViewSourceTestDB(t, steps)
+	s := newServer()
+	s.db = db
+	if err := s.beginManualTransaction("DBXT"); err != nil {
+		t.Fatal(err)
+	}
+	// The raw statement would panic and take the transaction down with it, so it
+	// must never reach the driver.
+	result, err := s.executeQuery(queryOptions{SQL: "SELECT ID, G FROM T_UDT", MaxRows: 10})
+	if err != nil {
+		t.Fatalf("expected the rewritten projection, got %v", err)
+	}
+	if !reflect.DeepEqual(result.Rows, [][]any{{int64(1), "<GEOM_T>"}}) {
+		t.Fatalf("unexpected rows: %#v", result.Rows)
+	}
+	if _, err := s.executeQuery(queryOptions{SQL: "SELECT 1 FROM DUAL", MaxRows: 10}); err != nil {
+		t.Fatalf("manual transaction became unusable: %v", err)
+	}
+	if scripted.next != len(steps) {
+		t.Fatalf("expected all %d scripted steps, ran %d", len(steps), scripted.next)
+	}
+	if err := s.rollbackManualTransaction(); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestReadQuerySessionPageConvertsPanicToError(t *testing.T) {
 	// A nil *sql.Rows makes rows.Next() panic with a nil pointer dereference,
 	// which stands in for go-ora panicking while decoding an unknown type.

agents/drivers/oracle-go/query_guard.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"database/sql"
 	"database/sql/driver"
 	"fmt"
 )
@@ -59,6 +60,17 @@ func (c *oracleQueryConnection) QueryContext(ctx context.Context, query string,
 	return c.oracleThinConnection.QueryContext(ctx, query, args)
 }
 
+// oracleConnPoisoned reports whether a driver panic already made conn unusable.
+func oracleConnPoisoned(conn *sql.Conn) bool {
+	poisoned := false
+	_ = conn.Raw(func(driverConn any) error {
+		guarded, ok := driverConn.(*oracleQueryConnection)
+		poisoned = ok && guarded.failed != nil
+		return nil
+	})
+	return poisoned
+}
+
 // Invalidate via Validator, not ErrBadConn: ErrBadConn would tell database/sql
 // to replay the original statement, even when it has already acquired locks or
 // invoked a function with side effects. The original panic remains diagnostic.