Improve QueryExecModeCacheDescribe and clarify documentation

QueryExecModeCacheDescribe actually is safe even when the schema or
search_path is modified. It may return an error on the first execution
but it should never silently encode or decode a value incorrectly. Add a
test to demonstrate and ensure this behavior.

Update documentation of QueryExecModeCacheDescribe to remove warning of
undetected result decoding errors.

Update documentation of QueryExecModeCacheStatement and
QueryExecModeCacheDescribe to indicate that the first execution of an
invalidated statement may fail.

conn.go

@@ -598,13 +598,16 @@ type QueryExecMode int32
 const (
 	_ QueryExecMode = iota
 
-	// Automatically prepare and cache statements. This uses the extended protocol. Queries are executed in a single
-	// round trip after the statement is cached. This is the default.
+	// Automatically prepare and cache statements. This uses the extended protocol. Queries are executed in a single round
+	// trip after the statement is cached. This is the default. If the database schema is modified or the search_path is
+	// changed after a statement is cached then the first execution of a previously cached query may fail. e.g. If the
+	// number of columns returned by a "SELECT *" changes or the type of a column is changed.
 	QueryExecModeCacheStatement
 
-	// Cache statement descriptions (i.e. argument and result types) and assume they do not change. This uses the
-	// extended protocol. Queries are executed in a single round trip after the description is cached. If the database
-	// schema is modified or the search_path is changed this may result in undetected result decoding errors.
+	// Cache statement descriptions (i.e. argument and result types) and assume they do not change. This uses the extended
+	// protocol. Queries are executed in a single round trip after the description is cached. If the database schema is
+	// modified or the search_path is changed after a statement is cached then the first execution of a previously cached
+	// query may fail. e.g. If the number of columns returned by a "SELECT *" changes or the type of a column is changed.
 	QueryExecModeCacheDescribe
 
 	// Get the statement description on every execution. This uses the extended protocol. Queries require two round trips

internal/stmtcache/stmtcache.go

@@ -38,19 +38,3 @@ type Cache interface {
 	// Cap returns the maximum number of cached prepared statement descriptions.
 	Cap() int
 }
-
-func IsStatementInvalid(err error) bool {
-	pgErr, ok := err.(*pgconn.PgError)
-	if !ok {
-		return false
-	}
-
-	// https://github.com/jackc/pgx/issues/1162
-	//
-	// We used to look for the message "cached plan must not change result type". However, that message can be localized.
-	// Unfortunately, error code "0A000" - "FEATURE NOT SUPPORTED" is used for many different errors and the only way to
-	// tell the difference is by the message. But all that happens is we clear a statement that we otherwise wouldn't
-	// have so it should be safe.
-	possibleInvalidCachedPlanError := pgErr.Code == "0A000"
-	return possibleInvalidCachedPlanError
-}

query_test.go

@@ -1928,6 +1928,64 @@ func TestQueryErrorWithDisabledStatementCache(t *testing.T) {
 	ensureConnValid(t, conn)
 }
 
+func TestConnQueryQueryExecModeCacheDescribeSafeEvenWhenTypesChange(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+	defer cancel()
+
+	conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE"))
+	defer closeConn(t, conn)
+
+	_, err := conn.Exec(ctx, `create temporary table to_change (
+	name text primary key,
+	age int
+);
+
+insert into to_change (name, age) values ('John', 42);`)
+	require.NoError(t, err)
+
+	var name string
+	var ageInt32 int32
+	err = conn.QueryRow(ctx, "select * from to_change where age = $1", pgx.QueryExecModeCacheDescribe, int32(42)).Scan(&name, &ageInt32)
+	require.NoError(t, err)
+	require.Equal(t, "John", name)
+	require.Equal(t, int32(42), ageInt32)
+
+	_, err = conn.Exec(ctx, `alter table to_change alter column age type float4;`)
+	require.NoError(t, err)
+
+	err = conn.QueryRow(ctx, "select * from to_change where age = $1", pgx.QueryExecModeCacheDescribe, int32(42)).Scan(&name, &ageInt32)
+	require.NoError(t, err)
+	require.Equal(t, "John", name)
+	require.Equal(t, int32(42), ageInt32)
+
+	var ageFloat32 float32
+	err = conn.QueryRow(ctx, "select * from to_change where age = $1", pgx.QueryExecModeCacheDescribe, int32(42)).Scan(&name, &ageFloat32)
+	require.NoError(t, err)
+	require.Equal(t, "John", name)
+	require.Equal(t, float32(42), ageFloat32)
+
+	_, err = conn.Exec(ctx, `alter table to_change drop column name;`)
+	require.NoError(t, err)
+
+	// Number of result columns has changed, so just like with a prepared statement, this will fail the first time.
+	err = conn.QueryRow(ctx, "select * from to_change where age = $1", pgx.QueryExecModeCacheDescribe, int32(42)).Scan(&ageFloat32)
+	require.EqualError(t, err, "ERROR: bind message has 2 result formats but query has 1 columns (SQLSTATE 08P01)")
+
+	// But it will work the second time after the cache is invalidated.
+	err = conn.QueryRow(ctx, "select * from to_change where age = $1", pgx.QueryExecModeCacheDescribe, int32(42)).Scan(&ageFloat32)
+	require.NoError(t, err)
+	require.Equal(t, float32(42), ageFloat32)
+
+	_, err = conn.Exec(ctx, `alter table to_change alter column age type numeric;`)
+	require.NoError(t, err)
+
+	err = conn.QueryRow(ctx, "select * from to_change where age = $1", pgx.QueryExecModeCacheDescribe, int32(42)).Scan(&ageFloat32)
+	require.NoError(t, err)
+	require.Equal(t, float32(42), ageFloat32)
+}
+
 func TestQueryWithQueryRewriter(t *testing.T) {
 	t.Parallel()
 

rows.go

@@ -8,7 +8,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/jackc/pgx/v5/internal/stmtcache"
 	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/jackc/pgx/v5/pgtype"
 )
@@ -174,14 +173,12 @@ func (rows *baseRows) Close() {
 	}
 
 	if rows.err != nil && rows.conn != nil && rows.sql != "" {
-		if stmtcache.IsStatementInvalid(rows.err) {
-			if sc := rows.conn.statementCache; sc != nil {
-				sc.Invalidate(rows.sql)
-			}
+		if sc := rows.conn.statementCache; sc != nil {
+			sc.Invalidate(rows.sql)
+		}
 
-			if sc := rows.conn.descriptionCache; sc != nil {
-				sc.Invalidate(rows.sql)
-			}
+		if sc := rows.conn.descriptionCache; sc != nil {
+			sc.Invalidate(rows.sql)
 		}
 	}