diff --git a/conn.go b/conn.go index a15c280e..7d5b79d0 100644 --- a/conn.go +++ b/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 diff --git a/internal/stmtcache/stmtcache.go b/internal/stmtcache/stmtcache.go index a86755cc..b2940e23 100644 --- a/internal/stmtcache/stmtcache.go +++ b/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 -} diff --git a/query_test.go b/query_test.go index 6d7e91df..92f431c7 100644 --- a/query_test.go +++ b/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() diff --git a/rows.go b/rows.go index a497738a..444a1241 100644 --- a/rows.go +++ b/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) } }