From d09d14178a72f7625d0869902b7087a3f1ee3415 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Tue, 18 Jan 2022 17:30:47 -0500 Subject: [PATCH] sql: fix casts between REG* types 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 #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 #74784 Release note: None --- pkg/sql/logictest/testdata/logic_test/cast | 37 ++++++++++++ pkg/sql/sem/tree/cast.go | 70 +++++++++++++++------- pkg/sql/sem/tree/type_check.go | 2 + 3 files changed, 88 insertions(+), 21 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/cast b/pkg/sql/logictest/testdata/logic_test/cast index 184c2f459326..402d361afe6c 100644 --- a/pkg/sql/logictest/testdata/logic_test/cast +++ b/pkg/sql/logictest/testdata/logic_test/cast @@ -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; diff --git a/pkg/sql/sem/tree/cast.go b/pkg/sql/sem/tree/cast.go index 671eb7a4a5e0..3eae323db49a 100644 --- a/pkg/sql/sem/tree/cast.go +++ b/pkg/sql/sem/tree/cast.go @@ -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}, @@ -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}, @@ -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}, @@ -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}, @@ -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}, @@ -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}, diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index 2400747e6a1e..e6d3009676c0 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -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) }