tabletserver: allow string bind vars for int cols

PHP is not very good at differentiating strings from ints. So,
we have to be more forgiving, especially when strings are received
where ints are expected.
For now, I'm allowing just this. Other implicit type conversions
are unsafe. In particular, 0x... notation is treated differently
in MySQL for strings.

go/sqltypes/value.go

@@ -108,6 +108,26 @@ func BuildValue(goval interface{}) (v Value, err error) {
 	return v, nil
 }
 
+// BuildConverted is like BuildValue except that it tries to
+// convert a string or []byte to an integral if the target type
+// is an integral. We don't perform other implicit conversions
+// because they're unsafe.
+func BuildConverted(typ querypb.Type, goval interface{}) (v Value, err error) {
+	if IsIntegral(typ) {
+		switch goval := goval.(type) {
+		case []byte:
+			return ValueFromBytes(typ, goval)
+		case string:
+			return ValueFromBytes(typ, []byte(goval))
+		case Value:
+			if goval.IsQuoted() {
+				return ValueFromBytes(typ, goval.Raw())
+			}
+		}
+	}
+	return BuildValue(goval)
+}
+
 // ValueFromBytes builds a Value using typ and val. It ensures that val
 // matches the requested type. If type is an integral it's converted to
 // a cannonical form. Otherwise, the original representation is preserved.

go/sqltypes/value_test.go

@@ -105,6 +105,48 @@ func TestBuildValue(t *testing.T) {
 	}
 }
 
