Skip to content

Commit c8d2027

Browse files
authored
implement double truncation and find correct hash comparison type (#3200)
1 parent 7db5799 commit c8d2027

File tree

8 files changed

+110
-9
lines changed

8 files changed

+110
-9
lines changed

enginetest/queries/join_queries.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,6 +1200,37 @@ var JoinScriptTests = []ScriptTest{
12001200
},
12011201
},
12021202
},
1203+
{
1204+
Name: "HashLookup with type int8 and string type conversions",
1205+
SetUpScript: []string{
1206+
"create table t1 (c1 boolean);",
1207+
"create table t2 (c2 varchar(500));",
1208+
"insert into t1 values (true), (false);",
1209+
"insert into t2 values ('abc'), ('def');", // will be converted to float64(0) and match false
1210+
"insert into t2 values ('1asdf');", // will be converted to '1' and match true
1211+
"insert into t2 values ('5five');", // will be converted to '5' and match nothing
1212+
},
1213+
Assertions: []ScriptTestAssertion{
1214+
{
1215+
// TODO: our warnings don't align with MySQL
1216+
Query: "select /*+ HASH_JOIN(t1, t2) */ * from t1 join t2 where c1 = c2 order by c1, c2;",
1217+
Expected: []sql.Row{
1218+
{0, "abc"},
1219+
{0, "def"},
1220+
{1, "1asdf"},
1221+
},
1222+
},
1223+
{
1224+
// TODO: our warnings don't align with MySQL
1225+
Query: "select /*+ INNER_JOIN(t1, t2) */ * from t1 join t2 where c1 = c2 order by c1, c2;",
1226+
Expected: []sql.Row{
1227+
{0, "abc"},
1228+
{0, "def"},
1229+
{1, "1asdf"},
1230+
},
1231+
},
1232+
},
1233+
},
12031234
}
12041235

