Jackie-Jiang commented on code in PR #9538:
URL: https://github.com/apache/pinot/pull/9538#discussion_r989694085
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java:
##########
@@ -37,7 +41,57 @@ public Object[] getValues() {
}
public byte[] asBytes() {
- return SerializationUtils.serialize(_values);
+ int sizeInBytes = 0;
+ byte[][] cache = new byte[_values.length][];
+ for (int i = 0; i < _values.length; i++) {
+ Object value = _values[i];
+ if (value == null) {
+ continue;
+ }
+
+ if (value instanceof Integer) {
+ sizeInBytes += Integer.BYTES;
+ } else if (value instanceof Long) {
+ sizeInBytes += Long.BYTES;
+ } else if (value instanceof String) {
+ cache[i] = ((String) value).getBytes(StandardCharsets.UTF_8);
+ sizeInBytes += cache[i].length + Integer.BYTES;
+ } else if (value instanceof ByteArray) {
+ cache[i] = ((ByteArray) value).getBytes();
+ sizeInBytes += ((ByteArray) value).length() + Integer.BYTES;
+ } else if (value instanceof Float) {
+ sizeInBytes += Float.BYTES;
+ } else if (value instanceof Double) {
+ sizeInBytes += Double.BYTES;
+ } else if (value instanceof BigDecimal) {
+ cache[i] = BigDecimalUtils.serialize((BigDecimal) value);
+ sizeInBytes += cache[i].length + Integer.BYTES;
+ } else {
+ throw new IllegalStateException("Data type not supported for
serializing Primary Key with value: " + value);
+ }
+ }
+
+ ByteBuffer byteBuffer = ByteBuffer.allocate(sizeInBytes);
+ for (int i = 0; i < _values.length; i++) {
+ Object value = _values[i];
+ if (value == null) {
Review Comment:
No need to check null here if checked above
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java:
##########
@@ -37,7 +41,57 @@ public Object[] getValues() {
}
public byte[] asBytes() {
- return SerializationUtils.serialize(_values);
+ int sizeInBytes = 0;
+ byte[][] cache = new byte[_values.length][];
+ for (int i = 0; i < _values.length; i++) {
+ Object value = _values[i];
+ if (value == null) {
+ continue;
+ }
+
+ if (value instanceof Integer) {
+ sizeInBytes += Integer.BYTES;
+ } else if (value instanceof Long) {
+ sizeInBytes += Long.BYTES;
+ } else if (value instanceof String) {
+ cache[i] = ((String) value).getBytes(StandardCharsets.UTF_8);
+ sizeInBytes += cache[i].length + Integer.BYTES;
+ } else if (value instanceof ByteArray) {
+ cache[i] = ((ByteArray) value).getBytes();
+ sizeInBytes += ((ByteArray) value).length() + Integer.BYTES;
Review Comment:
(nit)
```suggestion
sizeInBytes += cache[i].length + Integer.BYTES;
```
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java:
##########
@@ -37,7 +41,57 @@ public Object[] getValues() {
}
public byte[] asBytes() {
- return SerializationUtils.serialize(_values);
+ int sizeInBytes = 0;
+ byte[][] cache = new byte[_values.length][];
+ for (int i = 0; i < _values.length; i++) {
+ Object value = _values[i];
+ if (value == null) {
+ continue;
+ }
+
+ if (value instanceof Integer) {
+ sizeInBytes += Integer.BYTES;
+ } else if (value instanceof Long) {
+ sizeInBytes += Long.BYTES;
+ } else if (value instanceof String) {
+ cache[i] = ((String) value).getBytes(StandardCharsets.UTF_8);
+ sizeInBytes += cache[i].length + Integer.BYTES;
+ } else if (value instanceof ByteArray) {
+ cache[i] = ((ByteArray) value).getBytes();
+ sizeInBytes += ((ByteArray) value).length() + Integer.BYTES;
+ } else if (value instanceof Float) {
+ sizeInBytes += Float.BYTES;
+ } else if (value instanceof Double) {
+ sizeInBytes += Double.BYTES;
+ } else if (value instanceof BigDecimal) {
+ cache[i] = BigDecimalUtils.serialize((BigDecimal) value);
+ sizeInBytes += cache[i].length + Integer.BYTES;
+ } else {
+ throw new IllegalStateException("Data type not supported for
serializing Primary Key with value: " + value);
Review Comment:
We can handle `null` here:
```suggestion
throw new IllegalStateException(String.format("Unsupported value: %s
of type: %s", value, value != null ? value.getClass() : null));
```
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java:
##########
@@ -37,7 +41,57 @@ public Object[] getValues() {
}
public byte[] asBytes() {
- return SerializationUtils.serialize(_values);
+ int sizeInBytes = 0;
+ byte[][] cache = new byte[_values.length][];
Review Comment:
In majority of the use cases, primary key has only one value, and we can
optimize that case by doing one pass
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java:
##########
@@ -37,7 +41,57 @@ public Object[] getValues() {
}
public byte[] asBytes() {
- return SerializationUtils.serialize(_values);
+ int sizeInBytes = 0;
+ byte[][] cache = new byte[_values.length][];
+ for (int i = 0; i < _values.length; i++) {
+ Object value = _values[i];
+ if (value == null) {
Review Comment:
It should not be `null` or we cannot guarantee different primary key
generate different 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]
