Skip to content

Commit

Permalink
sql: add hints for timestamptz cast errors
Browse files Browse the repository at this point in the history
This adds hint to the error that you get if you try to cast a
TIMESTAMPTZ to TIMESTAMP or STRING in a computed column expression.

Release note: None
  • Loading branch information
RaduBerinde committed Jun 15, 2021
1 parent 9c583b9 commit 2defc06
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 30 deletions.
14 changes: 14 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/computed
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 17 additions & 3 deletions pkg/sql/sem/tree/casts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/sem/tree/datum_invariants_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down
65 changes: 40 additions & 25 deletions pkg/sql/sem/tree/type_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2defc06

Please sign in to comment.