PR feedback Pt 2

go/cmd/dolt/cli/arg_helpers.go

@@ -145,7 +145,7 @@ func ParseKeyValues(nbf *types.NomsBinFormat, sch schema.Schema, args []string)
 
 	convFuncs := make(map[uint64]func(*string) (types.Value, error))
 	err := sch.GetPKCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) {
-		if col.TypeInfo.Equals(typeinfo.StringDefaultType) {
+		if typeinfo.IsStringType(col.TypeInfo) {
 			convFuncs[tag] = func(v *string) (types.Value, error) {
 				return types.String(*v), nil
 			}

go/cmd/dolt/commands/select_where.go

@@ -69,7 +69,7 @@ func ParseWhere(sch schema.Schema, whereClause string) (FilterFn, error) {
 		}
 
 		var val types.Value
-		if cols[0].TypeInfo.Equals(typeinfo.StringDefaultType) {
+		if typeinfo.IsStringType(cols[0].TypeInfo) {
 			val = types.String(valStr)
 		} else {
 			var err error

go/libraries/doltcore/rowconv/row_converter.go

@@ -61,7 +61,7 @@ func NewRowConverter(mapping *FieldMapping) (*RowConverter, error) {
 				return v, nil
 			}
 		}
-		if destCol.TypeInfo.Equals(typeinfo.StringDefaultType) {
+		if typeinfo.IsStringType(destCol.TypeInfo) {
 			convFuncs[srcTag] = func(v types.Value) (types.Value, error) {
 				val, err := srcCol.TypeInfo.FormatValue(v)
 				if err != nil {
@@ -74,15 +74,7 @@ func NewRowConverter(mapping *FieldMapping) (*RowConverter, error) {
 			}
 		} else {
 			convFuncs[srcTag] = func(v types.Value) (types.Value, error) {
-				str, err := srcCol.TypeInfo.FormatValue(v)
-				if err != nil {
-					return nil, err
-				}
-				val, err := destCol.TypeInfo.ParseValue(str)
-				if err != nil {
-					return nil, err
-				}
-				return val, nil
+				return typeinfo.Convert(v, srcCol.TypeInfo, destCol.TypeInfo)
 			}
 		}
 	}

go/libraries/doltcore/schema/alterschema/addcolumn.go

@@ -187,8 +187,8 @@ func validateNewColumn(ctx context.Context, tbl *doltdb.Table, tag uint64, newCo
 		}
 	}
 
-	if !types.IsNull(defaultVal) && defaultVal.Kind() != typeInfo.NomsKind() {
-		return fmt.Errorf("Type of default value (%v) doesn't match type of column (%v)", types.KindToString[defaultVal.Kind()], types.KindToString[typeInfo.NomsKind()])
+	if !types.IsNull(defaultVal) && !typeInfo.IsValid(defaultVal) {
+		return fmt.Errorf("Default value (%v) is invalid for column (%v)", defaultVal, typeInfo.String())
 	}
 
 	return nil

go/libraries/doltcore/schema/alterschema/addcolumn_test.go

@@ -266,7 +266,7 @@ func TestAddColumnToTable(t *testing.T) {
 			colKind:     types.IntKind,
 			nullable:    NotNull,
 			defaultVal:  types.String("this shouldn't work"),
-			expectedErr: "Type of default value (String) doesn't match type of column (Int)",
+			expectedErr: "Default value (this shouldn't work) is invalid for column (Int64)",
 		},
 	}
 

go/libraries/doltcore/schema/typeinfo/typeinfo.go

@@ -88,7 +88,7 @@ type TypeInfo interface {
 	// serialization and deserialization of type information.
 	GetTypeParams() map[string]string
 
-	// IsValid takes in a value (go or Noms) and returns whether the value is valid for this type.
+	// IsValid takes in a types.Value and returns whether it is valid for this type.
 	IsValid(v types.Value) bool
 
 	// NomsKind returns the NomsKind that best matches this TypeInfo.
@@ -305,6 +305,26 @@ func FromKind(kind types.NomsKind) TypeInfo {
 	}
 }
 
+// Convert takes in a types.Value, as well as the source and destination TypeInfos, and
+// converts the TypeInfo into the applicable types.Value.
+func Convert(v types.Value, srcTi TypeInfo, destTi TypeInfo) (types.Value, error) {
+	str, err := srcTi.FormatValue(v)
+	if err != nil {
+		return nil, err
+	}
+	val, err := destTi.ParseValue(str)
+	if err != nil {
+		return nil, err
+	}
+	return val, nil
+}
+
+// IsStringType returns whether the given TypeInfo represents a CHAR, VARCHAR, or TEXT-derivative.
+func IsStringType(ti TypeInfo) bool {
+	_, ok := ti.(*varStringType)
+	return ok
+}
+
 // ParseIdentifier takes in an Identifier in string form and returns the matching Identifier.
 // Returns UnknownTypeIdentifier when the string match is not found.
 func ParseIdentifier(name string) Identifier {

go/libraries/doltcore/schema/typeinfo/varstring.go

@@ -91,6 +91,10 @@ func (ti *varStringType) ConvertNomsValueToValue(v types.Value) (interface{}, er
 		if !ok {
 			return nil, fmt.Errorf(`"%v" has unexpectedly encountered a value of type "%T" from embedded type`, ti.String(), v)
 		}
+		// As per the MySQL documentation, trailing spaces are removed when retrieved for CHAR types only.
+		// go-mysql-server does not currently have a concept of storage nor retrieval for its types, thus it must be
+		// implemented here. This function is used to retrieve dolt values, hence its inclusion here and not elsewhere.
+		// https://dev.mysql.com/doc/refman/8.0/en/char.html
 		if ti.sqlStringType.Type() == sqltypes.Char {
 			res = strings.TrimRightFunc(res, unicode.IsSpace)
 		}