Add support for groupArray(String) #43
Conversation
Support groupArray for string as well
Add a simple test for Array(String)
|
Please add a short description, what is the purpose of this PR |
encoder_test.go
Outdated
| @@ -83,6 +83,7 @@ func TestTextDecoder(t *testing.T) { | |||
| {"Enum16('one'=1)", "'one'", "one"}, | |||
| {"Array(UInt32)", "[1,2]", []uint32{1, 2}}, | |||
| {"Array(UInt32)", "[]", []uint32{}}, | |||
| {"Array(String)", "['one','one\\'']", []string{"one", "one'"}}, | |||
There was a problem hiding this comment.
this test passes on current HEAD without patch for method func (d *textDecoder) Decode(t string, value []byte) (driver.Value, error)
|
@bgaifullin update the test to actually show the issue 👍 |
encoder.go
Outdated
| if len(v) > 2 { | ||
| // check if array is string encoded (['example']) | ||
| if len(v) > 4 && v[1] == '\'' && v[len(v)-2] == '\'' { | ||
| items = strings.Split(v[2:len(v)-2], "','") |
There was a problem hiding this comment.
I think fix should be a little be more complicated, otherwise it brokes the parsing arrays of Datetime.
Need to check that subtype is String or FixedString and need to unescape for each item of array of strings.
|
@bgaifullin Should be good for another look |
encoder.go
Outdated
| // check that array is not empty ([]) | ||
| if len(v) > 2 { | ||
| subType := t[6 : len(t)-1] | ||
| // check if array is string encoded (['example']) |
There was a problem hiding this comment.
IMHO: check if array of strings and not empty will be a little bit clearly
encoder.go
Outdated
| for i, v := range items { | ||
| items[i] = unescape(v) | ||
| } | ||
| } else if len(v) > 2 { // check that array is not empty ([]) |
There was a problem hiding this comment.
IMHO, this condition can be checked at first, before all other staff.
- check that array is not empty.
- check that array of strings
- other cases
There was a problem hiding this comment.
Updated to what I think you had in mind? If not, if you want to include the ifs, am happy to add into PR.
|
LGTM, thanks |
This PR updates the decode function to split on comma surrounded by quotes instead of only comma for string arrays in order to avoid splitting inside of the returned value (updated test to demonstrate).