iemejia commented on code in PR #3927:
URL: https://github.com/apache/avro/pull/3927#discussion_r3735596035
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -481,33 +481,29 @@ private FieldReader createArrayReader(Schema
readerSchema, Container action) thr
if (reuse instanceof GenericArray) {
GenericArray<Object> reuseArray = (GenericArray<Object>) reuse;
long l = decoder.readArrayStart();
- long total = 0;
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
Review Comment:
Good catch. Fixed in `createArrayReader`: the returned array reader now
opens a `beginCollectionAllocationScope()` around the block-reading loop. When
the fast reader is entered through `GenericDatumReader.read` the scope simply
nests and accumulates into the outer datum scope (only the outermost resets the
running total); when the fast reader is used standalone via
`createDatumReader(...)` the scope makes the zero-byte cap cumulative across
all array blocks of the datum instead of degrading to a per-block stateless
check. So a large `array<null>`-style array split across many blocks can no
longer bypass the cap. (66750cb)
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -481,33 +481,29 @@ private FieldReader createArrayReader(Schema
readerSchema, Container action) thr
if (reuse instanceof GenericArray) {
GenericArray<Object> reuseArray = (GenericArray<Object>) reuse;
long l = decoder.readArrayStart();
- long total = 0;
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
reuseArray.clear();
while (l > 0) {
for (long i = 0; i < l; i++) {
reuseArray.add(elementReader.read(reuseArray.peek(), decoder));
}
- total += l;
l = decoder.arrayNext();
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
}
return reuseArray;
} else {
long l = decoder.readArrayStart();
- long total = 0;
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
List<Object> array = (reuse instanceof List) ? (List<Object>) reuse
: new
GenericData.Array<>(GenericDatumReader.initialCollectionCapacity(l),
readerSchema);
array.clear();
while (l > 0) {
for (long i = 0; i < l; i++) {
array.add(elementReader.read(null, decoder));
}
- total += l;
l = decoder.arrayNext();
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
}
return array;
}
Review Comment:
Done — the scope is opened and closed in a `try/finally` that wraps both the
reuse and non-reuse branches, so it is always closed and ThreadLocal state
cannot leak into later decodes on the same thread. This keeps the zero-byte
element accounting cumulative across every array block in the read even when
the fast reader is used standalone. (66750cb)
--
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]
