xiangfu0 commented on code in PR #19906:
URL: https://github.com/apache/pinot/pull/19906#discussion_r4296686674
##########
pinot-common/src/main/java/org/apache/pinot/common/datatable/DataTableImplV4.java:
##########
@@ -256,87 +255,96 @@ public String getString(int rowId, int colId) {
@Override
public ByteArray getBytes(int rowId, int colId) {
- int size = positionOffsetInVariableBufferAndGetLength(rowId, colId);
- byte[] buffer = new byte[size];
- _variableSizeData.get(buffer);
- return new ByteArray(buffer);
+ int offsetInFixed = getOffsetInFixedBuffer(rowId, colId);
+ return new ByteArray(copyFromVariableBuffer(offsetInFixed));
}
// --------------------------------------------------------------------------
// Variable sized element access.
// --------------------------------------------------------------------------
+ // These read with absolute offsets instead of a stream: a stream over a
paged buffer creates a view of every page
+ // up to the end of the section on each call.
@Override
public int[] getIntArray(int rowId, int colId) {
- int length = positionOffsetInVariableBufferAndGetLength(rowId, colId);
- int[] ints = new int[length];
- for (int i = 0; i < length; i++) {
- ints[i] = _variableSizeData.getInt();
+ int offsetInFixed = getOffsetInFixedBuffer(rowId, colId);
+ long offsetInVar = _fixedSizeData.getInt(offsetInFixed);
+ int[] ints = new int[_fixedSizeData.getInt(offsetInFixed + Integer.BYTES)];
+ for (int i = 0; i < ints.length; i++) {
+ ints[i] = _variableSizeData.getInt(offsetInVar + (long) i *
Integer.BYTES);
Review Comment:
The read-only invariant here depends on every OBJECT deserializer never
calling `array()` on this buffer. I verified the current callers are all safe,
but since the returned buffer can be read-only only for in-process tables (and
writable after deserialization), this is an easy trap for future code. Consider
stating the "read-only, do not call array()" contract on the `getCustomObject`
Javadoc, not just this inline comment, so implementers of new
`deserializeIntermediateResult` see it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
