andygrove commented on code in PR #1419:
URL: https://github.com/apache/arrow-java/pull/1419#discussion_r4514570100
##########
vector/src/main/codegen/templates/DenseUnionVector.java:
##########
@@ -518,15 +518,12 @@ public void reset() {
@Override
public Field getField() {
- int childCount = (int) Arrays.stream(typeFields).filter(field -> field !=
null).count();
- List<org.apache.arrow.vector.types.pojo.Field> childFields = new
ArrayList<>(childCount);
- int[] typeIds = new int[childCount];
- for (int i = 0; i < typeFields.length; i++) {
- if (typeFields[i] != null) {
- int curIdx = childFields.size();
- typeIds[curIdx] = i;
- childFields.add(typeFields[i]);
- }
+ // The children keep their order, which is not necessarily the order of
their type ids
+ List<org.apache.arrow.vector.types.pojo.Field> childFields = new
ArrayList<>(nextTypeId);
Review Comment:
Fixed in `27a6906c4` with a `typeCount()` helper, used for all the sizes and
loops you listed and also for the limit check in `registerNewTypeId`, which
compared against the wrapped value. The byte wraps on the 128th child.
`testMaximumNumberOfChildren` covers `getField()` and a split with 128
children, and that a 129th is rejected.
##########
vector/src/main/java/org/apache/arrow/vector/ExtensionTypeVector.java:
##########
@@ -154,8 +154,15 @@ public TransferPair getTransferPair(Field field,
BufferAllocator allocator, Call
return underlyingVector.getTransferPair(field, allocator, callBack);
}
+ /**
+ * Make a transfer pair to {@code target}. An extension vector target
receives the data in its
+ * underlying vector, any other target is passed to the transfer pair of the
underlying vector.
+ */
@Override
public TransferPair makeTransferPair(ValueVector target) {
+ if (target instanceof ExtensionTypeVector) {
Review Comment:
Applied in `27a6906c4`: a target of a different extension class now throws
`IllegalArgumentException`. `testTransferPairRejectsDifferentExtensionVector`
transfers an `OpaqueVector` into a `UuidVector` with the same storage.
##########
vector/src/main/codegen/templates/DenseUnionVector.java:
##########
@@ -669,29 +670,68 @@ public void splitAndTransfer(int startIndex, int length) {
to.reallocOffsetBuffer();
}
- int [] typeCounts = new int[nextTypeId];
- int [] typeStarts = new int[nextTypeId];
- for (int i = 0; i < typeCounts.length; i++) {
- typeCounts[i] = 0;
- typeStarts[i] = -1;
+ int[] childIndexes = new int[typeFields.length];
+ Arrays.fill(childIndexes, -1);
+ for (int i = 0; i < nextTypeId; i++) {
+ childIndexes[typeMapFields[i]] = i;
}
+ // The window selects childCounts[i] values of child i, which are
renumbered from 0. The
+ // offsets of a child only have to increase, so the selected values may
have gaps.
+ int[] childCounts = new int[nextTypeId];
+ int[] childStarts = new int[nextTypeId];
+ int[] childEnds = new int[nextTypeId];
+ boolean[] contiguous = new boolean[nextTypeId];
+ Arrays.fill(contiguous, true);
for (int i = startIndex; i < startIndex + length; i++) {
byte typeId = typeBuffer.getByte(i);
if (typeId >= 0) {
- to.offsetBuffer.setInt((long) (i - startIndex) * OFFSET_WIDTH,
typeCounts[typeId]);
- typeCounts[typeId] += 1;
- if (typeStarts[typeId] == -1) {
- typeStarts[typeId] = offsetBuffer.getInt((long) i * OFFSET_WIDTH);
+ int child = childIndexes[typeId];
Review Comment:
Fixed in `27a6906c4`: `splitAndTransfer` now clears the target and throws an
`IllegalArgumentException` naming the row and the type id. Covered by
`testSplitAndTransferWithUnknownTypeId`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
