Skip to content

Commit

Permalink
sql: fix casts between REG* types
Browse files Browse the repository at this point in the history 
The newly introduced `castMap` does not contain entries for casts
between all combinations of REG* types, which is consistent with
Postgres, but inconsistent with behavior in versions up to 21.2 where
these casts are allowed.

The `castMap` changes result in more than just backward incompatibility.
We allow branches of CASE statements to be equivalent types (i.e., types
in the same family), like `REGCLASS` and `REGTYPE`, and we automatically
add casts to a query plan to support this. However, because these casts
don't exist in the `castMap`, internal errors are raised when we try to
fetch the volatility of the cast while building logical properties.

According to Postgres's type conversion rules for CASE, we should only
allow branches to be different types if they can be implicitly cast to
the first non-NULL branch. Implicit casts between REG* types are not
allowed, so CASE expressions with branches of different REG* types
should result in a user error like `CASE/WHEN could not convert type
regclass to regtype`. However, this is a much larger project and the
change will not be fully backward compatible. This work is tracked by
issue cockroachdb#75103.

For now, this commit adds casts between REG* types to the `castMap` to
maintain backward compatibility and prevent an internal error.

There is no release note because this bug does not exist in any
releases.

Fixes cockroachdb#74784

Release note: None
  • Loading branch information
@mgartner
mgartner committed Jan 19, 2022
1 parent bed5b79 commit d09d141
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 21 deletions.
37 changes: 37 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/cast
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1342,3 +1342,40 @@ INSERT INTO t59489 (d4_2) VALUES (600)

statement error value with precision 4, scale 2 must round to an absolute value less than 10\^2
INSERT INTO t59489 (d4_2) VALUES (6e2)

