catalog/schemaexpr: break dependency on sql/parser

yuzefovich · yuzefovich · commit 11940299cd33 · 2025-08-27T16:06:17.000-07:00
Release note: None
diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
@@ -421,6 +421,7 @@ ALL_TESTS = [
     "//pkg/sql/catalog/replication:replication_test",
     "//pkg/sql/catalog/resolver:resolver_test",
     "//pkg/sql/catalog/schemadesc:schemadesc_test",
+    "//pkg/sql/catalog/schemaexpr:schemaexpr_disallowed_imports_test",
     "//pkg/sql/catalog/schemaexpr:schemaexpr_test",
     "//pkg/sql/catalog/schematelemetry:schematelemetry_test",
     "//pkg/sql/catalog/seqexpr:seqexpr_disallowed_imports_test",
diff --git a/pkg/sql/catalog/catformat/BUILD.bazel b/pkg/sql/catalog/catformat/BUILD.bazel
@@ -24,9 +24,13 @@ go_library(
 go_test(
     name = "catformat_test",
     size = "small",
-    srcs = ["index_test.go"],
+    srcs = [
+        "index_test.go",
+        "inject_parser_test.go",
+    ],
     embed = [":catformat"],
     deps = [
+        "//pkg/sql",
         "//pkg/sql/catalog/catenumpb",
         "//pkg/sql/catalog/catpb",
         "//pkg/sql/catalog/descpb",
diff --git a/pkg/sql/catalog/catformat/inject_parser_test.go b/pkg/sql/catalog/catformat/inject_parser_test.go
@@ -0,0 +1,12 @@
+// Copyright 2025 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package catformat_test
+
+import "github.com/cockroachdb/cockroach/pkg/sql"
+
+func init() {
+	sql.DoParserInjection()
+}
diff --git a/pkg/sql/catalog/funcdesc/BUILD.bazel b/pkg/sql/catalog/funcdesc/BUILD.bazel
@@ -45,6 +45,7 @@ go_test(
         "//pkg/clusterversion",
         "//pkg/jobs/jobspb",
         "//pkg/security/username",
+        "//pkg/sql",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/bootstrap",
         "//pkg/sql/catalog/catpb",
diff --git a/pkg/sql/catalog/funcdesc/func_desc_test.go b/pkg/sql/catalog/funcdesc/func_desc_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
+	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
@@ -34,6 +35,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func init() {
+	sql.DoParserInjection()
+}
+
 func TestValidateFuncDesc(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	ctx := context.Background()
diff --git a/pkg/sql/catalog/schemaexpr/BUILD.bazel b/pkg/sql/catalog/schemaexpr/BUILD.bazel
@@ -1,4 +1,5 @@
 load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("//pkg/testutils:buildutil/buildutil.bzl", "disallowed_imports_test")
 
 go_library(
     name = "schemaexpr",
@@ -26,7 +27,7 @@ go_library(
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/catalog/funcdesc",
-        "//pkg/sql/parser",
+        "//pkg/sql/parserutils",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/sem/builtins/builtinsregistry",
@@ -53,13 +54,15 @@ go_test(
         "column_test.go",
         "computed_column_rewrites_test.go",
         "expr_test.go",
+        "inject_parser_test.go",
         "partial_index_test.go",
         "testutils_test.go",
     ],
     data = glob(["testdata/**"]),
     embed = [":schemaexpr"],
     deps = [
         "//pkg/clusterversion",
+        "//pkg/sql",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/catalog/descpb",
@@ -76,3 +79,8 @@ go_test(
         "@com_github_cockroachdb_datadriven//:datadriven",
     ],
 )
+
+disallowed_imports_test(
+    src = "schemaexpr",
+    disallowed_list = ["//pkg/sql/parser"],
+)
diff --git a/pkg/sql/catalog/schemaexpr/column.go b/pkg/sql/catalog/schemaexpr/column.go
@@ -11,7 +11,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/builtinsregistry"
@@ -152,7 +152,7 @@ func FormatColumnForDisplay(
 // RenameColumn replaces any occurrence of the column from in expr with to, and
 // returns a string representation of the new expression.
 func RenameColumn(expr string, from tree.Name, to tree.Name) (string, error) {
-	parsed, err := parser.ParseExpr(expr)
+	parsed, err := parserutils.ParseExpr(expr)
 	if err != nil {
 		return "", err
 	}
diff --git a/pkg/sql/catalog/schemaexpr/computed_column.go b/pkg/sql/catalog/schemaexpr/computed_column.go
@@ -13,7 +13,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
@@ -176,7 +176,7 @@ func ValidateColumnHasNoDependents(desc catalog.TableDescriptor, col catalog.Col
 			continue
 		}
 
-		expr, err := parser.ParseExpr(c.GetComputeExpr())
+		expr, err := parserutils.ParseExpr(c.GetComputeExpr())
 		if err != nil {
 			// At this point, we should be able to parse the computed expression.
 			return errors.WithAssertionFailure(err)
@@ -237,7 +237,7 @@ func MakeComputedExprs(
 		}
 	}
 
-	exprs, err := parser.ParseExprs(exprStrings)
+	exprs, err := parserutils.ParseExprs(exprStrings)
 	if err != nil {
 		return nil, catalog.TableColSet{}, err
 	}
diff --git a/pkg/sql/catalog/schemaexpr/computed_column_rewrites.go b/pkg/sql/catalog/schemaexpr/computed_column_rewrites.go
@@ -8,7 +8,7 @@ package schemaexpr
 import (
 	"fmt"
 
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree/treebin"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
@@ -30,7 +30,7 @@ func ParseComputedColumnRewrites(val string) (ComputedColumnRewritesMap, error)
 	if val == "" {
 		return nil, nil
 	}
-	stmt, err := parser.ParseOne(fmt.Sprintf("SET ROW (%s)", val))
+	stmt, err := parserutils.ParseOne(fmt.Sprintf("SET ROW (%s)", val))
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to parse column rewrites")
 	}
diff --git a/pkg/sql/catalog/schemaexpr/default_exprs.go b/pkg/sql/catalog/schemaexpr/default_exprs.go
@@ -9,7 +9,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/transform"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -50,7 +50,7 @@ func MakeDefaultExprs(
 			exprStrings = append(exprStrings, col.GetDefaultExpr())
 		}
 	}
-	exprs, err := parser.ParseExprs(exprStrings)
+	exprs, err := parserutils.ParseExprs(exprStrings)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/sql/catalog/schemaexpr/expr.go b/pkg/sql/catalog/schemaexpr/expr.go
@@ -15,7 +15,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc"
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/cast"
@@ -342,7 +342,7 @@ func deserializeExprForFormatting(
 	semaCtx *tree.SemaContext,
 	fmtFlags tree.FmtFlags,
 ) (tree.Expr, error) {
-	expr, err := parser.ParseExpr(exprStr)
+	expr, err := parserutils.ParseExpr(exprStr)
 	if err != nil {
 		return nil, err
 	}
@@ -587,7 +587,7 @@ func ValidatePartialIndex(
 ) error {
 	for _, idx := range tableDesc.AllIndexes() {
 		if idx.IsPartial() {
-			expr, err := parser.ParseExpr(idx.GetPredicate())
+			expr, err := parserutils.ParseExpr(idx.GetPredicate())
 			if err != nil {
 				return err
 			}
@@ -625,7 +625,7 @@ func ValidateTTLExpirationExpression(
 		return nil
 	}
 
-	exprs, err := parser.ParseExprs([]string{string(ttl.ExpirationExpr)})
+	exprs, err := parserutils.ParseExprs([]string{string(ttl.ExpirationExpr)})
 	if err != nil {
 		return pgerror.Wrapf(
 			err,
@@ -717,7 +717,7 @@ func GetUDFIDs(e tree.Expr) (catalog.DescriptorIDSet, error) {
 // expression string, assuming that the UDF names has been replaced with OID
 // references. It's a convenient wrapper of GetUDFIDs.
 func GetUDFIDsFromExprStr(exprStr string) (catalog.DescriptorIDSet, error) {
-	expr, err := parser.ParseExpr(exprStr)
+	expr, err := parserutils.ParseExpr(exprStr)
 	if err != nil {
 		return catalog.DescriptorIDSet{}, err
 	}
@@ -727,7 +727,7 @@ func GetUDFIDsFromExprStr(exprStr string) (catalog.DescriptorIDSet, error) {
 func validateExpressionDoesNotDependOnColumn(
 	tableDesc catalog.TableDescriptor, expirationExpr string, dependentColID descpb.ColumnID,
 ) (bool, error) {
-	expr, err := parser.ParseExpr(expirationExpr)
+	expr, err := parserutils.ParseExpr(expirationExpr)
 	if err != nil {
 		// At this point, we should be able to parse the expression.
 		return false, errors.WithAssertionFailure(err)
diff --git a/pkg/sql/catalog/schemaexpr/inject_parser_test.go b/pkg/sql/catalog/schemaexpr/inject_parser_test.go
@@ -0,0 +1,12 @@
+// Copyright 2025 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package schemaexpr_test
+
+import "github.com/cockroachdb/cockroach/pkg/sql"
+
+func init() {
+	sql.DoParserInjection()
+}
diff --git a/pkg/sql/catalog/schemaexpr/partial_index.go b/pkg/sql/catalog/schemaexpr/partial_index.go
@@ -11,7 +11,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
@@ -184,7 +184,7 @@ func makePartialIndexHelper(
 func (pi partialIndexHelper) makePartialIndexExpr(
 	ctx context.Context, idx catalog.Index,
 ) (tree.TypedExpr, catalog.TableColSet, error) {
-	expr, err := parser.ParseExpr(idx.GetPredicate())
+	expr, err := parserutils.ParseExpr(idx.GetPredicate())
 	if err != nil {
 		return nil, catalog.TableColSet{}, err
 	}
diff --git a/pkg/sql/catalog/schemaexpr/sequence_options.go b/pkg/sql/catalog/schemaexpr/sequence_options.go
@@ -9,7 +9,7 @@ import (
 	"math"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
-	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/parserutils"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
@@ -23,7 +23,7 @@ import (
 func ParseSequenceOpts(
 	s string, defaultIntSize int32,
 ) (*descpb.TableDescriptor_SequenceOpts, error) {
-	stmt, err := parser.ParseOne("CREATE SEQUENCE fake_seq " + s)
+	stmt, err := parserutils.ParseOne("CREATE SEQUENCE fake_seq " + s)
 	if err != nil {
 		return nil, errors.Wrap(err, "cannot parse sequence option")
 	}
@@ -78,7 +78,7 @@ func AssignSequenceOptions(
 ) error {
 
 	// Set the default integer type of a sequence.
-	integerType := parser.NakedIntTypeFromDefaultIntSize(defaultIntSize)
+	integerType := parserutils.NakedIntTypeFromDefaultIntSize(defaultIntSize)
 	// All other defaults are dependent on the value of increment
 	// and the AS integerType. (i.e. whether the sequence is ascending
 	// or descending, bigint vs. smallint)
diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go
@@ -153,6 +153,7 @@ func init() {
 func DoParserInjection() {
 	parserutils.Parse = parser.Parse
 	parserutils.ParseExpr = parser.ParseExpr
+	parserutils.ParseExprs = parser.ParseExprs
 	parserutils.ParseOne = parser.ParseOne
 	parserutils.ParseQualifiedTableName = parser.ParseQualifiedTableName
 }
diff --git a/pkg/sql/parser/parse.go b/pkg/sql/parser/parse.go
@@ -70,18 +70,6 @@ func (po ParseOptions) WithIntType(t *types.T) ParseOptions {
 // INT := INT4 will simply use INT4 in any resulting code.
 var defaultNakedIntType = types.Int
 
-// NakedIntTypeFromDefaultIntSize given the size in bits or bytes (preferred)
-// of how a "naked" INT type should be parsed returns the corresponding integer
-// type.
-func NakedIntTypeFromDefaultIntSize(defaultIntSize int32) *types.T {
-	switch defaultIntSize {
-	case 4, 32:
-		return types.Int4
-	default:
-		return types.Int
-	}
-}
-
 // Parse parses the sql and returns a list of statements.
 func (p *Parser) Parse(sql string) (statements.Statements, error) {
 	return p.parseWithDepth(1, sql, DefaultParseOptions)
diff --git a/pkg/sql/parserutils/BUILD.bazel b/pkg/sql/parserutils/BUILD.bazel
@@ -11,6 +11,7 @@ go_library(
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/sem/tree",
+        "//pkg/sql/types",
         "@com_github_cockroachdb_errors//:errors",
     ],
 )
diff --git a/pkg/sql/parserutils/utils.go b/pkg/sql/parserutils/utils.go
@@ -14,6 +14,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/errors"
 )
 
@@ -57,6 +58,18 @@ func PopulateErrorDetails(
 	return errors.WithDetail(retErr, buf.String())
 }
 
+// NakedIntTypeFromDefaultIntSize given the size in bits or bytes (preferred)
+// of how a "naked" INT type should be parsed returns the corresponding integer
+// type.
+func NakedIntTypeFromDefaultIntSize(defaultIntSize int32) *types.T {
+	switch defaultIntSize {
+	case 4, 32:
+		return types.Int4
+	default:
+		return types.Int
+	}
+}
+
 // Parse is the same as sql/parser.Parse but is injected to avoid a dependency
 // on the parser package.
 var Parse = func(sql string) (statements.Statements, error) {
@@ -69,6 +82,12 @@ var ParseExpr = func(sql string) (tree.Expr, error) {
 	return nil, errors.AssertionFailedf("sql.DoParserInjection hasn't been called")
 }
 
+// ParseExprs is the same as sql/parser.ParseExprs but is injected to avoid a
+// dependency on the parser package.
+var ParseExprs = func(exprs []string) (tree.Exprs, error) {
+	return nil, errors.AssertionFailedf("sql.DoParserInjection hasn't been called")
+}
+
 // ParseOne is the same as sql/parser.ParseOne but is injected to avoid a
 // dependency on the parser package.
 var ParseOne = func(sql string) (statements.Statement[tree.Statement], error) {
diff --git a/pkg/sql/pgwire/BUILD.bazel b/pkg/sql/pgwire/BUILD.bazel
@@ -48,6 +48,7 @@ go_library(
         "//pkg/sql/lexbase",
         "//pkg/sql/parser",
         "//pkg/sql/parser/statements",
+        "//pkg/sql/parserutils",
         "//pkg/sql/pgrepl/pgreplparser",
         "//pkg/sql/pgrepl/pgrepltree",
         "//pkg/sql/pgwire/hba",
diff --git a/pkg/sql/pgwire/server.go b/pkg/sql/pgwire/server.go
diff --git a/pkg/sql/session_state.go b/pkg/sql/session_state.go

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ import (`
`13`	`13`	`"github.com/cockroachdb/cockroach/pkg/clusterversion"`
`14`	`14`	`"github.com/cockroachdb/cockroach/pkg/sql/catalog"`
`15`	`15`	`"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"`
`16`		`- "github.com/cockroachdb/cockroach/pkg/sql/parser"`
	`16`	`+ "github.com/cockroachdb/cockroach/pkg/sql/parserutils"`
`17`	`17`	`"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"`
`18`	`18`	`"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"`
`19`	`19`	`"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"`
`@@ -176,7 +176,7 @@ func ValidateColumnHasNoDependents(desc catalog.TableDescriptor, col catalog.Col`
`176`	`176`	`continue`
`177`	`177`	`}`
`178`	`178`
`179`		`- expr, err := parser.ParseExpr(c.GetComputeExpr())`
	`179`	`+ expr, err := parserutils.ParseExpr(c.GetComputeExpr())`
`180`	`180`	`if err != nil {`
`181`	`181`	`// At this point, we should be able to parse the computed expression.`
`182`	`182`	`return errors.WithAssertionFailure(err)`
`@@ -237,7 +237,7 @@ func MakeComputedExprs(`
`237`	`237`	`}`
`238`	`238`	`}`
`239`	`239`
`240`		`- exprs, err := parser.ParseExprs(exprStrings)`
	`240`	`+ exprs, err := parserutils.ParseExprs(exprStrings)`
`241`	`241`	`if err != nil {`
`242`	`242`	`return nil, catalog.TableColSet{}, err`
`243`	`243`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ import (`
`9`	`9`	`"context"`
`10`	`10`
`11`	`11`	`"github.com/cockroachdb/cockroach/pkg/sql/catalog"`
`12`		`- "github.com/cockroachdb/cockroach/pkg/sql/parser"`
	`12`	`+ "github.com/cockroachdb/cockroach/pkg/sql/parserutils"`
`13`	`13`	`"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"`
`14`	`14`	`"github.com/cockroachdb/cockroach/pkg/sql/sem/transform"`
`15`	`15`	`"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"`
`@@ -50,7 +50,7 @@ func MakeDefaultExprs(`
`50`	`50`	`exprStrings = append(exprStrings, col.GetDefaultExpr())`
`51`	`51`	`}`
`52`	`52`	`}`
`53`		`- exprs, err := parser.ParseExprs(exprStrings)`
	`53`	`+ exprs, err := parserutils.ParseExprs(exprStrings)`
`54`	`54`	`if err != nil {`
`55`	`55`	`return nil, err`
`56`	`56`	`}`