sql: resolve inferred types in prepare using appropriate planner

Before this change we'd resolve user-defined types into placeholder hints
using a planner which may not be in a valid state. This could result in
a nil-pointer panic.

I don't actually know how to trigger this from a driver but there's evidence
that it's possible. It's easy enough to hit using SQL if the previous commit
were not here.

Fixes #64975

Release note (bug fix): Fixed a bug which could cause a panic when issuing
a query referencing a user-defined type as a placeholder.

pkg/sql/conn_executor_exec.go

@@ -46,6 +46,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/errors"
+	"github.com/lib/pq/oid"
 )
 
 // execStmt executes one statement by dispatching according to the current
@@ -521,8 +522,9 @@ func (ex *connExecutor) execStmtInOpenState(
 			},
 			ex.generateID(),
 		)
+		var rawTypeHints []oid.Oid
 		if _, err := ex.addPreparedStmt(
-			ctx, name, prepStmt, typeHints, PreparedStatementOriginSQL,
+			ctx, name, prepStmt, typeHints, rawTypeHints, PreparedStatementOriginSQL,
 		); err != nil {
 			return makeErrEvent(err)
 		}

pkg/sql/conn_executor_prepare.go

@@ -49,36 +49,13 @@ func (ex *connExecutor) execPrepare(
 		ex.deletePreparedStmt(ctx, "")
 	}
 
