lidavidm commented on code in PR #14910:
URL: https://github.com/apache/arrow/pull/14910#discussion_r1046079911
##########
java/vector/src/main/java/org/apache/arrow/vector/dictionary/ListSubfieldEncoder.java:
##########
@@ -132,24 +137,29 @@ public static BaseListVector
decodeListSubField(BaseListVector vector,
// clone list vector and initialize data vector
BaseListVector decoded = cloneVector(vector, allocator);
- Field dataVectorField = getDataVector(dictionaryVector).getField();
-
decoded.initializeChildrenFromFields(Collections.singletonList(dataVectorField));
+ try {
+ Field dataVectorField = getDataVector(dictionaryVector).getField();
+
decoded.initializeChildrenFromFields(Collections.singletonList(dataVectorField));
- // get data vector
- ValueVector dataVector = getDataVector(decoded);
+ // get data vector
+ ValueVector dataVector = getDataVector(decoded);
- TransferPair transfer =
getDataVector(dictionaryVector).makeTransferPair(dataVector);
- BaseIntVector indices = (BaseIntVector) getDataVector(vector);
+ TransferPair transfer =
getDataVector(dictionaryVector).makeTransferPair(dataVector);
+ BaseIntVector indices = (BaseIntVector) getDataVector(vector);
- for (int i = 0; i < valueCount; i++) {
+ for (int i = 0; i < valueCount; i++) {
- if (!vector.isNull(i)) {
- int start = vector.getElementStartIndex(i);
- int end = vector.getElementEndIndex(i);
+ if (!vector.isNull(i)) {
+ int start = vector.getElementStartIndex(i);
+ int end = vector.getElementEndIndex(i);
- DictionaryEncoder.retrieveIndexVector(indices, transfer,
dictionaryValueCount, start, end);
+ DictionaryEncoder.retrieveIndexVector(indices, transfer,
dictionaryValueCount, start, end);
+ }
}
+ return decoded;
+ } catch (Exception e) {
+ decoded.close();
+ throw e;
Review Comment:
Perhaps use [`AutoCloseables#close(Throwable,
AutoCloseable...)`](https://github.com/apache/arrow/blob/master/java/memory/memory-core/src/main/java/org/apache/arrow/util/AutoCloseables.java#L54)
for this kind of thing?
##########
java/vector/src/test/java/org/apache/arrow/vector/TestDictionaryVector.java:
##########
@@ -916,6 +916,132 @@ public void testNoMemoryLeak() {
assertEquals("decode memory leak", 0, allocator.getAllocatedMemory());
}
+ @Test
+ public void testListNoMemoryLeak() {
+ // Create a new value vector
+ try (final ListVector vector = ListVector.empty("vector", allocator);
+ final ListVector dictionaryVector = ListVector.empty("dict",
allocator)) {
+
+ UnionListWriter writer = vector.getWriter();
+ writer.allocate();
+ writeListVector(writer, new int[]{10, 20});
+ writer.setValueCount(1);
+
+ UnionListWriter dictWriter = dictionaryVector.getWriter();
+ dictWriter.allocate();
+ writeListVector(dictWriter, new int[]{10});
+ dictionaryVector.setValueCount(1);
+
+ Dictionary dictionary = new Dictionary(dictionaryVector, new
DictionaryEncoding(1L, false, null));
+ ListSubfieldEncoder encoder = new ListSubfieldEncoder(dictionary,
allocator);
+
+ try (final ListVector encoded = (ListVector)
encoder.encodeListSubField(vector)) {
+ fail("There should be an exception when encoding");
+ } catch (Exception e) {
+ assertEquals("Dictionary encoding not defined for value:" + 20,
e.getMessage());
Review Comment:
nit: why concat here, when the string is fully hardcoded below?
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
