paul-rogers commented on a change in pull request #1429: DRILL-6676: Add Union,
List and Repeated List types to Result Set Loader
URL: https://github.com/apache/drill/pull/1429#discussion_r211115976
##########
File path:
exec/vector/src/main/java/org/apache/drill/exec/vector/complex/ListVector.java
##########
@@ -250,22 +270,140 @@ public void load(UserBitShared.SerializedField
metadata, DrillBuf buffer) {
bits.load(bitMetadata, buffer.slice(offsetLength, bitLength));
final UserBitShared.SerializedField vectorMetadata = metadata.getChild(2);
- if (getDataVector() == DEFAULT_DATA_VECTOR) {
+ if (isEmptyType()) {
addOrGetVector(VectorDescriptor.create(vectorMetadata.getMajorType()));
}
final int vectorLength = vectorMetadata.getBufferLength();
vector.load(vectorMetadata, buffer.slice(offsetLength + bitLength,
vectorLength));
}
+ public boolean isEmptyType() {
+ return getDataVector() == DEFAULT_DATA_VECTOR;
+ }
+
+ @Override
+ public void setChildVector(ValueVector childVector) {
+
+ // Unlike the repeated list vector, the (plain) list vector
+ // adds the dummy vector as a child type.
+
+ assert field.getChildren().size() == 1;
+ assert field.getChildren().iterator().next().getType().getMinorType() ==
MinorType.LATE;
+ field.removeChild(vector.getField());
+
+ super.setChildVector(childVector);
+
+ // Initial LATE type vector not added as a subtype initially.
+ // So, no need to remove it, just add the new subtype. Since the
+ // MajorType is immutable, must build a new one and replace the type
+ // in the materialized field. (We replace the type, rather than creating
+ // a new materialized field, to preserve the link to this field from
+ // a parent map, list or union.)
+
+ assert field.getType().getSubTypeCount() == 0;
+ field.replaceType(
+ field.getType().toBuilder()
+ .addSubType(childVector.getField().getType().getMinorType())
+ .build());
+ }
+
+ /**
+ * Promote the list to a union. Called from old-style writers. This
implementation
+ * relies on the caller to set the types vector for any existing values.
+ * This method simply clears the existing vector.
+ *
+ * @return the new union vector
+ */
+
public UnionVector promoteToUnion() {
- MaterializedField newField =
MaterializedField.create(getField().getName(), Types.optional(MinorType.UNION));
- UnionVector vector = new UnionVector(newField, allocator, null);
+ UnionVector vector = createUnion();
+
+ // Replace the current vector, clearing its data. (This is the
+ // old behavior.
+
replaceDataVector(vector);
reader = new UnionListReader(this);
return vector;
}
+ /**
+ * Revised form of promote to union that correctly fixes up the list
+ * field metadata to match the new union type.
+ *
+ * @return the new union vector
+ */
+
+ public UnionVector promoteToUnion2() {
Review comment:
We do need both versions because I've not removed the old "complex writer"
classes. Given the excellent work you and others did on the batch sizing stuff,
it is unlikely that we'll use result set loader for internal operators anytime
soon, so we'll need to retain the older complex writers, and so we need to
retain the older `promoteToUnion()` version.
Revised the comments and renamed the method to attempt to make this a bit
clearer.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
