Jackie-Jiang commented on code in PR #9538: URL: https://github.com/apache/pinot/pull/9538#discussion_r989416345
########## pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java: ########## @@ -37,7 +40,56 @@ public Object[] getValues() { } public byte[] asBytes() { - return SerializationUtils.serialize(_values); + int arraySize = 0; + byte[][] cache = new byte[_values.length][]; + for (int i = 0; i < _values.length; i++) { + Object value = _values[i]; + if (value instanceof Integer) { + arraySize += Integer.BYTES; + } else if (value instanceof Double) { + arraySize += Double.BYTES; + } else if (value instanceof Float) { + arraySize += Float.BYTES; + } else if (value instanceof Long) { + arraySize += Long.BYTES; + } else if (value instanceof String) { + cache[i] = ((String) value).getBytes(StandardCharsets.UTF_8); + arraySize += cache[i].length + Integer.BYTES; + } else if (value instanceof BigDecimal) { + cache[i] = BigDecimalUtils.serialize((BigDecimal) value); + arraySize += cache[i].length + Integer.BYTES; + } else if (value instanceof byte[]) { + arraySize += ((byte[]) value).length + Integer.BYTES; + } else { + throw new IllegalStateException("Data type not supported for serializing Primary Key"); + } + } + + ByteBuffer byteBuffer = ByteBuffer.allocate(arraySize); + for (int i = 0; i < _values.length; i++) { + Object value = _values[i]; + if (value instanceof Integer) { + byteBuffer.putInt((Integer) value); + } else if (value instanceof Double) { + byteBuffer.putDouble((Double) value); + } else if (value instanceof Float) { + byteBuffer.putFloat((Float) value); + } else if (value instanceof Long) { + byteBuffer.putLong((Long) value); + } else if (value instanceof String) { + byteBuffer.putInt(cache[i].length); + byteBuffer.put(cache[i]); + } else if (value instanceof BigDecimal) { + byteBuffer.putInt(cache[i].length); + byteBuffer.put(cache[i]); + } else if (value instanceof byte[]) { + byteBuffer.putInt(cache[i].length); + byteBuffer.put(cache[i]); + } else { + throw new IllegalStateException("Data type not supported for serializing Primary Key"); + } Review Comment: This part can be simplified ```suggestion } else { byteBuffer.putInt(cache[i].length); byteBuffer.put(cache[i]); } ``` ########## pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java: ########## @@ -37,7 +40,56 @@ public Object[] getValues() { } public byte[] asBytes() { - return SerializationUtils.serialize(_values); + int arraySize = 0; Review Comment: (optional) `sizeInBytes` ########## pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java: ########## @@ -37,7 +40,56 @@ public Object[] getValues() { } public byte[] asBytes() { - return SerializationUtils.serialize(_values); + int arraySize = 0; + byte[][] cache = new byte[_values.length][]; + for (int i = 0; i < _values.length; i++) { + Object value = _values[i]; + if (value instanceof Integer) { + arraySize += Integer.BYTES; + } else if (value instanceof Double) { + arraySize += Double.BYTES; + } else if (value instanceof Float) { + arraySize += Float.BYTES; + } else if (value instanceof Long) { + arraySize += Long.BYTES; + } else if (value instanceof String) { + cache[i] = ((String) value).getBytes(StandardCharsets.UTF_8); + arraySize += cache[i].length + Integer.BYTES; + } else if (value instanceof BigDecimal) { + cache[i] = BigDecimalUtils.serialize((BigDecimal) value); + arraySize += cache[i].length + Integer.BYTES; + } else if (value instanceof byte[]) { Review Comment: (MAJOR) For `BYTES` type, value will be `ByteArray` ########## pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java: ########## @@ -37,7 +40,56 @@ public Object[] getValues() { } public byte[] asBytes() { - return SerializationUtils.serialize(_values); + int arraySize = 0; + byte[][] cache = new byte[_values.length][]; + for (int i = 0; i < _values.length; i++) { + Object value = _values[i]; + if (value instanceof Integer) { Review Comment: (minor) Consider reordering based on the how common it is used as primary key: INT, LONG, STRING, BYTES, FLOAT, DOUBLE, BIG_DECIMAL ########## pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/PrimaryKey.java: ########## @@ -37,7 +40,56 @@ public Object[] getValues() { } public byte[] asBytes() { - return SerializationUtils.serialize(_values); + int arraySize = 0; + byte[][] cache = new byte[_values.length][]; + for (int i = 0; i < _values.length; i++) { + Object value = _values[i]; + if (value instanceof Integer) { + arraySize += Integer.BYTES; + } else if (value instanceof Double) { + arraySize += Double.BYTES; + } else if (value instanceof Float) { + arraySize += Float.BYTES; + } else if (value instanceof Long) { + arraySize += Long.BYTES; + } else if (value instanceof String) { + cache[i] = ((String) value).getBytes(StandardCharsets.UTF_8); + arraySize += cache[i].length + Integer.BYTES; + } else if (value instanceof BigDecimal) { + cache[i] = BigDecimalUtils.serialize((BigDecimal) value); + arraySize += cache[i].length + Integer.BYTES; + } else if (value instanceof byte[]) { + arraySize += ((byte[]) value).length + Integer.BYTES; + } else { + throw new IllegalStateException("Data type not supported for serializing Primary Key"); Review Comment: Put the value class in the exception message (also handle `null` properly) -- 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