Skip to content

Commit

Permalink
sql: do not allow subqueries to be cast to enums in views and UDFs
Browse files Browse the repository at this point in the history
This commit is a follow-up to cockroachdb#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 cockroachdb#108184 to track this limitation.

Fixes cockroachdb#107654

There is no release note because the release note from cockroachdb#106868 should be
sufficient.

Release note: None
  • Loading branch information
mgartner committed Aug 23, 2023
1 parent 9adfd48 commit 3062dc8
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 61 deletions.
4 changes: 3 additions & 1 deletion pkg/sql/create_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
34 changes: 18 additions & 16 deletions pkg/sql/create_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
24 changes: 10 additions & 14 deletions pkg/sql/logictest/testdata/logic_test/udf
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 10 additions & 16 deletions pkg/sql/logictest/testdata/logic_test/views
Original file line number Diff line number Diff line change
Expand Up @@ -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
21 changes: 7 additions & 14 deletions pkg/sql/schemachanger/scbuild/builder_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down

0 comments on commit 3062dc8

Please sign in to comment.