xiangfu0 commented on code in PR #19836:
URL: https://github.com/apache/pinot/pull/19836#discussion_r4290729207
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -1240,9 +1323,28 @@ public Geometry deserialize(ByteBuffer byteBuffer) {
public static final ObjectSerDe<RoaringBitmap> ROARING_BITMAP_SER_DE = new
ObjectSerDe<RoaringBitmap>() {
+ @Override
+ public int getSerializedSize(RoaringBitmap bitmap) {
+ int size = bitmap.serializedSizeInBytes();
Review Comment:
Added a note in f9738e798b26c145e4faa7a75f93394779717e14: the buffer writer
rechecks size because it can be called without a preceding sizing call. Reusing
a trusted caller-supplied size can remain a follow-up.
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -1240,9 +1323,28 @@ public Geometry deserialize(ByteBuffer byteBuffer) {
public static final ObjectSerDe<RoaringBitmap> ROARING_BITMAP_SER_DE = new
ObjectSerDe<RoaringBitmap>() {
+ @Override
+ public int getSerializedSize(RoaringBitmap bitmap) {
+ int size = bitmap.serializedSizeInBytes();
+ Preconditions.checkArgument(size >= 0, "Serialized bitmap exceeds the
maximum buffer size");
+ return size;
+ }
+
@Override
public byte[] serialize(RoaringBitmap bitmap) {
- return RoaringBitmapUtils.serialize(bitmap);
+ return ObjectSerDe.super.serialize(bitmap);
+ }
+
+ @Override
+ public void serialize(RoaringBitmap bitmap, ByteBuffer destination) {
+ checkSerializationBuffer(destination, getSerializedSize(bitmap));
+ ByteOrder originalOrder = destination.order();
+ destination.order(ByteOrder.LITTLE_ENDIAN);
Review Comment:
Added the comment in f9738e798b26c145e4faa7a75f93394779717e14. Roaring
payloads are little-endian; setting that order explicitly avoids the library's
temporary endian view, and the finally block restores the caller's order.
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -323,11 +326,35 @@ public static ObjectType getObjectType(Object value) {
/// Serializer/De-serializer for a specific type of object.
///
+ /// Implementations must override either the byte-array serializer or both
[#getSerializedSize(Object)] and
+ /// the buffer serializer [#serialize(Object, ByteBuffer)].
+ ///
/// @param <T> Type of the object
public interface ObjectSerDe<T> {
/// Serializes a value into a byte array.
- byte[] serialize(T value);
+ /// Typed forwarding overrides preserve receiver specialization when
invoking this default.
+ default byte[] serialize(T value) {
+ byte[] bytes = new byte[getSerializedSize(value)];
+ serialize(value, ByteBuffer.wrap(bytes));
+ return bytes;
+ }
+
+ /// Returns the exact size of a representable payload, or `-1` when
allocation-free sizing is unsupported.
+ /// This method must not serialize the value or allocate a payload array.
The value must remain unchanged between
+ /// sizing and serialization. The destination must be bounded to this size
when serializing.
+ default int getSerializedSize(T value) {
+ return -1;
+ }
+
+ /// Writes the canonical payload at the destination's current position and
advances it by the payload size.
+ /// Supports heap and direct buffers, preserves the destination's limit
and byte order, and does not retain the
+ /// buffer. Direct-writing overrides must not mutate the value. A
read-only destination or insufficient remaining
+ /// space fails before any write. The default implementation preserves the
legacy serializer behavior and
+ /// serializes once into a temporary array; codecs can override it for
direct writes.
+ default void serialize(T value, ByteBuffer destination) {
Review Comment:
Documented in f9738e798b26c145e4faa7a75f93394779717e14 that the
override-pair requirement is enforced by convention. The default
implementations and their delegation remain unchanged.
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -323,11 +326,35 @@ public static ObjectType getObjectType(Object value) {
/// Serializer/De-serializer for a specific type of object.
///
+ /// Implementations must override either the byte-array serializer or both
[#getSerializedSize(Object)] and
+ /// the buffer serializer [#serialize(Object, ByteBuffer)].
+ ///
/// @param <T> Type of the object
public interface ObjectSerDe<T> {
/// Serializes a value into a byte array.
- byte[] serialize(T value);
+ /// Typed forwarding overrides preserve receiver specialization when
invoking this default.
+ default byte[] serialize(T value) {
+ byte[] bytes = new byte[getSerializedSize(value)];
+ serialize(value, ByteBuffer.wrap(bytes));
+ return bytes;
+ }
+
+ /// Returns the exact size of a representable payload, or `-1` when
allocation-free sizing is unsupported.
+ /// This method must not serialize the value or allocate a payload array.
The value must remain unchanged between
+ /// sizing and serialization. The destination must be bounded to this size
when serializing.
Review Comment:
Updated in f9738e798b26c145e4faa7a75f93394779717e14 to require sufficient
remaining space. The existing buffer test now covers both exact and excess
remaining capacity while checking the written position, unchanged limit/order
and untouched surrounding bytes.
--
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]
