From 5eba198fa623f07a8215a1177722d626f41d9bac Mon Sep 17 00:00:00 2001 From: Evan Wall Date: Tue, 1 Mar 2022 17:04:24 -0500 Subject: [PATCH] Unary Complement execution has different results when the parameters are different fixes https://github.com/cockroachdb/cockroach/issues/74493 Release note (sql change): Return ambiguous unary operator error for ambiguous input like ~'1' which can be interpreted as an integer (resulting in -2) or a bit string (resulting in 0). Release justification: Improves a confusing error message saying that an operator is invalid instead of ambiguous. --- .../logictest/testdata/logic_test/operator | 44 +++++++++++++++++++ pkg/sql/sem/tree/overload.go | 11 +++++ pkg/sql/sem/tree/overload_test.go | 4 +- pkg/sql/sem/tree/type_check_test.go | 2 +- 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/operator b/pkg/sql/logictest/testdata/logic_test/operator index c55255e9c31e..8766db95715a 100644 --- a/pkg/sql/logictest/testdata/logic_test/operator +++ b/pkg/sql/logictest/testdata/logic_test/operator @@ -3,3 +3,47 @@ SELECT |/ -1.0::float query error cannot take square root of a negative number SELECT |/ -1.0::decimal + +query I +SELECT ~-1; +---- +0 + +query I +SELECT ~0; +---- +-1 + +query I +SELECT ~1; +---- +-2 + +query I +SELECT ~2; +---- +-3 + +query T +SELECT ~B'0'; +---- +1 + +query T +SELECT ~B'1'; +---- +0 + +statement error lexical error +SELECT ~B'2'; + +statement error ambiguous unary operator +SELECT ~'0'; + +statement error ambiguous unary operator +SELECT ~'1'; + +query I +SELECT ~2; +---- +-3 diff --git a/pkg/sql/sem/tree/overload.go b/pkg/sql/sem/tree/overload.go index 5415911a0c41..7a5a1ec741a4 100644 --- a/pkg/sql/sem/tree/overload.go +++ b/pkg/sql/sem/tree/overload.go @@ -750,6 +750,10 @@ func typeCheckOverloadedExprs( // The fourth heuristic is to prefer candidates that accepts the "best" // mutual type in the resolvable type set of all constants. if bestConstType, ok := commonConstantType(s.exprs, s.constIdxs); ok { + // In case all overloads are filtered out at this step, + // keep track of previous overload indexes to return ambiguous error (>1 overloads) + // instead of unsupported error (0 overloads) when applicable. + prevOverloadIdxs := s.overloadIdxs for _, i := range s.constIdxs { s.overloadIdxs = filterOverloads(s.overloads, s.overloadIdxs, func(o overloadImpl) bool { @@ -757,6 +761,13 @@ func typeCheckOverloadedExprs( }) } if ok, typedExprs, fns, err := checkReturn(ctx, semaCtx, &s); ok { + if len(fns) == 0 { + var overloadImpls []overloadImpl + for i := range prevOverloadIdxs { + overloadImpls = append(overloadImpls, s.overloads[i]) + } + return typedExprs, overloadImpls, err + } return typedExprs, fns, err } if homogeneousTyp != nil { diff --git a/pkg/sql/sem/tree/overload_test.go b/pkg/sql/sem/tree/overload_test.go index 0525efdd8081..5402e64c9a74 100644 --- a/pkg/sql/sem/tree/overload_test.go +++ b/pkg/sql/sem/tree/overload_test.go @@ -187,7 +187,7 @@ func TestTypeCheckOverloadedExprs(t *testing.T) { {nil, []Expr{intConst("1")}, []overloadImpl{unaryIntFn, unaryIntFn}, ambiguous, false}, {nil, []Expr{intConst("1")}, []overloadImpl{unaryIntFn, unaryFloatFn}, unaryIntFn, false}, {nil, []Expr{decConst("1.0")}, []overloadImpl{unaryIntFn, unaryDecimalFn}, unaryDecimalFn, false}, - {nil, []Expr{decConst("1.0")}, []overloadImpl{unaryIntFn, unaryFloatFn}, unsupported, false}, + {nil, []Expr{decConst("1.0")}, []overloadImpl{unaryIntFn, unaryFloatFn}, ambiguous, false}, {nil, []Expr{intConst("1")}, []overloadImpl{unaryIntFn, binaryIntFn}, unaryIntFn, false}, {nil, []Expr{intConst("1")}, []overloadImpl{unaryFloatFn, unaryStringFn}, unaryFloatFn, false}, {nil, []Expr{intConst("1")}, []overloadImpl{unaryStringFn, binaryIntFn}, unsupported, false}, @@ -249,7 +249,7 @@ func TestTypeCheckOverloadedExprs(t *testing.T) { {nil, []Expr{NewDInt(1), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, binaryIntFn, false}, {nil, []Expr{NewDFloat(1), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, unsupported, false}, {nil, []Expr{intConst("1"), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, binaryIntFn, false}, - {nil, []Expr{decConst("1.0"), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, unsupported, false}, // Limitation. + {nil, []Expr{decConst("1.0"), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, ambiguous, false}, // Limitation. {types.Date, []Expr{NewDInt(1), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, binaryIntDateFn, false}, {types.Date, []Expr{NewDFloat(1), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, unsupported, false}, {types.Date, []Expr{intConst("1"), placeholder(1)}, []overloadImpl{binaryIntFn, binaryIntDateFn}, binaryIntDateFn, false}, diff --git a/pkg/sql/sem/tree/type_check_test.go b/pkg/sql/sem/tree/type_check_test.go index e30c0b7abceb..14e7e30b2389 100644 --- a/pkg/sql/sem/tree/type_check_test.go +++ b/pkg/sql/sem/tree/type_check_test.go @@ -244,7 +244,7 @@ func TestTypeCheckError(t *testing.T) { expr string expected string }{ - {`'1' + '2'`, `unsupported binary operator:`}, + {`'1' + '2'`, `ambiguous binary operator:`}, {`'a' + 0`, `unsupported binary operator:`}, {`1.1 # 3.1`, `unsupported binary operator:`}, {`~0.1`, `unsupported unary operator:`},