sql: check unsupported types during schema changes and type/function …

…creation

It is necessary to check the cluster version when building an expression
that references a newly-added type, in order to ensure that all nodes
support that type. Previously, these checks were omitted for casts in
the vectorized engine, expressions for partial index predicates and
check constraints, function parameters and return types, and user-defined
composite types.

This patch adds version-checking for function/procedure parameters and
return types, as well as for user-defined types. It also augments the
type-checking logic for casts and type annotations with a version
check; this handles the remaining cases. The execution-time checks
for cast expressions are left untouched, just in case the new
type-checking logic misses important cases.

For now, these changes only apply to the `PG_LSN` type, which will
be new in 23.2. A future commit will add support for `REFCURSOR`, and
will need to use the same checks.

Informs cockroachdb#111560

Release note: None

pkg/sql/backfill/backfill.go

@@ -236,6 +236,7 @@ func (cb *ColumnBackfiller) InitForDistributedUse(
 		// Set up a SemaContext to type check the default and computed expressions.
 		semaCtx := tree.MakeSemaContext()
 		semaCtx.TypeResolver = &resolver
+		semaCtx.Version = evalCtx.Settings.Version
 		var err error
 		defaultExprs, err = schemaexpr.MakeDefaultExprs(
 			ctx, cb.added, &transform.ExprTransformContext{}, evalCtx, &semaCtx,
@@ -670,6 +671,7 @@ func (ib *IndexBackfiller) InitForDistributedUse(
 		// Set up a SemaContext to type check the default and computed expressions.
 		semaCtx := tree.MakeSemaContext()
 		semaCtx.TypeResolver = &resolver
+		semaCtx.Version = evalCtx.Settings.Version
 		// Convert any partial index predicate strings into expressions.
 		predicates, colExprs, referencedColumns, err = constructExprs(
 			ctx, desc, ib.added, ib.cols, ib.addedCols, ib.computedCols, evalCtx, &semaCtx,

pkg/sql/create_type.go

@@ -423,6 +423,10 @@ func CreateCompositeTypeDesc(
 		if err != nil {
 			return nil, err
 		}
+		err = tree.CheckUnsupportedType(params.ctx, params.extendedEvalCtx.Settings.Version, typ)
+		if err != nil {
+			return nil, err
+		}
 		if typ.UserDefined() {
 			return nil, unimplemented.NewWithIssue(91779,
 				"composite types that reference user-defined types not yet supported")

pkg/sql/logictest/testdata/logic_test/pg_lsn_mixed

@@ -1,8 +1,124 @@
-# LogicTest: local-mixed-22.2-23.1
-# TODO(otan): add tests for mixed 23.1-23.2.
+# LogicTest: cockroach-go-testserver-upgrade-to-master
 
-statement error must be finalized to use pg_lsn
-SELECT '1010F/AAAA'::text::pg_lsn
+statement ok
+CREATE TABLE xy (x INT, y INT);
 
-statement error pg_lsn not supported until version 23.2
-CREATE TABLE pg_lsn_table(id pg_lsn, val pg_lsn)
+# ----------------------------------------------------------------------
+# Test PG_LSN references with all nodes running old binaries.
+# ----------------------------------------------------------------------
+
+# Cast to PG_LSN.
+statement error pgcode 0A000 unimplemented: this syntax
+SELECT 'foo'::PG_LSN;
+
+# Cast to PG_LSN using the vectorized engine.
+statement error pgcode 0A000 unimplemented: this syntax
+SELECT 'foo'::PG_LSN FROM generate_series(1, 100) LIMIT 1;
+
+# Table that references PG_LSN.
+statement error pgcode 0A000 unimplemented: this syntax
+CREATE TABLE t (x PG_LSN);
+
+# Add a PG_LSN column.
+statement error pgcode 0A000 unimplemented: this syntax
+ALTER TABLE xy ADD COLUMN curs PG_LSN;
+
+# Alter a column type to PG_LSN.
+statement ok
+SET enable_experimental_alter_column_type_general=true;
+
+statement error pgcode 0A000 unimplemented: this syntax
+ALTER TABLE xy ALTER COLUMN y TYPE PG_LSN;
+
+# Create a partial index that uses the PG_LSN type.
+statement error pgcode 0A000 unimplemented: this syntax
+CREATE INDEX part ON xy (x) WHERE y::PG_LSN < 'foo';
+
+# Add a check constraint that uses the PG_LSN type.
+statement error pgcode 0A000 unimplemented: this syntax
+ALTER TABLE xy ADD CONSTRAINT bar CHECK (y::PG_LSN < 'baz');
+
+# UDT that references PG_LSN.
+statement error pgcode 0A000 unimplemented: this syntax
+CREATE TYPE typ AS (x INT, y PG_LSN);
+
+# Function that returns PG_LSN.
+statement error pgcode 0A000 unimplemented: this syntax
+CREATE OR REPLACE FUNCTION f() RETURNS PG_LSN AS $$
+  SELECT 'foo';
+$$ LANGUAGE SQL;
+
+# Function that takes PG_LSN argument.
+statement error pgcode 0A000 unimplemented: this syntax
+CREATE OR REPLACE FUNCTION f(curs PG_LSN) RETURNS STRING AS $$
+  SELECT curs;
+$$ LANGUAGE SQL;
+
+# Function that references PG_LSN internally.
+statement error pgcode 0A000 unimplemented: this syntax
+CREATE OR REPLACE FUNCTION f() RETURNS INT AS $$
+  SELECT 'foo'::PG_LSN;
+  SELECT 0;
+$$ LANGUAGE SQL;
+
+# ----------------------------------------------------------------------
+# Verify that PG_LSN is not allowed after upgrading the gateway.
+# ----------------------------------------------------------------------
+
+upgrade 0
+
+user root nodeidx=0
+
+# Cast to PG_LSN.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+SELECT 'foo'::PG_LSN;
+
+# Cast to PG_LSN using the vectorized engine.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+SELECT 'foo'::PG_LSN FROM generate_series(1, 100) LIMIT 1;
+
+# Table that references PG_LSN.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+CREATE TABLE t (x PG_LSN);
+
+# Add a PG_LSN column.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+ALTER TABLE xy ADD COLUMN curs PG_LSN;
+
+# Alter a column type to PG_LSN.
+statement ok
+SET enable_experimental_alter_column_type_general=true;
+
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+ALTER TABLE xy ALTER COLUMN y TYPE PG_LSN;
+
+# Create a partial index that uses the PG_LSN type.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+CREATE INDEX part ON xy (x) WHERE y::PG_LSN < 'foo';
+
+# Add a check constraint that uses the PG_LSN type.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+ALTER TABLE xy ADD CONSTRAINT bar CHECK (y::PG_LSN < 'baz');
+
+# UDT that references PG_LSN.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+CREATE TYPE typ AS (x INT, y PG_LSN);
+
+# Function that returns PG_LSN.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+CREATE OR REPLACE FUNCTION f() RETURNS PG_LSN AS $$
+  SELECT 'foo';
+$$ LANGUAGE SQL;
+
+# Function that takes PG_LSN argument.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+CREATE OR REPLACE FUNCTION f(curs PG_LSN) RETURNS INT AS $$
+  SELECT 0;
+$$ LANGUAGE SQL;
+
+# Function that references PG_LSN internally.
+statement error pgcode 0A000 pq: pg_lsn not supported until version 23.2
+CREATE OR REPLACE FUNCTION f() RETURNS INT AS $$
+  SELECT 'foo'::PG_LSN;
+  SELECT 0;
+$$ LANGUAGE SQL;

pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/BUILD.bazel

@@ -15,7 +15,7 @@ go_test(
         "//pkg/sql/logictest:testdata",  # keep
     ],
     exec_properties = {"Pool": "large"},
-    shard_count = 11,
+    shard_count = 12,
     tags = [
         "cpu:2",
     ],

pkg/sql/opt/optbuilder/BUILD.bazel

@@ -50,6 +50,7 @@ go_library(
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/clusterversion",
         "//pkg/kv/kvserver/concurrency/isolation",
         "//pkg/server/telemetry",
         "//pkg/settings",

pkg/sql/opt/optbuilder/create_function.go

@@ -11,8 +11,10 @@
 package optbuilder
 
 import (
+	"context"
 	"fmt"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/funcinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
@@ -139,13 +141,16 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateRoutine, inScope *scope) (o
 	// named parameters to the scope so that references to them in the body can
 	// be resolved.
 	bodyScope := b.allocScope()
+	version := b.evalCtx.Settings.Version
 	var paramTypes tree.ParamTypes
 	for i := range cf.Params {
 		param := &cf.Params[i]
 		typ, err := tree.ResolveType(b.ctx, param.Type, b.semaCtx.TypeResolver)
 		if err != nil {
 			panic(err)
 		}
+		// The parameter type must be supported by the current cluster version.
+		checkUnsupportedType(b.ctx, version, typ)
 		if types.IsRecordType(typ) {
 			if language == tree.RoutineLangSQL {
 				panic(pgerror.Newf(pgcode.InvalidFunctionDefinition,
@@ -258,7 +263,7 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateRoutine, inScope *scope) (o
 		// TODO(mgartner): stmtScope.cols does not describe the result
 		// columns of the statement. We should use physical.Presentation
 		// instead.
-		err = validateReturnType(funcReturnType, stmtScope.cols)
+		err = validateReturnType(b.ctx, version, funcReturnType, stmtScope.cols)
 		if err != nil {
 			panic(err)
 		}
@@ -301,7 +306,15 @@ func formatFuncBodyStmt(fmtCtx *tree.FmtCtx, ast tree.NodeFormatter, newLine boo
 	fmtCtx.WriteString(";")
 }
 
-func validateReturnType(expected *types.T, cols []scopeColumn) error {
+func validateReturnType(
+	ctx context.Context, version clusterversion.Handle, expected *types.T, cols []scopeColumn,
+) error {
+	// The return type must be supported by the current cluster version.
+	checkUnsupportedType(ctx, version, expected)
+	for i := range cols {
+		checkUnsupportedType(ctx, version, cols[i].typ)
+	}
+
 	// If return type is void, any column types are valid.
 	if expected.Equivalent(types.Void) {
 		return nil
@@ -406,3 +419,9 @@ func checkStmtVolatility(
 		}
 	}
 }
+
+func checkUnsupportedType(ctx context.Context, version clusterversion.Handle, typ *types.T) {
+	if err := tree.CheckUnsupportedType(ctx, version, typ); err != nil {
+		panic(err)
+	}
+}

pkg/sql/planner.go

@@ -404,6 +404,7 @@ func newInternalPlanner(
 	p.semaCtx.NameResolver = p
 	p.semaCtx.DateStyle = sd.GetDateStyle()
 	p.semaCtx.IntervalStyle = sd.GetIntervalStyle()
+	p.semaCtx.Version = execCfg.Settings.Version
 
 	plannerMon := mon.NewMonitor(redact.Sprintf("internal-planner.%s.%s", user, opName),
 		mon.MemoryResource,
@@ -903,6 +904,7 @@ func (p *planner) resetPlanner(
 	p.semaCtx.NameResolver = p
 	p.semaCtx.DateStyle = sd.GetDateStyle()
 	p.semaCtx.IntervalStyle = sd.GetIntervalStyle()
+	p.semaCtx.Version = p.execCfg.Settings.Version
 
 	p.autoCommit = false
 

pkg/sql/schemachanger/scbuild/tree_context_builder.go

@@ -36,6 +36,7 @@ func newSemaCtx(d Dependencies) *tree.SemaContext {
 	semaCtx.NameResolver = d.CatalogReader()
 	semaCtx.DateStyle = d.SessionData().GetDateStyle()
 	semaCtx.IntervalStyle = d.SessionData().GetIntervalStyle()
+	semaCtx.Version = d.ClusterSettings().Version
 	return &semaCtx
 }
 

pkg/sql/sem/tree/BUILD.bazel

@@ -122,6 +122,7 @@ go_library(
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/sem/tree",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/clusterversion",
         "//pkg/col/coldata",
         "//pkg/col/typeconv",  # keep
         "//pkg/geo",

pkg/sql/sem/tree/type_check.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/cast"
@@ -75,6 +76,10 @@ type SemaContext struct {
 	DateStyle pgdate.DateStyle
 	// IntervalStyle refers to the IntervalStyle to parse as.
 	IntervalStyle duration.IntervalStyle
+
+	// Version accesses the current cluster version. It is used to verify that
+	// types are supported by the current cluster version. It may be unset.
+	Version clusterversion.Handle
 }
 
 // SemaProperties is a holder for required and derived properties
@@ -599,6 +604,9 @@ func (expr *CastExpr) TypeCheck(
 	if err != nil {
 		return nil, err
 	}
+	if err = CheckUnsupportedType(ctx, semaCtx.Version, exprType); err != nil {
+		return nil, err
+	}
 	expr.Type = exprType
 	canElideCast := true
 	switch {
@@ -739,6 +747,9 @@ func (expr *AnnotateTypeExpr) TypeCheck(
 	if err != nil {
 		return nil, err
 	}
+	if err = CheckUnsupportedType(ctx, semaCtx.Version, annotateType); err != nil {
+		return nil, err
+	}
 	expr.Type = annotateType
 	subExpr, err := typeCheckAndRequire(
 		ctx,
@@ -3411,3 +3422,22 @@ func getMostSignificantOverload(
 	}
 	return ret, nil
 }
+
+// CheckUnsupportedType returns an error if the given type is not supported by
+// the current cluster version. If the given version handle is nil,
+// CheckUnsupportedType returns nil.
+func CheckUnsupportedType(ctx context.Context, version clusterversion.Handle, typ *types.T) error {
+	if version == nil {
+		return nil
+	}
+	var errorTypeString string
+	if typ.Family() == types.PGLSNFamily {
+		errorTypeString = "pg_lsn"
+	}
+	if errorTypeString != "" && !version.IsActive(ctx, clusterversion.V23_2) {
+		return pgerror.Newf(pgcode.FeatureNotSupported,
+			"%s not supported until version 23.2", errorTypeString,
+		)
+	}
+	return nil
+}

pkg/sql/types/types.go

@@ -2941,7 +2941,6 @@ var postgresPredefinedTypeIssues = map[string]int{
 	"macaddr8":      45813,
 	"money":         41578,
 	"path":          21286,
-	"pg_lsn":        -1,
 	"txid_snapshot": -1,
 	"xml":           43355,
 }