sql/mysql: fix binary with size #869
Conversation
There was a problem hiding this comment.
@calvin2021y, thanks for the contribution!
Can you please add a test case that will serve both as documentation for the behavior and enforce it doesn't break in the future? Thanks
| @@ -186,7 +186,7 @@ func TestMySQL_AddColumns(t *testing.T) { | |||
| &schema.Column{Name: "b", Type: &schema.ColumnType{Raw: "mediumblob", Type: &schema.BinaryType{T: "mediumblob"}}}, | |||
| &schema.Column{Name: "c", Type: &schema.ColumnType{Raw: "blob", Type: &schema.BinaryType{T: "blob"}}}, | |||
| &schema.Column{Name: "d", Type: &schema.ColumnType{Raw: "longblob", Type: &schema.BinaryType{T: "longblob"}}}, | |||
| &schema.Column{Name: "e", Type: &schema.ColumnType{Raw: "binary", Type: &schema.BinaryType{T: "binary"}}}, | |||
| &schema.Column{Name: "e", Type: &schema.ColumnType{Raw: "binary(255)", Type: &schema.BinaryType{T: "binary(255)"}}}, | |||
There was a problem hiding this comment.
Please add a case instead of deleting an existing one
There was a problem hiding this comment.
I am not sure why it not pass without a size.
If I don't give a size it will provide a default non-zero size for binary ?
There was a problem hiding this comment.
If an existing test fails because of a change that was made, it generally should mean that the change is not backward compatible (i.e it changes the existing behavior of the system).
There was a problem hiding this comment.
Just checked it with both MySQL 8 and 5.7 and that's the expected behavior.
Version 8
Version 5.7
There was a problem hiding this comment.
If this is the case, I think I should remove this line ?
&schema.Column{Name: "e", Type: &schema.ColumnType{Raw: "binary", Type: &schema.BinaryType{T: "binary"}}},
fix #751