From 3062dc8a8ce512e9b21dd40fe04a65f827e3e256 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Fri, 4 Aug 2023 12:58:19 -0400 Subject: [PATCH] sql: do not allow subqueries to be cast to enums in views and UDFs This commit is a follow-up to #106868 after additional reproductions of the original bug were found. For now, we disallow any CAST expressions that contain a subquery in the input and the target type is an ENUM. I've created #108184 to track this limitation. Fixes #107654 There is no release note because the release note from #106868 should be sufficient. Release note: None --- pkg/sql/create_function.go | 4 ++- pkg/sql/create_view.go | 34 ++++++++++--------- pkg/sql/logictest/testdata/logic_test/udf | 24 ++++++------- pkg/sql/logictest/testdata/logic_test/views | 26 ++++++-------- .../schemachanger/scbuild/builder_state.go | 21 ++++-------- 5 files changed, 48 insertions(+), 61 deletions(-) diff --git a/pkg/sql/create_function.go b/pkg/sql/create_function.go index a80b194db6aa..9670325f0d91 100644 --- a/pkg/sql/create_function.go +++ b/pkg/sql/create_function.go @@ -432,7 +432,9 @@ func setFuncOption(params runParams, udfDesc *funcdesc.Mutable, option tree.Func if err != nil { return err } - typeReplacedFuncBody, err := serializeUserDefinedTypes(params.ctx, params.p.SemaCtx(), seqReplacedFuncBody, true /* multiStmt */) + typeReplacedFuncBody, err := serializeUserDefinedTypes( + params.ctx, params.p.SemaCtx(), seqReplacedFuncBody, true /* multiStmt */, "UDFs", + ) if err != nil { return err } diff --git a/pkg/sql/create_view.go b/pkg/sql/create_view.go index 5973a98a68af..c0f39816b3bc 100644 --- a/pkg/sql/create_view.go +++ b/pkg/sql/create_view.go @@ -411,7 +411,8 @@ func makeViewTableDesc( desc.ViewQuery = sequenceReplacedQuery } - typeReplacedQuery, err := serializeUserDefinedTypes(ctx, semaCtx, desc.ViewQuery, false /* multiStmt */) + typeReplacedQuery, err := serializeUserDefinedTypes(ctx, semaCtx, desc.ViewQuery, + false /* multiStmt */, "view queries") if err != nil { return tabledesc.Mutable{}, err } @@ -490,7 +491,7 @@ func replaceSeqNamesWithIDs( // and serialize any user defined types, so that renaming the type // does not corrupt the view. func serializeUserDefinedTypes( - ctx context.Context, semaCtx *tree.SemaContext, queries string, multiStmt bool, + ctx context.Context, semaCtx *tree.SemaContext, queries string, multiStmt bool, parentType string, ) (string, error) { replaceFunc := func(expr tree.Expr) (recurse bool, newExpr tree.Expr, err error) { var innerExpr tree.Expr @@ -505,20 +506,6 @@ func serializeUserDefinedTypes( default: return true, expr, nil } - // We cannot type-check subqueries without using optbuilder, and there - // is no need to because we only need to rewrite string values that are - // directly cast to enums. For example, we must rewrite the 'foo' in: - // - // SELECT 'foo'::myenum - // - // We don't need to rewrite the 'foo' in the query below, which can be - // corrupted by renaming the 'foo' value in the myenum type. - // - // SELECT (SELECT 'foo')::myenum - // - if _, ok := innerExpr.(*tree.Subquery); ok { - return true, expr, nil - } // semaCtx may be nil if this is a virtual view being created at // init time. var typeResolver tree.TypeReferenceResolver @@ -533,6 +520,14 @@ func serializeUserDefinedTypes( if !typ.UserDefined() { return true, expr, nil } + { + // We cannot type-check subqueries without using optbuilder, so we + // currently do not support casting expressions with subqueries to + // UDTs. + context := "casts to enums within " + parentType + defer semaCtx.Properties.Restore(semaCtx.Properties) + semaCtx.Properties.Require(context, tree.RejectSubqueries) + } texpr, err := innerExpr.TypeCheck(ctx, semaCtx, typ) if err != nil { return false, expr, err @@ -603,6 +598,13 @@ func (p *planner) replaceViewDesc( toReplace.ViewQuery = updatedQuery } + typeReplacedQuery, err := serializeUserDefinedTypes(ctx, p.SemaCtx(), toReplace.ViewQuery, + false /* multiStmt */, "view queries") + if err != nil { + return nil, err + } + toReplace.ViewQuery = typeReplacedQuery + // Reset the columns to add the new result columns onto. toReplace.Columns = make([]descpb.ColumnDescriptor, 0, len(n.columns)) toReplace.NextColumnID = 0 diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf index 99f6ce527bba..78a6e6496acd 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf +++ b/pkg/sql/logictest/testdata/logic_test/udf @@ -3575,29 +3575,25 @@ SELECT public."LOWERCASE_HINT_ERROR_EXPLICIT_SCHEMA_FN"(); subtest end -# Regression test for #105259. Do not type-check subqueries in UDFs outside -# optbuilder. Doing so can cause internal errors. +# Regression tests for #105259 and #107654. Do not type-check subqueries in UDFs +# outside optbuilder. Doing so can cause internal errors. subtest regression_105259 statement ok CREATE TYPE e105259 AS ENUM ('foo'); -statement ok +statement error pgcode 0A000 subqueries are not allowed in casts to enums within UDFs CREATE FUNCTION f() RETURNS VOID LANGUAGE SQL AS $$ SELECT (SELECT 'foo')::e105259; SELECT NULL; $$ -query T -SELECT f() ----- -NULL - -statement ok -ALTER TYPE e105259 RENAME VALUE 'foo' TO 'bar' - -# Renaming the enum value corrupts the UDF. This is expected behavior. -statement error pgcode 22P02 invalid input value for enum e105259: "foo" -SELECT f() +statement error pgcode 0A000 subqueries are not allowed in casts to enums within UDFs +CREATE FUNCTION f() RETURNS VOID LANGUAGE SQL AS $$ + SELECT ( + CASE WHEN true THEN (SELECT 'foo') ELSE NULL END + )::e105259; + SELECT NULL; +$$ subtest end diff --git a/pkg/sql/logictest/testdata/logic_test/views b/pkg/sql/logictest/testdata/logic_test/views index 9f8e531fd5db..3ff91b8b9d6c 100644 --- a/pkg/sql/logictest/testdata/logic_test/views +++ b/pkg/sql/logictest/testdata/logic_test/views @@ -1850,27 +1850,21 @@ DROP VIEW cd_v1 CASCADE; subtest end -# Regression test for #105259. Do not type-check subqueries in views outside -# optbuilder. Doing so can cause internal errors. -subtest regression_105259 +# Regression tests for #105259 and #107654. Do not type-check subqueries in +# views outside optbuilder. Doing so can cause internal errors. +subtest regression_105259_107654 statement ok CREATE TYPE e105259 AS ENUM ('foo'); -statement ok -CREATE VIEW v105259 AS +statement error pgcode 0A000 subqueries are not allowed in casts to enums within view queries +CREATE VIEW v AS SELECT (SELECT 'foo')::e105259 -query T -SELECT * FROM v105259 ----- -foo - -statement ok -ALTER TYPE e105259 RENAME VALUE 'foo' TO 'bar' - -# Renaming the enum value corrupts the view. This is expected behavior. -statement error pgcode 22P02 invalid input value for enum e105259: "foo" -SELECT * FROM v105259 +statement error pgcode 0A000 subqueries are not allowed in casts to enums within view queries +CREATE VIEW v AS +SELECT ( + CASE WHEN true THEN (SELECT 'foo') ELSE NULL END +)::e105259 subtest end diff --git a/pkg/sql/schemachanger/scbuild/builder_state.go b/pkg/sql/schemachanger/scbuild/builder_state.go index 5f1ab117a885..2da9d5d8d67b 100644 --- a/pkg/sql/schemachanger/scbuild/builder_state.go +++ b/pkg/sql/schemachanger/scbuild/builder_state.go @@ -1485,20 +1485,6 @@ func (b *builderState) serializeUserDefinedTypes(queryStr string) string { default: return true, expr, nil } - // We cannot type-check subqueries without using optbuilder, and there - // is no need to because we only need to rewrite string values that are - // directly cast to enums. For example, we must rewrite the 'foo' in: - // - // SELECT 'foo'::myenum - // - // We don't need to rewrite the 'foo' in the query below, which can be - // corrupted by renaming the 'foo' value in the myenum type. - // - // SELECT (SELECT 'foo')::myenum - // - if _, ok := innerExpr.(*tree.Subquery); ok { - return true, expr, nil - } var typ *types.T typ, err = tree.ResolveType(b.ctx, typRef, b.semaCtx.TypeResolver) if err != nil { @@ -1507,6 +1493,13 @@ func (b *builderState) serializeUserDefinedTypes(queryStr string) string { if !typ.UserDefined() { return true, expr, nil } + { + // We cannot type-check subqueries without using optbuilder, so we + // currently do not support casting expressions with subqueries to + // UDTs. + defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) + b.semaCtx.Properties.Require("casts to enums within UDFs", tree.RejectSubqueries) + } texpr, err := innerExpr.TypeCheck(b.ctx, b.semaCtx, typ) if err != nil { return false, expr, err