Jackie-Jiang commented on code in PR #11674:
URL: https://github.com/apache/pinot/pull/11674#discussion_r1337663695
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/VarByteChunkWriter.java:
##########
@@ -28,4 +28,8 @@ public interface VarByteChunkWriter extends Closeable {
void putString(String value);
void putBytes(byte[] value);
+
+ void putStrings(String[] values);
+
+ void putByteArrays(byte[][] values);
Review Comment:
Suggest renaming them to be more clear
```suggestion
void putStringMV(String[] values);
void putBytesMV(byte[][] values);
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/VarByteChunkForwardIndexWriterV4.java:
##########
@@ -121,6 +123,42 @@ public void putBigDecimal(BigDecimal bigDecimal) {
putBytes(BigDecimalUtils.serialize(bigDecimal));
}
+ @Override
Review Comment:
(minor) Suggest keeping the method the same order as in the interface to be
easier to navigate
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/VarByteChunkForwardIndexWriterV4.java:
##########
@@ -121,6 +123,42 @@ public void putBigDecimal(BigDecimal bigDecimal) {
putBytes(BigDecimalUtils.serialize(bigDecimal));
}
+ @Override
+ public void putByteArrays(byte[][] values) {
+ // num values + length of each value
+ int size = Integer.BYTES + Integer.BYTES * values.length;
+ for (byte[] value : values) {
+ size += value.length;
+ }
+ byte[] serializedBytes = new byte[size];
+ ByteBuffer byteBuffer = ByteBuffer.wrap(serializedBytes);
+ byteBuffer.putInt(values.length);
+
+ for (byte[] value : values) {
Review Comment:
Consider using the same format as the old index `[numValues, length1,
length2, ..., bytes1, bytes2, ...]`
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/forward/BaseVarByteChunkForwardIndexReaderV4.java:
##########
@@ -43,9 +43,9 @@
* (BIG_DECIMAL, STRING, BYTES).
* <p>For data layout, please refer to the documentation for {@link
VarByteChunkForwardIndexWriterV4}
*/
-public class VarByteChunkSVForwardIndexReaderV4
- implements
ForwardIndexReader<VarByteChunkSVForwardIndexReaderV4.ReaderContext> {
- private static final Logger LOGGER =
LoggerFactory.getLogger(VarByteChunkSVForwardIndexReaderV4.class);
+public class BaseVarByteChunkForwardIndexReaderV4
Review Comment:
Rename it to `VarByteChunkForwardIndexReaderV4`. Also update the javadoc for
this class
--
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]
