From a6203613cfce0fc7adecd9a9cecd38aa46ea6a62 Mon Sep 17 00:00:00 2001
From: Marcus Gartner <marcus@cockroachlabs.com>
Date: Fri, 14 Jul 2023 16:30:23 -0400
Subject: [PATCH] sql: do not type-check subqueries in views outside of
 optbuilder

This commit fixes a bug that was caused when attempting to re-type-check
a view's query that contains a subquery. This type-checking occurred
outside of optbuilder. However, the logic for type-checking subqueries
is only implemented within optbuilder, so it failed.

Fixes #105259

Release note (bug fix): A bug has been fixed that caused internal errors
when using user-defined types in views and user-defined functions that
have subqueries. This bug was present when using views since version
v21.2. It was present when using user-defined functions since v23.1.
---
 pkg/sql/create_view.go                        | 14 +++++++++++
 pkg/sql/logictest/testdata/logic_test/udf     | 25 +++++++++++++++++++
 pkg/sql/logictest/testdata/logic_test/views   | 23 +++++++++++++++++
 .../schemachanger/scbuild/builder_state.go    | 14 +++++++++++
 4 files changed, 76 insertions(+)

diff --git a/pkg/sql/create_view.go b/pkg/sql/create_view.go
index 460c4ff309fd..d115c245c741 100644
--- a/pkg/sql/create_view.go
+++ b/pkg/sql/create_view.go
@@ -514,6 +514,20 @@ 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
diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf
index 006209f4e339..7e95f4ab28e7 100644
--- a/pkg/sql/logictest/testdata/logic_test/udf
+++ b/pkg/sql/logictest/testdata/logic_test/udf
@@ -3619,3 +3619,28 @@ CREATE FUNCTION public.func104242_not_null()
 $$
 
 subtest end
+
+# Regression test for #105259. 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
+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()
diff --git a/pkg/sql/logictest/testdata/logic_test/views b/pkg/sql/logictest/testdata/logic_test/views
index 4c09f0ab79c3..6c110c135983 100644
--- a/pkg/sql/logictest/testdata/logic_test/views
+++ b/pkg/sql/logictest/testdata/logic_test/views
@@ -1893,3 +1893,26 @@ query T
 SELECT * FROM v104927
 ----
 [{"i": 1, "s": "foo"}]
+
+# Regression test for #105259. Do not type-check subqueries in views outside
+# optbuilder. Doing so can cause internal errors.
+subtest regression_105259
+
+statement ok
+CREATE TYPE e105259 AS ENUM ('foo');
+
+statement ok
+CREATE VIEW v105259 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
diff --git a/pkg/sql/schemachanger/scbuild/builder_state.go b/pkg/sql/schemachanger/scbuild/builder_state.go
index bb5707fb0ff8..8fc0f5ca0222 100644
--- a/pkg/sql/schemachanger/scbuild/builder_state.go
+++ b/pkg/sql/schemachanger/scbuild/builder_state.go
@@ -1570,6 +1570,20 @@ 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 {