# Regression for #74784. Case expressions with branches of different types in
# the OID family should not cause internal errors.
# TODO(#75103): These CASE expressions are allowed for backward compatibility,
# but ultimately should not allow expressions such as these because implicit
# casts between REG* are not allowed.
statement ok
select CASE WHEN false THEN 1::REGCLASS ELSE 2::REGNAMESPACE END;
select CASE WHEN false THEN 1::REGCLASS ELSE 2::REGPROC END;
select CASE WHEN false THEN 1::REGCLASS ELSE 2::REGPROCEDURE END;
select CASE WHEN false THEN 1::REGCLASS ELSE 2::REGROLE END;
select CASE WHEN false THEN 1::REGCLASS ELSE 2::REGTYPE END;
select CASE WHEN false THEN 1::REGNAMESPACE ELSE 2::REGCLASS END;
select CASE WHEN false THEN 1::REGNAMESPACE ELSE 2::REGPROC END;
select CASE WHEN false THEN 1::REGNAMESPACE ELSE 2::REGPROCEDURE END;
select CASE WHEN false THEN 1::REGNAMESPACE ELSE 2::REGROLE END;
select CASE WHEN false THEN 1::REGNAMESPACE ELSE 2::REGTYPE END;
select CASE WHEN false THEN 1::REGPROC ELSE 2::REGCLASS END;
select CASE WHEN false THEN 1::REGPROC ELSE 2::REGNAMESPACE END;
select CASE WHEN false THEN 1::REGPROC ELSE 2::REGPROCEDURE END;
select CASE WHEN false THEN 1::REGPROC ELSE 2::REGROLE END;
select CASE WHEN false THEN 1::REGPROC ELSE 2::REGTYPE END;
select CASE WHEN false THEN 1::REGPROCEDURE ELSE 2::REGCLASS END;
select CASE WHEN false THEN 1::REGPROCEDURE ELSE 2::REGNAMESPACE END;
select CASE WHEN false THEN 1::REGPROCEDURE ELSE 2::REGPROC END;
select CASE WHEN false THEN 1::REGPROCEDURE ELSE 2::REGROLE END;
select CASE WHEN false THEN 1::REGPROCEDURE ELSE 2::REGTYPE END;
select CASE WHEN false THEN 1::REGROLE ELSE 2::REGCLASS END;
select CASE WHEN false THEN 1::REGROLE ELSE 2::REGNAMESPACE END;
select CASE WHEN false THEN 1::REGROLE ELSE 2::REGPROC END;
select CASE WHEN false THEN 1::REGROLE ELSE 2::REGPROCEDURE END;
select CASE WHEN false THEN 1::REGROLE ELSE 2::REGTYPE END;
select CASE WHEN false THEN 1::REGTYPE ELSE 2::REGCLASS END;
select CASE WHEN false THEN 1::REGTYPE ELSE 2::REGNAMESPACE END;
select CASE WHEN false THEN 1::REGTYPE ELSE 2::REGPROC END;
select CASE WHEN false THEN 1::REGTYPE ELSE 2::REGPROCEDURE END;
select CASE WHEN false THEN 1::REGTYPE ELSE 2::REGROLE END;
70 changes: 49 additions & 21 deletions pkg/sql/sem/tree/cast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,10 +738,15 @@ var castMap = map[oid.Oid]map[oid.Oid]cast{
},
oid.T_regclass: {
// TODO(mgartner): Casts to INT2 should not be allowed.
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regnamespace: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regproc: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regprocedure: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regrole: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regtype: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
// Automatic I/O conversions to string types.
oid.T_bpchar: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
oid.T_char: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
Expand All @@ -751,10 +756,15 @@ var castMap = map[oid.Oid]map[oid.Oid]cast{
},
oid.T_regnamespace: {
// TODO(mgartner): Casts to INT2 should not be allowed.
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regclass: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regproc: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regprocedure: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regrole: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regtype: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
// Automatic I/O conversions to string types.
oid.T_bpchar: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
oid.T_char: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
Expand All @@ -769,6 +779,10 @@ var castMap = map[oid.Oid]map[oid.Oid]cast{
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regprocedure: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regclass: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regnamespace: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regrole: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regtype: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
// Automatic I/O conversions to string types.
oid.T_bpchar: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
oid.T_char: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
Expand All @@ -778,11 +792,15 @@ var castMap = map[oid.Oid]map[oid.Oid]cast{
},
oid.T_regprocedure: {
// TODO(mgartner): Casts to INT2 should not be allowed.
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regproc: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regproc: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regclass: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regnamespace: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regrole: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regtype: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
// Automatic I/O conversions to string types.
oid.T_bpchar: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
oid.T_char: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
Expand All @@ -792,10 +810,15 @@ var castMap = map[oid.Oid]map[oid.Oid]cast{
},
oid.T_regrole: {
// TODO(mgartner): Casts to INT2 should not be allowed.
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regclass: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regnamespace: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regproc: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regprocedure: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regtype: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
// Automatic I/O conversions to string types.
oid.T_bpchar: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
oid.T_char: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
Expand All @@ -805,10 +828,15 @@ var castMap = map[oid.Oid]map[oid.Oid]cast{
},
oid.T_regtype: {
// TODO(mgartner): Casts to INT2 should not be allowed.
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int2: {maxContext: CastContextAssignment, origin: contextOriginLegacyConversion, volatility: VolatilityImmutable},
oid.T_int4: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_int8: {maxContext: CastContextAssignment, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_oid: {maxContext: CastContextImplicit, origin: contextOriginPgCast, volatility: VolatilityImmutable},
oid.T_regclass: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regnamespace: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regproc: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regprocedure: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
oid.T_regrole: {maxContext: CastContextExplicit, origin: contextOriginLegacyConversion, volatility: VolatilityStable},
// Automatic I/O conversions to string types.
oid.T_bpchar: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
oid.T_char: {maxContext: CastContextAssignment, origin: contextOriginAutomaticIOConversion, volatility: VolatilityStable},
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/sem/tree/type_check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2308,6 +2308,8 @@ func TypeCheckSameTypedExprs(
if err != nil {
return nil, nil, err
}
// TODO(#75103): For UNION, CASE, and related expressions we should
// only allow types that can be implicitly cast to firstValidType.
if typ := typedExpr.ResolvedType(); !(typ.Equivalent(firstValidType) || typ.Family() == types.UnknownFamily) {
return nil, nil, unexpectedTypeError(exprs[i], firstValidType, typ)
}
Expand Down

0 comments on commit d09d141

Please sign in to comment.