From 7274c208a842c101d56fbec8cb8da94255a76d05 Mon Sep 17 00:00:00 2001 From: Drew Kimball Date: Wed, 19 Mar 2025 22:48:31 +0000 Subject: [PATCH] sql: only re-use resolved routine type when flag is set This commit adds a `RoutineUseResolvedType` flag to the `SemaContext` properties to indicate that type-checking for a routine that has already been resolved to a concrete type should short-circuit. This is used to determine the column type for a `Values` operator which depends on a RECORD-returning routine, which only determines its type after the body is built. This will prevent regressions in other code paths that do not desire this short-circuiting behavior. Fixes #142615 Release note (bug fix): Fixed a bug in `v24.1.14`, `v24.3.7`, `v24.3.8`, and `v25.1` which could cause a nil-pointer error when a column's default expression contained a volatile expression (like `nextval`) as a UDF argument. --- pkg/sql/logictest/testdata/logic_test/udf | 25 +++++++++++++++++++++++ pkg/sql/opt/optbuilder/values.go | 17 +++++++-------- pkg/sql/sem/tree/type_check.go | 13 +++++++++--- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf index 32a89b93684b..d2aae4d65bea 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf +++ b/pkg/sql/logictest/testdata/logic_test/udf @@ -979,6 +979,8 @@ LIMIT subtest end # Regression test for #104009. +subtest regression_104009 + statement ok CREATE TABLE ab104009(a INT PRIMARY KEY, b INT) @@ -1002,3 +1004,26 @@ PREPARE p AS SELECT f($1::REGCLASS::INT) statement ok EXECUTE p(10) + +# Regression test for #142615. +subtest regression_142615 + +statement ok +create function app_to_db_id(app_id INT8) RETURNS INT8 LANGUAGE SQL AS $$ SELECT app_id * 2; $$; + +statement ok +create sequence seq1; + +statement ok +create table test (id int8 not null default app_to_db_id(nextval('seq1'::REGCLASS))); + +query TTITT rowsort +select * from pg_catalog.pg_attrdef; +---- +1508958170 132 2 unique_rowid() unique_rowid() +1202826234 151 1 gen_random_uuid() gen_random_uuid() +1202826232 151 3 now() now() +2121222035 174 1 public.app_to_db_id(nextval('public.seq1'::REGCLASS)) public.app_to_db_id(nextval('public.seq1'::REGCLASS)) +2121222032 174 2 unique_rowid() unique_rowid() + +subtest end diff --git a/pkg/sql/opt/optbuilder/values.go b/pkg/sql/opt/optbuilder/values.go index ecb03d7f5050..c1bfeff27204 100644 --- a/pkg/sql/opt/optbuilder/values.go +++ b/pkg/sql/opt/optbuilder/values.go @@ -77,14 +77,15 @@ func (b *Builder) buildValuesClause( elemPos += numCols if texpr.ResolvedType().IsWildcardType() { // Type-check the expression once again in order to update expressions - // that wrap a routine to reflect the modified type. Make sure to use - // the previously resolved type as the desired type, since the AST may - // have been modified to remove type annotations. This can happen when - // the routine's return type is unknown until its body is built. - texpr, err = tree.TypeCheck(b.ctx, texpr, b.semaCtx, texpr.ResolvedType()) - if err != nil { - panic(err) - } + // that wrap a routine to reflect the modified type. + func() { + defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) + b.semaCtx.Properties.RoutineUseResolvedType = true + texpr, err = tree.TypeCheck(b.ctx, texpr, b.semaCtx, desired) + if err != nil { + panic(err) + } + }() } if typ := texpr.ResolvedType(); typ.Family() != types.UnknownFamily { if colTypes[colIdx].Family() == types.UnknownFamily { diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index ba0ff45ba5fe..a1cc3231270b 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -96,6 +96,12 @@ type SemaProperties struct { // IgnoreUnpreferredOverloads is set to true when "unpreferred" overloads // should not be used during type-checking and overload resolution. IgnoreUnpreferredOverloads bool + + // RoutineUseResolvedType is set to true when type-checking for a routine + // should reuse the already resolved type, if any. This is used to handle + // "re-type-checking" that occurs for a RECORD-returning routine, for which + // the return type is not known until the routine body is built. + RoutineUseResolvedType bool } type semaRequirements struct { @@ -1172,9 +1178,10 @@ func (expr *FuncExpr) typeCheckWithFuncAncestor(semaCtx *SemaContext, fn func() func (expr *FuncExpr) TypeCheck( ctx context.Context, semaCtx *SemaContext, desired *types.T, ) (TypedExpr, error) { - if expr.fn != nil && expr.fn.Type != BuiltinRoutine && expr.typ != nil { - // Don't overwrite the resolved properties for a user-defined routine if the - // routine has already been resolved. + if semaCtx != nil && semaCtx.Properties.RoutineUseResolvedType && + expr.typ != nil && !expr.typ.IsWildcardType() { + // Don't overwrite the resolved properties for a routine if the routine has + // already been resolved (and we are in a context that needs this behavior). return expr, nil }