somandal commented on code in PR #9510:
URL: https://github.com/apache/pinot/pull/9510#discussion_r995964873
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueVarByteRawIndexCreator.java:
##########
@@ -119,4 +117,55 @@ public void close()
throws IOException {
_indexWriter.close();
}
+
+ /**
+ * The actual content in an MV array is prepended with 2 prefixes:
+ * 1. elementLengthStoragePrefix - bytes required to store the length of
each array element
+ * 2. numberOfElementsStoragePrefix - Number of elements in the array
+ *
+ * This function returns the total bytes needed to store (1)
elementLengthStoragePrefix
+ */
+ public static int getElementLengthStoragePrefix(int maxNumberOfElements) {
+ return Integer.BYTES * maxNumberOfElements;
+ }
+
+ /**
+ * The actual content in an MV array is prepended with 2 prefixes:
+ * 1. elementLengthStoragePrefix - bytes required to store the length of
each array element
+ * 2. numberOfElementsStoragePrefix - Number of elements in the array
+ *
+ * This function returns the bytes needed to store (2)
numberOfElementsStoragePrefix
+ */
+ public static int getNumberOfElementsStoragePrefix() {
Review Comment:
nit: Same as `getElementLengthStoragePrefix`, perhaps modify the function
name to indicate it returns a size in bytes.
Just calling it "StoragePrefix" makes me think that this is going to return
the prefix itself.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueVarByteRawIndexCreator.java:
##########
@@ -119,4 +117,55 @@ public void close()
throws IOException {
_indexWriter.close();
}
+
+ /**
+ * The actual content in an MV array is prepended with 2 prefixes:
+ * 1. elementLengthStoragePrefix - bytes required to store the length of
each array element
+ * 2. numberOfElementsStoragePrefix - Number of elements in the array
+ *
+ * This function returns the total bytes needed to store (1)
elementLengthStoragePrefix
+ */
+ public static int getElementLengthStoragePrefix(int maxNumberOfElements) {
Review Comment:
nit: considering that this function returns a length in bytes, can you
rename the function to indicate this? e.g.
`getElementLengthStoragePrefixInBytes`
Also, this accounts for the worst case size of each (i.e. max). Perhaps the
function description and name should also indicate this.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
