ppadma 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_r211063693
##########
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:
do we need both versions of promoteToUnion ? please fix the comments. Both
of them say old behavior. Also, if we want to keep both of them, can we name
them better ? may be promoteToUnionNew instead of promoteToUnion2 ?
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