-	// If we were provided any type hints, attempt to resolve any user defined
-	// type OIDs into types.T's.
-	if parseCmd.TypeHints != nil {
-		for i := range parseCmd.TypeHints {
-			if parseCmd.TypeHints[i] == nil {
-				if i >= len(parseCmd.RawTypeHints) {
-					return retErr(
-						pgwirebase.NewProtocolViolationErrorf(
-							"expected %d arguments, got %d",
-							len(parseCmd.TypeHints),
-							len(parseCmd.RawTypeHints),
-						),
-					)
-				}
-				if types.IsOIDUserDefinedType(parseCmd.RawTypeHints[i]) {
-					var err error
-					parseCmd.TypeHints[i], err = ex.planner.ResolveTypeByOID(ctx, parseCmd.RawTypeHints[i])
-					if err != nil {
-						return retErr(err)
-					}
-				}
-			}
-		}
-	}
 	stmt := makeStatement(parseCmd.Statement, ex.generateID())
 	ps, err := ex.addPreparedStmt(
 		ctx,
 		parseCmd.Name,
 		stmt,
 		parseCmd.TypeHints,
+		parseCmd.RawTypeHints,
 		PreparedStatementOriginWire,
 	)
 	if err != nil {
@@ -115,20 +92,23 @@ func (ex *connExecutor) execPrepare(
 // also returned. It is illegal to call this when a statement with that name
 // already exists (even for anonymous prepared statements).
 //
-// placeholderHints are used to assist in inferring placeholder types.
+// placeholderHints are used to assist in inferring placeholder types. The
+// rawTypeHints are optional and represent OIDs indicated for the placeholders
+// coming from the client via the wire protocol.
 func (ex *connExecutor) addPreparedStmt(
 	ctx context.Context,
 	name string,
 	stmt Statement,
 	placeholderHints tree.PlaceholderTypes,
+	rawTypeHints []oid.Oid,
 	origin PreparedStatementOrigin,
 ) (*PreparedStatement, error) {
 	if _, ok := ex.extraTxnState.prepStmtsNamespace.prepStmts[name]; ok {
 		panic(errors.AssertionFailedf("prepared statement already exists: %q", name))
 	}
 
 	// Prepare the query. This completes the typing of placeholders.
-	prepared, err := ex.prepare(ctx, stmt, placeholderHints, origin)
+	prepared, err := ex.prepare(ctx, stmt, placeholderHints, rawTypeHints, origin)
 	if err != nil {
 		return nil, err
 	}
@@ -151,18 +131,11 @@ func (ex *connExecutor) prepare(
 	ctx context.Context,
 	stmt Statement,
 	placeholderHints tree.PlaceholderTypes,
+	rawTypeHints []oid.Oid,
 	origin PreparedStatementOrigin,
 ) (*PreparedStatement, error) {
-	if placeholderHints == nil {
-		placeholderHints = make(tree.PlaceholderTypes, stmt.NumPlaceholders)
-	}
 
 	prepared := &PreparedStatement{
-		PrepareMetadata: querycache.PrepareMetadata{
-			PlaceholderTypesInfo: tree.PlaceholderTypesInfo{
-				TypeHints: placeholderHints,
-			},
-		},
 		memAcc:   ex.sessionMon.MakeBoundAccount(),
 		refCount: 1,
 
@@ -176,16 +149,6 @@ func (ex *connExecutor) prepare(
 	if stmt.AST == nil {
 		return prepared, nil
 	}
-	prepared.Statement = stmt.Statement
-	prepared.AnonymizedStr = stmt.AnonymizedStr
-
-	// Point to the prepared state, which can be further populated during query
-	// preparation.
-	stmt.Prepared = prepared
-
-	if err := tree.ProcessPlaceholderAnnotations(&ex.planner.semaCtx, stmt.AST, placeholderHints); err != nil {
-		return nil, err
-	}
 
 	// Preparing needs a transaction because it needs to retrieve db/table
 	// descriptors for type checking. If we already have an open transaction for
@@ -209,6 +172,48 @@ func (ex *connExecutor) prepare(
 			// instrumentation.
 			ex.resetPlanner(ctx, p, txn, ex.server.cfg.Clock.PhysicalTime() /* stmtTS */)
 		}
+
+		if placeholderHints == nil {
+			placeholderHints = make(tree.PlaceholderTypes, stmt.NumPlaceholders)
+		} else if rawTypeHints != nil {
+			// If we were provided any type hints, attempt to resolve any user defined
+			// type OIDs into types.T's.
+			for i := range placeholderHints {
+				if placeholderHints[i] == nil {
+					if i >= len(rawTypeHints) {
+						return pgwirebase.NewProtocolViolationErrorf(
+							"expected %d arguments, got %d",
+							len(placeholderHints),
+							len(rawTypeHints),
+						)
+					}
+					if types.IsOIDUserDefinedType(rawTypeHints[i]) {
+						var err error
+						placeholderHints[i], err = ex.planner.ResolveTypeByOID(ctx, rawTypeHints[i])
+						if err != nil {
+							return err
+						}
+					}
+				}
+			}
+		}
+
+		prepared.PrepareMetadata = querycache.PrepareMetadata{
+			PlaceholderTypesInfo: tree.PlaceholderTypesInfo{
+				TypeHints: placeholderHints,
+			},
+		}
+		prepared.Statement = stmt.Statement
+		prepared.AnonymizedStr = stmt.AnonymizedStr
+
+		// Point to the prepared state, which can be further populated during query
+		// preparation.
+		stmt.Prepared = prepared
+
+		if err := tree.ProcessPlaceholderAnnotations(&ex.planner.semaCtx, stmt.AST, placeholderHints); err != nil {
+			return err
+		}
+
 		p.stmt = stmt
 		p.semaCtx.Annotations = tree.MakeAnnotations(stmt.NumAnnotations)
 		flags, err = ex.populatePrepared(ctx, txn, placeholderHints, p)

pkg/sql/pgwire/testdata/pgtest/enum

@@ -1,5 +1,23 @@
 # This test verifies some of the pgwire encoding process for ENUMs.
 
+# Demonstrate that attempting to prepare a query which hints a user-defined
+# type as the very first thing on a new connection does not cause a panic.
+# Prior to the change which introduced this test logic, the below prepare
+# would hit a nil-pointer panic.
+
+send crdb_only
+Parse {"Name": "s1", "Query": "SELECT $1", "ParameterOIDs": [100052]}
+Sync
+----
+
+until crdb_only
+ErrorResponse
+ReadyForQuery
+----
+{"Type":"ErrorResponse","Code":"42704"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+
 # Prepare the environment.
 send noncrdb_only
 Query {"String": "DROP TYPE IF EXISTS te CASCADE"}