diff --git a/pkg/sql/logictest/testdata/logic_test/computed b/pkg/sql/logictest/testdata/logic_test/computed index a278f92425b7..000e37722df3 100644 --- a/pkg/sql/logictest/testdata/logic_test/computed +++ b/pkg/sql/logictest/testdata/logic_test/computed @@ -317,6 +317,20 @@ CREATE TABLE y ( b TIMESTAMP AS (a::TIMESTAMP) STORED ) +# Make sure the error has a hint that mentions AT TIME ZONE. +statement error context-dependent operators are not allowed in computed column.*\nHINT:.*AT TIME ZONE +CREATE TABLE y ( + a TIMESTAMPTZ, + b STRING AS (a::STRING) STORED +) + +# Make sure the error has a hint that mentions AT TIME ZONE. +statement error context-dependent operators are not allowed in computed column.*\nHINT:.*AT TIME ZONE +CREATE TABLE y ( + a TIMESTAMPTZ, + b TIMESTAMP AS (a::TIMESTAMP) STORED +) + statement error context-dependent operators are not allowed in computed column CREATE TABLE y ( a TIMESTAMPTZ, diff --git a/pkg/sql/sem/tree/casts.go b/pkg/sql/sem/tree/casts.go index bc040f56ed8d..b5633b58e7b7 100644 --- a/pkg/sql/sem/tree/casts.go +++ b/pkg/sql/sem/tree/casts.go @@ -42,6 +42,11 @@ type castInfo struct { to types.Family volatility Volatility + // volatilityHint is an optional string for VolatilityStable casts. When set, + // it is used as an error hint suggesting a possible workaround when stable + // casts are not allowed. + volatilityHint string + // Telemetry counter; set by init(). counter telemetry.Counter @@ -170,7 +175,10 @@ var validCasts = []castInfo{ {from: types.GeographyFamily, to: types.StringFamily, volatility: VolatilityImmutable}, {from: types.BytesFamily, to: types.StringFamily, volatility: VolatilityStable}, {from: types.TimestampFamily, to: types.StringFamily, volatility: VolatilityImmutable}, - {from: types.TimestampTZFamily, to: types.StringFamily, volatility: VolatilityStable}, + { + from: types.TimestampTZFamily, to: types.StringFamily, volatility: VolatilityStable, + volatilityHint: "TIMESTAMPTZ to STRING casts depend on the current timezone; consider using (t AT TIME ZONE 'UTC')::STRING instead.", + }, {from: types.IntervalFamily, to: types.StringFamily, volatility: VolatilityImmutable}, {from: types.UuidFamily, to: types.StringFamily, volatility: VolatilityImmutable}, {from: types.DateFamily, to: types.StringFamily, volatility: VolatilityImmutable}, @@ -246,11 +254,17 @@ var validCasts = []castInfo{ // Casts to TimestampFamily. {from: types.UnknownFamily, to: types.TimestampFamily, volatility: VolatilityImmutable}, - {from: types.StringFamily, to: types.TimestampFamily, volatility: VolatilityStable}, + { + from: types.StringFamily, to: types.TimestampFamily, volatility: VolatilityStable, + volatilityHint: "STRING to TIMESTAMP casts are context-dependent because of relative timestamp strings like 'now'; use parse_timestamp() instead.", + }, {from: types.CollatedStringFamily, to: types.TimestampFamily, volatility: VolatilityStable}, {from: types.DateFamily, to: types.TimestampFamily, volatility: VolatilityImmutable}, {from: types.TimestampFamily, to: types.TimestampFamily, volatility: VolatilityImmutable}, - {from: types.TimestampTZFamily, to: types.TimestampFamily, volatility: VolatilityStable}, + { + from: types.TimestampTZFamily, to: types.TimestampFamily, volatility: VolatilityStable, + volatilityHint: "TIMESTAMPTZ to TIMESTAMP casts depend on the current timezone; consider using AT TIME ZONE 'UTC' instead", + }, {from: types.IntFamily, to: types.TimestampFamily, volatility: VolatilityImmutable}, // Casts to TimestampTZFamily. diff --git a/pkg/sql/sem/tree/datum_invariants_test.go b/pkg/sql/sem/tree/datum_invariants_test.go index 5d39871c28a7..6f772cfe953d 100644 --- a/pkg/sql/sem/tree/datum_invariants_test.go +++ b/pkg/sql/sem/tree/datum_invariants_test.go @@ -22,7 +22,7 @@ func TestAllTypesCastableToString(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) for _, typ := range types.Scalar { - if ok, _, _ := isCastDeepValid(typ, types.String); !ok { + if err := resolveCast("", typ, types.String, true /* allowStable */); err != nil { t.Errorf("%s is not castable to STRING, all types should be", typ) } } @@ -32,7 +32,7 @@ func TestAllTypesCastableFromString(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) for _, typ := range types.Scalar { - if ok, _, _ := isCastDeepValid(types.String, typ); !ok { + if err := resolveCast("", types.String, typ, true /* allowStable */); err != nil { t.Errorf("%s is not castable from STRING, all types should be", typ) } } diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index d73cf66b3b8f..17ffca9e4aef 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -416,26 +416,47 @@ func (expr *CaseExpr) TypeCheck( return expr, nil } -func isCastDeepValid(castFrom, castTo *types.T) (bool, telemetry.Counter, Volatility) { +// resolveCast checks that the cast from the two types is valid. If allowStable +// is false, it also checks that the cast has VolatilityImmutable. +// +// On success, any relevant telemetry counters are incremented. +func resolveCast(context string, castFrom, castTo *types.T, allowStable bool) error { toFamily := castTo.Family() fromFamily := castFrom.Family() switch { case toFamily == types.ArrayFamily && fromFamily == types.ArrayFamily: - ok, c, v := isCastDeepValid(castFrom.ArrayContents(), castTo.ArrayContents()) - if ok { - telemetry.Inc(sqltelemetry.ArrayCastCounter) + err := resolveCast(context, castFrom.ArrayContents(), castTo.ArrayContents(), allowStable) + if err != nil { + return err } - return ok, c, v + telemetry.Inc(sqltelemetry.ArrayCastCounter) + return nil + case toFamily == types.EnumFamily && fromFamily == types.EnumFamily: - // Casts from ENUM to ENUM type can only succeed if the two enums - return castFrom.Equivalent(castTo), sqltelemetry.EnumCastCounter, VolatilityImmutable - } + // Casts from ENUM to ENUM type can only succeed if the two types are the + // same. + if !castFrom.Equivalent(castTo) { + return pgerror.Newf(pgcode.CannotCoerce, "invalid cast: %s -> %s", castFrom, castTo) + } + telemetry.Inc(sqltelemetry.EnumCastCounter) + return nil - cast := lookupCast(fromFamily, toFamily) - if cast == nil { - return false, nil, 0 + default: + cast := lookupCast(fromFamily, toFamily) + if cast == nil { + return pgerror.Newf(pgcode.CannotCoerce, "invalid cast: %s -> %s", castFrom, castTo) + } + if !allowStable && cast.volatility >= VolatilityStable { + err := NewContextDependentOpsNotAllowedError(context) + err = pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s::%s", castFrom, castTo) + if cast.volatilityHint != "" { + err = errors.WithHint(err, cast.volatilityHint) + } + return err + } + telemetry.Inc(cast.counter) + return nil } - return true, cast.counter, cast.volatility } func isEmptyArray(expr Expr) bool { @@ -495,22 +516,16 @@ func (expr *CastExpr) TypeCheck( } castFrom := typedSubExpr.ResolvedType() - - ok, c, volatility := isCastDeepValid(castFrom, exprType) - if !ok { - return nil, pgerror.Newf(pgcode.CannotCoerce, "invalid cast: %s -> %s", castFrom, exprType) + allowStable := true + context := "" + if semaCtx != nil && semaCtx.Properties.required.rejectFlags&RejectStableOperators != 0 { + allowStable = false + context = semaCtx.Properties.required.context } - if err := semaCtx.checkVolatility(volatility); err != nil { - err = pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s::%s", castFrom, exprType) - // Special cases where we can provide useful hints. - if castFrom.Family() == types.StringFamily && exprType.Family() == types.TimestampFamily { - err = errors.WithHint(err, "string to timestamp casts are context-dependent because "+ - "of relative timestamp strings like 'now'; use parse_timestamp() instead.") - } + err = resolveCast(context, castFrom, exprType, allowStable) + if err != nil { return nil, err } - - telemetry.Inc(c) expr.Expr = typedSubExpr expr.Type = exprType expr.typ = exprType