Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134060544
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
---
@@ -359,30 +361,109 @@ public Mutator(OperatorExecContext oContext,
BufferAllocator allocator, VectorCo
public <T extends ValueVector> T addField(MaterializedField field,
Class<T> clazz) throws
SchemaChangeException {
// Check if the field exists.
- ValueVector v = fieldVectorMap.get(field.getPath());
- if (v == null || v.getClass() != clazz) {
+ ValueVector vector = fieldVectorMap.get(field.getName());
+ ValueVector childVector = vector;
+ // if a vector does not exist yet, creates value vector, or if it
exists and has map type, omit this code
+ if (vector == null || (vector.getClass() != clazz
+ && (vector.getField().getType().getMinorType() != MinorType.MAP
+ || field.getType().getMinorType() != MinorType.MAP))) {
// Field does not exist--add it to the map and the output
container.
- v = TypeHelper.getNewVector(field, allocator, callBack);
- if (!clazz.isAssignableFrom(v.getClass())) {
+ vector = TypeHelper.getNewVector(field, allocator, callBack);
+ childVector = vector;
+ // gets inner field if the map was created the first time
+ if (field.getType().getMinorType() == MinorType.MAP) {
+ childVector = getChildVectorByField(vector, field);
+ } else if (!clazz.isAssignableFrom(vector.getClass())) {
throw new SchemaChangeException(
String.format(
"The class that was provided, %s, does not correspond to
the "
+ "expected vector type of %s.",
- clazz.getSimpleName(), v.getClass().getSimpleName()));
+ clazz.getSimpleName(),
vector.getClass().getSimpleName()));
}
- final ValueVector old = fieldVectorMap.put(field.getPath(), v);
+ final ValueVector old = fieldVectorMap.put(field.getName(),
vector);
if (old != null) {
old.clear();
container.remove(old);
}
- container.add(v);
+ container.add(vector);
// Added new vectors to the container--mark that the schema has
changed.
schemaChanged = true;
}
+ // otherwise, checks that field and existing vector have a map type
--- End diff --
Does this ever actually happen? It would require that the client
1. Have access to the chain of map fields (since maps can contain maps can
contain maps...)
2. Hold onto the root map.
3. When needing a new field, first check the child list in the materialized
field of the leaf map (there is no such check at present.)
4. If the field is missing, add it to the materialized field for the map.
5. Pass in the root map to this method.
The above is very unlikely. And, indeed, this code won't work if we have
nested maps (a map within a map.)
Probably what actually happens is that this method is called only for
columns in the root tuple (the row.) If I want to add a column to a child map
(at any nesting depth), I'll call the `addOrGet` method on the map field.
So, I think there is a faulty assumption here. I think we should assume
that, if the field is a map, and it is new, then the child list must be empty.
Map fields are then added to the map directly, they are not added via this
method.
Note that the new "result set loader" takes care of all this complexity...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
