Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/1125#discussion_r169860310
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchSizer.java
---
@@ -245,16 +251,30 @@ private void buildVectorInitializer(VectorInitializer
initializer) {
else if (width > 0) {
initializer.variableWidth(name, width);
}
+
--- End diff --
There may be a but in the original code above this line.
```
String name = prefix + metadata.getName();
...
initializer.variableWidthArray(name, ...
```
First problem: this method does not recurse into the child vectors of maps.
Second, if it did, since each initializer is supposed to hold a name relative
to its parent tuple (row or map), the prefix is not needed.
The vector initializer code was originally created for sort and none of the
tests for sort include maps, so I probably did not catch the fact that maps
don't work correctly with the original code.
Suggestion: add unit tests for this stuff. This code is getting very
complex and tests would be very helpful to catch these subtle bugs.
Disclaimer: this is all supposed to be replaced with the ResultSetLoader
code. The vector allocation code there is much cleaner, does handle maps, and
has been unit tested.
---
