fix: /inserts-stream endpoint now supports nested types #5621
Conversation
| private Result coerceArray(final Object value, final SqlArray targetType) { | ||
| final ListObject list; | ||
| if (value instanceof List<?>) { | ||
| list = JavaListObject.of((List<?>) value); |
There was a problem hiding this comment.
I don't think it's necessary to create special classes JavaListObject/JsonListObject etc.
Can just call getList() on the JsonArray to get the List. This should simplify things quite a bit?
There was a problem hiding this comment.
I had this in an earlier version but it fails for structs nested inside arrays/maps because once getList() is called on the JsonArray, extracting a nested struct-type comes back as a LinkedHashMap rather than a JsonObject. When the LinkedHashMap is passed to coerceStruct(), coerceStruct() throws an exception because DefaultSqlValueCoercer is intended to fail if a caller attempts to coerce a map to a struct.
The other solutions I could think of for working around this were more complex than having the special classes JavaListObject/JsonListObject/etc.
| @@ -167,4 +208,162 @@ private static Result coerceMap(final Object value, final SqlMap targetType) { | |||
|
|
|||
| return Result.of(coerced); | |||
| } | |||
|
|
|||
| private interface ListObject { | |||
There was a problem hiding this comment.
As previous comment, I don't think these classes are necessary (?)
There was a problem hiding this comment.
As above.
Merging this for now, will open a follow-up if it turns out I've missed something. Thanks!
Description
Fixes #5620 and adds a lot more test coverage, not only for structs for also for other nested types: array of arrays, array of maps, array of structs, map of arrays, map of maps, map of structs, etc.
This fix turned out to be a lot larger than I had hoped in order to accommodate the following constraints of the /inserts-stream endpoint:
Testing done
Unit + integration tests.
Reviewer checklist