+func TestBuildConverted(t *testing.T) {
+	testcases := []struct {
+		typ querypb.Type
+		val interface{}
+		out Value
+	}{{
+		typ: Int64,
+		val: 123,
+		out: testVal(Int64, "123"),
+	}, {
+		typ: Int64,
+		val: "123",
+		out: testVal(Int64, "123"),
+	}, {
+		typ: Uint64,
+		val: "123",
+		out: testVal(Uint64, "123"),
+	}, {
+		typ: Int64,
+		val: []byte("123"),
+		out: testVal(Int64, "123"),
+	}, {
+		typ: Int64,
+		val: testVal(VarBinary, "123"),
+		out: testVal(Int64, "123"),
+	}, {
+		typ: Int64,
+		val: testVal(Float32, "123"),
+		out: testVal(Float32, "123"),
+	}}
+	for _, tcase := range testcases {
+		v, err := BuildConverted(tcase.typ, tcase.val)
+		if err != nil {
+			t.Errorf("BuildValue(%v, %#v) error: %v", tcase.typ, tcase.val, err)
+			continue
+		}
+		if !reflect.DeepEqual(v, tcase.out) {
+			t.Errorf("BuildValue(%v, %#v) = %v, want %v", tcase.typ, tcase.val, makePretty(v), makePretty(tcase.out))
+		}
+	}
+}
+
 const (
 	InvalidNeg = "-9223372036854775809"
 	MinNeg     = "-9223372036854775808"

go/vt/tabletserver/codex.go

@@ -98,7 +98,7 @@ func resolveListArg(col *schema.TableColumn, key string, bindVars map[string]int
 	list := val.([]interface{})
 	resolved := make([]sqltypes.Value, len(list))
 	for i, v := range list {
-		sqlval, err := sqltypes.BuildValue(v)
+		sqlval, err := sqltypes.BuildConverted(col.Type, v)
 		if err != nil {
 			return nil, NewTabletError(ErrFail, vtrpcpb.ErrorCode_BAD_INPUT, "%v", err)
 		}
@@ -139,7 +139,7 @@ func resolveValue(col *schema.TableColumn, value interface{}, bindVars map[strin
 		if err != nil {
 			return result, NewTabletError(ErrFail, vtrpcpb.ErrorCode_BAD_INPUT, "%v", err)
 		}
-		sqlval, err := sqltypes.BuildValue(val)
+		sqlval, err := sqltypes.BuildConverted(col.Type, val)
 		if err != nil {
 			return result, NewTabletError(ErrFail, vtrpcpb.ErrorCode_BAD_INPUT, "%v", err)
 		}

go/vt/tabletserver/codex_test.go

@@ -66,7 +66,7 @@ func TestCodexBuildValuesList(t *testing.T) {
 	// type mismatch int
 	bindVars["pk1"] = "str"
 	pkValues = []interface{}{":pk1"}
-	wantErr = "error: type mismatch, expecting numeric type for str"
+	wantErr = "error: strconv.ParseInt"
 
 	got, err = buildValueList(&tableInfo, pkValues, bindVars)
 	if err == nil || !strings.Contains(err.Error(), wantErr) {
@@ -210,12 +210,16 @@ func TestCodexResolvePKValues(t *testing.T) {
 
 	pkValues := make([]interface{}, 0, 10)
 	pkValues = append(pkValues, []interface{}{":" + key})
-	// resolvePKValues fail because type mismatch. pk column 0 has int type but
-	// list variables are strings.
-	_, _, err := resolvePKValues(&tableInfo, pkValues, bindVariables)
-	testUtils.checkTabletError(t, err, ErrFail, "type mismatch")
-	// pkValues is a list of sqltypes.Value and bypasses bind variables.
-	// But, the type mismatches, pk column 0 is int but variable is string.
+	// resolvePKValues should succeed for strings that can be converted to int.
+	v, _, err := resolvePKValues(&tableInfo, pkValues, bindVariables)
+	if err != nil {
+		t.Error(err)
+	}
+	wantV := []interface{}{[]sqltypes.Value{sqltypes.MakeTrusted(sqltypes.Int64, []byte("1"))}}
+	if !reflect.DeepEqual(v, wantV) {
+		t.Errorf("reslovePKValues: %#v, want %#v", v, wantV)
+	}
+	// resolvePKValues should fail because of conversion error.
 	pkValues = make([]interface{}, 0, 10)
 	pkValues = append(pkValues, sqltypes.MakeString([]byte("type_mismatch")))
 	_, _, err = resolvePKValues(&tableInfo, pkValues, nil)
@@ -248,9 +252,16 @@ func TestCodexResolveListArg(t *testing.T) {
 	_, err := resolveListArg(tableInfo.GetPKColumn(0), "::"+key, bindVariables)
 	testUtils.checkTabletError(t, err, ErrFail, "")
 
+	// This should successfully convert.
 	bindVariables[key] = []interface{}{"1"}
-	_, err = resolveListArg(tableInfo.GetPKColumn(0), "::"+key, bindVariables)
-	testUtils.checkTabletError(t, err, ErrFail, "type mismatch")
+	v, err := resolveListArg(tableInfo.GetPKColumn(0), "::"+key, bindVariables)
+	if err != nil {
+		t.Error(err)
+	}
+	wantV := []sqltypes.Value{sqltypes.MakeTrusted(sqltypes.Int64, []byte("1"))}
+	if !reflect.DeepEqual(v, wantV) {
+		t.Errorf("reslovePKValues: %#v, want %#v", v, wantV)
+	}
 
 	bindVariables[key] = []interface{}{10}
 	result, err := resolveListArg(tableInfo.GetPKColumn(0), "::"+key, bindVariables)

go/vt/tabletserver/endtoend/cache_case_test.go

@@ -44,6 +44,36 @@ func TestCacheCases1(t *testing.T) {
 			Table:        "vitess_cached1",
 			Hits:         1,
 		},
+		// (1)
+		&framework.TestCase{
+			Name:  "PK_IN int bind var",
+			Query: "select * from vitess_cached1 where eid = :eid",
+			BindVars: map[string]interface{}{
+				"eid": 1,
+			},
+			Result: [][]string{
+				{"1", "a", "abcd"},
+			},
+			RowsAffected: 1,
+			Plan:         "PK_IN",
+			Table:        "vitess_cached1",
+			Hits:         1,
+		},
+		// (1)
+		&framework.TestCase{
+			Name:  "PK_IN string bind var",
+			Query: "select * from vitess_cached1 where eid = :eid",
+			BindVars: map[string]interface{}{
+				"eid": "1",
+			},
+			Result: [][]string{
+				{"1", "a", "abcd"},
+			},
+			RowsAffected: 1,
+			Plan:         "PK_IN",
+			Table:        "vitess_cached1",
+			Hits:         1,
+		},
 		// (1, 3)
 		&framework.TestCase{
 			Name:  "PK_IN (empty cache)",

go/vt/tabletserver/endtoend/cache_test.go

@@ -280,27 +280,33 @@ func TestCacheTypes(t *testing.T) {
 	badRequests := []struct {
 		query string
 		bv    map[string]interface{}
+		out   string
 	}{{
 		query: "select * from vitess_cached2 where eid = 'str' and bid = 'str'",
+		out:   "error: type mismatch",
 	}, {
 		query: "select * from vitess_cached2 where eid = :str and bid = :str",
 		bv:    map[string]interface{}{"str": "str"},
+		out:   "error: strconv.ParseInt",
 	}, {
 		query: "select * from vitess_cached2 where eid = 1 and bid = 1",
+		out:   "error: type mismatch",
 	}, {
 		query: "select * from vitess_cached2 where eid = :id and bid = :id",
 		bv:    map[string]interface{}{"id": 1},
+		out:   "error: type mismatch",
 	}, {
 		query: "select * from vitess_cached2 where eid = 1.2 and bid = 1.2",
+		out:   "error: type mismatch",
 	}, {
 		query: "select * from vitess_cached2 where eid = :fl and bid = :fl",
 		bv:    map[string]interface{}{"fl": 1.2},
+		out:   "error: type mismatch",
 	}}
-	want := "error: type mismatch"
-	for _, request := range badRequests {
-		_, err := client.Execute(request.query, request.bv)
-		if err == nil || !strings.HasPrefix(err.Error(), want) {
-			t.Errorf("Error: %v, want %s", err, want)
+	for _, tcase := range badRequests {
+		_, err := client.Execute(tcase.query, tcase.bv)
+		if err == nil || !strings.HasPrefix(err.Error(), tcase.out) {
+			t.Errorf("%s: %v, want %s", tcase.query, err, tcase.out)
 		}
 	}
 }

go/vt/tabletserver/endtoend/compatibility_test.go

@@ -393,46 +393,55 @@ func TestTypeLimits(t *testing.T) {
 		}
 	}
 
-	want := "error: type mismatch"
 	mismatchCases := []struct {
 		query string
 		bv    map[string]interface{}
+		out   string
 	}{{
 		query: "insert into vitess_ints(tiny) values('str')",
 		bv:    nil,
+		out:   "error: type mismatch",
 	}, {
 		query: "insert into vitess_ints(tiny) values(:str)",
 		bv:    map[string]interface{}{"str": "str"},
+		out:   "error: strconv.ParseInt",
 	}, {
 		query: "insert into vitess_ints(tiny) values(1.2)",
 		bv:    nil,
+		out:   "error: type mismatch",
 	}, {
 		query: "insert into vitess_ints(tiny) values(:fl)",
 		bv:    map[string]interface{}{"fl": 1.2},
+		out:   "error: type mismatch",
 	}, {
 		query: "insert into vitess_strings(vb) values(1)",
 		bv:    nil,
+		out:   "error: type mismatch",
 	}, {
 		query: "insert into vitess_strings(vb) values(:id)",
 		bv:    map[string]interface{}{"id": 1},
+		out:   "error: type mismatch",
 	}, {
 		query: "insert into vitess_strings(vb) select tiny from vitess_ints",
 		bv:    nil,
+		out:   "error: type mismatch",
 	}, {
 		query: "insert into vitess_ints(tiny) select num from vitess_fracts",
 		bv:    nil,
+		out:   "error: type mismatch",
 	}, {
 		query: "insert into vitess_ints(tiny) select vb from vitess_strings",
 		bv:    nil,
+		out:   "error: type mismatch",
 	}}
-	for _, request := range mismatchCases {
-		_, err := client.Execute(request.query, request.bv)
-		if err == nil || !strings.HasPrefix(err.Error(), want) {
-			t.Errorf("Error(%s): %v, want %s", request.query, err, want)
+	for _, tcase := range mismatchCases {
+		_, err := client.Execute(tcase.query, tcase.bv)
+		if err == nil || !strings.HasPrefix(err.Error(), tcase.out) {
+			t.Errorf("Error(%s): %v, want %s", tcase.query, err, tcase.out)
 		}
 	}
 
-	want = "error: Out of range"
+	want := "error: Out of range"
 	for _, query := range []string{
 		"insert into vitess_ints(tiny) values(-129)",
 		"insert into vitess_ints(tiny) select medium from vitess_ints",