[
https://issues.apache.org/jira/browse/DRILL-6373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16508224#comment-16508224
]
ASF GitHub Bot commented on DRILL-6373:
---------------------------------------
vrozov commented on issue #1244: DRILL-6373: Refactor Result Set Loader for
Union, List support
URL: https://github.com/apache/drill/pull/1244#issuecomment-396287379
@paul-rogers Please track the stack trace for the modification part. There
is an attempt to add a child to the children of a vector that is part of the
`incoming` batch. Please see `PartitionerTemplate.java:381`:
```
ValueVector outgoingVector = TypeHelper.getNewVector(v.getField(),
allocator);
```
The existing assumption is that `v.getField()` is read-only (immutable), but
it is passed to `AbstractMapVector` that iterates over passed `field` children
instead of using cloned version. Then,
`child` is passed to `NullableVarCharVector` that again modifies passed
parameter.
Please consider changing constructor of vectors to clone passed in field and
using cloned version to add child:
```
BaseValueVector.java (add clone()):
protected BaseValueVector(MaterializedField field, BufferAllocator
allocator) {
this.field = Preconditions.checkNotNull(field, "field cannot be
null").*clone*();
this.allocator = Preconditions.checkNotNull(allocator, "allocator cannot
be null");
}
NullableVarCharVector.java (use cloned version):
public NullableVarCharVector(MaterializedField field, BufferAllocator
allocator) {
super(field, allocator);
// replace field with it's clone
*field = getField();*
// The values vector has the same type and attributes
// as the nullable vector, but with a mode of required. This ensures that
// things like scale and precision are preserved in the values vector.
// For backward compatibility, the values vector must have the same
// name as the enclosing vector.
// Setting the child to REQUIRED is a change, prior it was OPTIONAL. But
// if we then use the field to create a vector, we get the wrong type
// (we get an optional child vector when we want required.)
values = new VarCharVector(
MaterializedField.create(field.getName(),
field.getType().toBuilder()
.setMode(DataMode.REQUIRED)
.build()),
allocator);
field.addChild(bits.getField());
field.addChild(values.getField());
accessor = new Accessor();
}
AbstractMapVector.java (use cloned version):
protected AbstractMapVector(MaterializedField field, BufferAllocator
allocator, CallBack callBack) {
super(field.clone(), allocator, callBack);
// replace field with it's clone
*field = getField();*
// create the hierarchy of the child vectors based on the materialized
field
for (MaterializedField child : field.getChildren()) {
if
(child.getName().equals(BaseRepeatedValueVector.OFFSETS_FIELD.getName())) {
continue;
}
final String fieldName = child.getName();
final ValueVector v = BasicTypeHelper.getNewVector(child, allocator,
callBack);
putVector(fieldName, v);
}
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Refactor the Result Set Loader to prepare for Union, List support
> -----------------------------------------------------------------
>
> Key: DRILL-6373
> URL: https://issues.apache.org/jira/browse/DRILL-6373
> Project: Apache Drill
> Issue Type: Improvement
> Affects Versions: 1.13.0
> Reporter: Paul Rogers
> Assignee: Paul Rogers
> Priority: Major
> Fix For: 1.14.0
>
>
> As the next step in merging the "batch sizing" enhancements, refactor the
> {{ResultSetLoader}} and related classes to prepare for Union and List
> support. This fix follows the refactoring of the column accessors for the
> same purpose. Actual Union and List support is to follow in a separate PR.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