12051236
var LateralJoinScriptTests = []ScriptTest{

enginetest/queries/queries.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,26 @@ var QueryTests = []QueryTest{
892892
Query: "SELECT 1 % true",
893893
Expected: []sql.Row{{"0"}},
894894
},
895+
{
896+
Query: "select 'abc' = false",
897+
Expected: []sql.Row{{true}},
898+
},
899+
{
900+
Query: "select '123abc' = 123",
901+
Expected: []sql.Row{{true}},
902+
},
903+
{
904+
Query: "select '1abc' = true",
905+
Expected: []sql.Row{{true}},
906+
},
907+
{
908+
Query: "select '123abc' = false",
909+
Expected: []sql.Row{{false}},
910+
},
911+
{
912+
Query: "select '123abc' = true",
913+
Expected: []sql.Row{{false}},
914+
},
895915
{
896916
Query: "SELECT * from mytable where (0.000 and true)",
897917
Expected: []sql.Row{},

sql/errors.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,9 @@ var (
947947
ErrColumnSpecifiedTwice = errors.NewKind("column '%v' specified twice")
948948

949949
ErrEnumTypeTruncated = errors.NewKind("new enum type change truncates value")
950+
951+
// ErrTruncatedIncorrect is thrown when converting a value results in portions of the data to be trimmed.
952+
ErrTruncatedIncorrect = errors.NewKind("Truncated incorrect %s value: %v")
950953
)
951954

952955
// CastSQLError returns a *mysql.SQLError with the error code and in some cases, also a SQL state, populated for the

sql/expression/convert.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,10 @@ func convertValue(ctx *sql.Context, val interface{}, castTo string, originType s
372372
}
373373
d, _, err := types.Float64.Convert(ctx, value)
374374
if err != nil {
375+
if sql.ErrTruncatedIncorrect.Is(err) {
376+
ctx.Warn(1265, "%s", err.Error())
377+
return d, nil
378+
}
375379
return types.Float64.Zero(), nil
376380
}
377381
return d, nil

sql/plan/hash_lookup.go

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@ import (
3434
// simply delegates to the child.
3535
func NewHashLookup(n sql.Node, rightEntryKey sql.Expression, leftProbeKey sql.Expression, joinType JoinType) *HashLookup {
3636
leftKeySch := hash.ExprsToSchema(leftProbeKey)
37+
compareType := GetCompareType(leftProbeKey.Type(), rightEntryKey.Type())
3738
return &HashLookup{
3839
UnaryNode: UnaryNode{n},
3940
RightEntryKey: rightEntryKey,
4041
LeftProbeKey: leftProbeKey,
42+
CompareType: compareType,
4143
Mutex: new(sync.Mutex),
4244
JoinType: joinType,
4345
leftKeySch: leftKeySch,
@@ -48,6 +50,7 @@ type HashLookup struct {
4850
UnaryNode
4951
RightEntryKey sql.Expression
5052
LeftProbeKey sql.Expression
53+
CompareType sql.Type
5154
Mutex *sync.Mutex
5255
Lookup *map[interface{}][]sql.Row
5356
leftKeySch sql.Schema
@@ -58,6 +61,46 @@ var _ sql.Node = (*HashLookup)(nil)
5861
var _ sql.Expressioner = (*HashLookup)(nil)
5962
var _ sql.CollationCoercible = (*HashLookup)(nil)
6063

64+
// GetCompareType returns the type to use when comparing values of types left and right.
65+
func GetCompareType(left, right sql.Type) sql.Type {
66+
// TODO: much of this logic is very similar to castLeftAndRight() from sql/expression/comparison.go
67+
// consider consolidating
68+
if left.Equals(right) {
69+
return left
70+
}
71+
if types.IsTuple(left) && types.IsTuple(right) {
72+
return left
73+
}
74+
if types.IsTime(left) || types.IsTime(right) {
75+
return types.DatetimeMaxPrecision
76+
}
77+
if types.IsJSON(left) || types.IsJSON(right) {
78+
return types.JSON
79+
}
80+
if types.IsBinaryType(left) || types.IsBinaryType(right) {
81+
return types.LongBlob
82+
}
83+
if types.IsNumber(left) || types.IsNumber(right) {
84+
if types.IsDecimal(left) {
85+
return left
86+
}
87+
if types.IsDecimal(right) {
88+
return right
89+
}
90+
if types.IsFloat(left) || types.IsFloat(right) {
91+
return types.Float64
92+
}
93+
if types.IsSigned(left) && types.IsSigned(right) {
94+
return types.Int64
95+
}
96+
if types.IsUnsigned(left) && types.IsUnsigned(right) {
97+
return types.Uint64
98+
}
99+
return types.Float64
100+
}
101+
return types.LongText
102+
}
103+
61104
func (n *HashLookup) Expressions() []sql.Expression {
62105
return []sql.Expression{n.RightEntryKey, n.LeftProbeKey}
63106
}
@@ -113,7 +156,7 @@ func (n *HashLookup) CollationCoercibility(ctx *sql.Context) (collation sql.Coll
113156
return sql.GetCoercibility(ctx, n.Child)
114157
}
115158

116-
// Convert a tuple expression returning []interface{} into something comparable.
159+
// GetHashKey converts a tuple expression returning []interface{} into something comparable.
117160
// Fast paths a few smaller slices into fixed size arrays, puts everything else
118161
// through string serialization and a hash for now. It is OK to hash lossy here
119162
// as the join condition is still evaluated after the matching rows are returned.
@@ -122,13 +165,13 @@ func (n *HashLookup) GetHashKey(ctx *sql.Context, e sql.Expression, row sql.Row)
122165
if err != nil {
123166
return nil, err
124167
}
125-
typ := n.LeftProbeKey.Type()
126-
key, _, err = typ.Convert(ctx, key)
168+
key, _, err = n.CompareType.Convert(ctx, key)
127169
if types.ErrValueNotNil.Is(err) {
128170
// The LHS expression was NullType. This is allowed.
129171
return nil, nil
130172
}
131-
if err != nil {
173+
if err != nil && !sql.ErrTruncatedIncorrect.Is(err) {
174+
// Truncated warning is already thrown elsewhere.
132175
return nil, err
133176
}
134177
if s, ok := key.([]interface{}); ok {
@@ -139,7 +182,7 @@ func (n *HashLookup) GetHashKey(ctx *sql.Context, e sql.Expression, row sql.Row)
139182
return string(k), nil
140183
}
141184

142-
return hash.HashOfSimple(ctx, key, typ)
185+
return hash.HashOfSimple(ctx, key, n.CompareType)
143186
}
144187

145188
func (n *HashLookup) Dispose() {

sql/types/conversion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ func TypeAwareConversion(ctx *sql.Context, val interface{}, originalType sql.Typ
766766
// ConvertOrTruncate converts the value |i| to type |t| and returns the converted value; if the value does not convert
767767
// cleanly and the type is automatically coerced (i.e. string and numeric types), then a warning is logged and the
768768
// value is truncated to the Zero value for type |t|. If the value does not convert and the type is not automatically
769-
// coerced, then an error is returned.
769+
// coerced, then return an error.
770770
func ConvertOrTruncate(ctx *sql.Context, i interface{}, t sql.Type) (interface{}, error) {
771771
converted, _, err := t.Convert(ctx, i)
772772
if err == nil {

sql/types/number.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ func (t NumberTypeImpl_) SQLUint64(ctx *sql.Context, dest []byte, v interface{})
530530

531531
func (t NumberTypeImpl_) SQLFloat64(ctx *sql.Context, dest []byte, v interface{}) ([]byte, error) {
532532
num, err := convertToFloat64(t, v)
533-
if err != nil {
533+
if err != nil && !sql.ErrTruncatedIncorrect.Is(err) {
534534
return nil, err
535535
}
536536
dest = strconv.AppendFloat(dest, num, 'g', -1, 64)
@@ -1543,7 +1543,7 @@ func convertToFloat64(t NumberTypeImpl_, v interface{}) (float64, error) {
15431543
// parse the first longest valid numbers
15441544
s := numre.FindString(v)
15451545
i, _ = strconv.ParseFloat(s, 64)
1546-
return i, sql.ErrInvalidValue.New(v, t.String())
1546+
return i, sql.ErrTruncatedIncorrect.New(t.String(), v)
15471547
}
15481548
return i, nil
15491549
case bool:

sql/types/number_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func TestNumberSQL_NumberFromString(t *testing.T) {
253253

254254
val, err = Float64.SQL(sql.NewEmptyContext(), nil, "also not a number")
255255
require.NoError(t, err)
256-
assert.Equal(t, "also not a number", val.ToString())
256+
assert.Equal(t, "0", val.ToString())
257257
}
258258

259259
func TestNumberString(t *testing.T) {

0 commit comments

Comments
 (0)