Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tree: allow string concat with any non-array type (anynonarray approach) #43143

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/generated/sql/operators.md
Original file line number Diff line number Diff line change
Expand Up @@ -414,11 +414,13 @@
<table><thead>
<tr><td><code>||</code></td><td>Return</td></tr>
</thead><tbody>
<tr><td>anynonarray <code>||</code> <a href="string.html">string</a></td><td><a href="string.html">string</a></td></tr>
<tr><td><a href="bool.html">bool</a> <code>||</code> <a href="bool.html">bool[]</a></td><td><a href="bool.html">bool[]</a></td></tr>
<tr><td><a href="bool.html">bool[]</a> <code>||</code> <a href="bool.html">bool</a></td><td><a href="bool.html">bool[]</a></td></tr>
<tr><td><a href="bool.html">bool[]</a> <code>||</code> <a href="bool.html">bool[]</a></td><td><a href="bool.html">bool[]</a></td></tr>
<tr><td><a href="bytes.html">bytes</a> <code>||</code> <a href="bytes.html">bytes</a></td><td><a href="bytes.html">bytes</a></td></tr>
<tr><td><a href="bytes.html">bytes</a> <code>||</code> <a href="bytes.html">bytes[]</a></td><td><a href="bytes.html">bytes[]</a></td></tr>
<tr><td><a href="bytes.html">bytes</a> <code>||</code> <a href="string.html">string</a></td><td><a href="bytes.html">bytes</a></td></tr>
<tr><td><a href="bytes.html">bytes[]</a> <code>||</code> <a href="bytes.html">bytes</a></td><td><a href="bytes.html">bytes[]</a></td></tr>
<tr><td><a href="bytes.html">bytes[]</a> <code>||</code> <a href="bytes.html">bytes[]</a></td><td><a href="bytes.html">bytes[]</a></td></tr>
<tr><td><a href="date.html">date</a> <code>||</code> <a href="date.html">date[]</a></td><td><a href="date.html">date[]</a></td></tr>
Expand All @@ -441,6 +443,8 @@
<tr><td><a href="interval.html">interval[]</a> <code>||</code> <a href="interval.html">interval[]</a></td><td><a href="interval.html">interval[]</a></td></tr>
<tr><td>jsonb <code>||</code> jsonb</td><td>jsonb</td></tr>
<tr><td>oid <code>||</code> oid</td><td>oid</td></tr>
<tr><td><a href="string.html">string</a> <code>||</code> anynonarray</td><td><a href="string.html">string</a></td></tr>
<tr><td><a href="string.html">string</a> <code>||</code> <a href="bytes.html">bytes</a></td><td><a href="bytes.html">bytes</a></td></tr>
<tr><td><a href="string.html">string</a> <code>||</code> <a href="string.html">string</a></td><td><a href="string.html">string</a></td></tr>
<tr><td><a href="string.html">string</a> <code>||</code> <a href="string.html">string[]</a></td><td><a href="string.html">string[]</a></td></tr>
<tr><td><a href="string.html">string[]</a> <code>||</code> <a href="string.html">string</a></td><td><a href="string.html">string[]</a></td></tr>
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/builtins/pg_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func makeNotUsableFalseBuiltin() builtinDefinition {
// the existence of this map.
var typeBuiltinsHaveUnderscore = map[oid.Oid]struct{}{
types.Any.Oid(): {},
types.AnyNonArray.Oid(): {},
types.AnyArray.Oid(): {},
types.Date.Oid(): {},
types.Time.Oid(): {},
Expand Down
18 changes: 17 additions & 1 deletion pkg/sql/sem/tree/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func (op *BinOp) returnType() ReturnTyper {
return op.retType
}

func (*BinOp) preferred() bool {
func (op *BinOp) preferred() bool {
return false
}

Expand Down Expand Up @@ -1411,6 +1411,22 @@ var BinOps = map[BinaryOperator]binOpOverload{
return NewDString(string(MustBeDString(left) + MustBeDString(right))), nil
},
},
&BinOp{
LeftType: types.String,
RightType: types.AnyNonArray,
ReturnType: types.String,
Fn: func(_ *EvalContext, left Datum, right Datum) (Datum, error) {
return NewDString(string(MustBeDString(left)) + right.String()), nil
},
},
&BinOp{
LeftType: types.AnyNonArray,
RightType: types.String,
ReturnType: types.String,
Fn: func(_ *EvalContext, left Datum, right Datum) (Datum, error) {
return NewDString(left.String() + string(MustBeDString(right))), nil
},
},
&BinOp{
LeftType: types.Bytes,
RightType: types.Bytes,
Expand Down
23 changes: 22 additions & 1 deletion pkg/sql/sem/tree/overload.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ func typeCheckOverloadedExprs(
if len(overloads) > math.MaxUint8 {
return nil, nil, errors.AssertionFailedf("too many overloads (%d > 255)", len(overloads))
}

var s typeCheckOverloadState
s.exprs = exprs
s.overloads = overloads
Expand Down Expand Up @@ -649,6 +648,21 @@ func typeCheckOverloadedExprs(
}
}

// We apply another heuristic to try remove any attempts where the overload has Any.
// We prefer more strictly typed options if available.
if ok, typedExprs, fns, err := filterAttempt(ctx, &s, func() {
s.overloadIdxs = filterOverloads(s.overloads, s.overloadIdxs, func(o overloadImpl) bool {
for _, typ := range o.params().Types() {
if typ.Family() == types.AnyFamily {
return false
}
}
return true
})
}); ok {
return typedExprs, fns, err
}

// In a binary expression, in the case of one of the arguments being untyped NULL,
// we prefer overloads where we infer the type of the NULL to be the same as the
// other argument. This is used to differentiate the behavior of
Expand Down Expand Up @@ -684,6 +698,13 @@ func typeCheckOverloadedExprs(
}
s.overloadIdxs = filterOverloads(s.overloads, s.overloadIdxs,
func(o overloadImpl) bool {
// If any side is Any, this filtering will not be useful
// for whittling down types. Ignore them.
// This is equivalent to adding the AnyFamily check above.
if o.params().GetAt(0).Family() == types.AnyFamily ||
o.params().GetAt(1).Family() == types.AnyFamily {
return false
}
return o.params().GetAt(0).Equivalent(leftType) &&
o.params().GetAt(1).Equivalent(rightType)
})
Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/sem/tree/testdata/eval/concat
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ eval
----
'a3'

eval
b'hello' || b'world'
----
'\x68656c6c6f776f726c64'

eval
b'hello' || 'world'
----
Expand All @@ -19,3 +24,25 @@ eval
array['foo'] || '{a,b}'
----
ARRAY['foo','a','b']

# String || any; any || String

eval
'b' || (5)::char || (8)::char || 'c'
----
'b58c'

eval
3 || 'a' || 3
----
3a3

eval
3::oid || 'a' || 3::oid
----
3a3

eval
3.33 || 'a' || 3.33
----
3.33a3.33
2 changes: 1 addition & 1 deletion pkg/sql/sqlbase/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ var (
func init() {
for _, typ := range types.OidToType {
switch typ.Oid() {
case oid.T_unknown, oid.T_anyelement:
case oid.T_unknown, oid.T_anyelement, oid.T_anynonarray:
// Don't include these.
case oid.T_anyarray, oid.T_oidvector, oid.T_int2vector:
// Include these.
Expand Down
21 changes: 21 additions & 0 deletions pkg/sql/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,12 @@ var (
Any = &T{InternalType: InternalType{
Family: AnyFamily, Oid: oid.T_anyelement, Locale: &emptyLocale}}

// AnyNonArray is a special type used only during static analysis as a wildcard
// type that matches non-array types.
// Execution-time values should never have this type.
AnyNonArray = &T{InternalType: InternalType{
Family: AnyFamily, Oid: oid.T_anynonarray, Locale: &emptyLocale}}

// AnyArray is a special type used only during static analysis as a wildcard
// type that matches an array having elements of any (uniform) type (including
// nested array types). Execution-time values should never have this type.
Expand Down Expand Up @@ -884,6 +890,10 @@ func (t *T) TupleLabels() []string {
func (t *T) Name() string {
switch t.Family() {
case AnyFamily:
switch t.Oid() {
case oid.T_anynonarray:
return "anynonarray"
}
return "anyelement"
case ArrayFamily:
switch t.Oid() {
Expand Down Expand Up @@ -1021,6 +1031,10 @@ func (t *T) SQLStandardNameWithTypmod(haveTypmod bool, typmod int) string {
var buf strings.Builder
switch t.Family() {
case AnyFamily:
switch t.Oid() {
case oid.T_anynonarray:
return "anynonarray"
}
return "anyelement"
case ArrayFamily:
switch t.Oid() {
Expand Down Expand Up @@ -1273,6 +1287,13 @@ func (t *T) SQLString() string {
// types. And a wildcard collation (empty string) matches any other collation.
func (t *T) Equivalent(other *T) bool {
if t.Family() == AnyFamily || other.Family() == AnyFamily {
// Non array families must not have an array family on the other side.
if t == AnyNonArray && other.Family() == ArrayFamily {
return false
}
if other == AnyNonArray && t.Family() == ArrayFamily {
return false
}
return true
}
if t.Family() != other.Family() {
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ func TestEquivalent(t *testing.T) {
}{
// ARRAY
{Int2Vector, IntArray, true},
{Int2Vector, AnyNonArray, false},
{OidVector, MakeArray(Oid), true},
{MakeArray(Int), MakeArray(Int4), true},
{MakeArray(String), MakeArray(MakeChar(10)), true},
Expand All @@ -394,6 +395,7 @@ func TestEquivalent(t *testing.T) {
{MakeBit(1), MakeBit(2), true},
{MakeBit(1), MakeVarBit(2), true},
{MakeVarBit(10), Any, true},
{MakeVarBit(10), AnyNonArray, true},
{VarBit, Bytes, false},

// COLLATEDSTRING
Expand All @@ -413,13 +415,15 @@ func TestEquivalent(t *testing.T) {
{Int2, Int4, true},
{Int4, Int, true},
{Int, Any, true},
{Int, AnyNonArray, true},
{Int, IntArray, false},

// TUPLE
{MakeTuple([]T{}), MakeTuple([]T{}), true},
{MakeTuple([]T{*Int, *String}), MakeTuple([]T{*Int4, *VarChar}), true},
{MakeTuple([]T{*Int, *String}), AnyTuple, true},
{AnyTuple, MakeTuple([]T{*Int, *String}), true},
{AnyNonArray, MakeTuple([]T{*Int, *String}), true},
{MakeTuple([]T{*Int, *String}),
MakeLabeledTuple([]T{*Int4, *VarChar}, []string{"label2", "label1"}), true},
{MakeLabeledTuple([]T{*Int, *String}, []string{"label1", "label2"}),
Expand All @@ -430,6 +434,7 @@ func TestEquivalent(t *testing.T) {
{Unknown, &T{InternalType: InternalType{
Family: UnknownFamily, Oid: oid.T_unknown, Locale: &emptyLocale}}, true},
{Any, Unknown, true},
{AnyNonArray, Unknown, true},
{Unknown, Int, false},
}

Expand Down