paul-rogers commented on a change in pull request #11853:
URL: https://github.com/apache/druid/pull/11853#discussion_r738893409
##########
File path: core/src/main/java/org/apache/druid/math/expr/ExprEval.java
##########
@@ -26,138 +26,56 @@
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.UOE;
+import org.apache.druid.segment.column.ObjectByteStrategy;
+import org.apache.druid.segment.column.Types;
import javax.annotation.Nullable;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.List;
-import java.util.Objects;
/**
* Generic result holder for evaluated {@link Expr} containing the value and
{@link ExprType} of the value to allow
*/
public abstract class ExprEval<T>
{
- private static final int NULL_LENGTH = -1;
-
- /**
- * Deserialize an expression stored in a bytebuffer, e.g. for an agg.
- *
- * This should be refactored to be consolidated with some of the standard
type handling of aggregators probably
- */
- public static ExprEval deserialize(ByteBuffer buffer, int position)
- {
- final ExprType type = ExprType.fromByte(buffer.get(position));
- return deserialize(buffer, position + 1, type);
- }
-
/**
* Deserialize an expression stored in a bytebuffer, e.g. for an agg.
*
* This should be refactored to be consolidated with some of the standard
type handling of aggregators probably
*/
- public static ExprEval deserialize(ByteBuffer buffer, int offset, ExprType
type)
+ public static ExprEval deserialize(ByteBuffer buffer, int offset,
ExpressionType type)
{
- // | expression bytes |
- switch (type) {
+ switch (type.getType()) {
case LONG:
- // | expression type (byte) | is null (byte) | long bytes |
- if (buffer.get(offset++) == NullHandling.IS_NOT_NULL_BYTE) {
- return of(buffer.getLong(offset));
+ if (Types.isNullableNull(buffer, offset)) {
+ return ofLong(null);
}
- return ofLong(null);
+ return of(Types.readNullableLong(buffer, offset));
case DOUBLE:
- // | expression type (byte) | is null (byte) | double bytes |
- if (buffer.get(offset++) == NullHandling.IS_NOT_NULL_BYTE) {
- return of(buffer.getDouble(offset));
+ if (Types.isNullableNull(buffer, offset)) {
+ return ofDouble(null);
}
- return ofDouble(null);
+ return of(Types.readNullableDouble(buffer, offset));
case STRING:
- // | expression type (byte) | string length (int) | string bytes |
- final int length = buffer.getInt(offset);
- if (length < 0) {
+ if (Types.isNullableNull(buffer, offset)) {
return of(null);
}
- final byte[] stringBytes = new byte[length];
- final int oldPosition = buffer.position();
- buffer.position(offset + Integer.BYTES);
- buffer.get(stringBytes, 0, length);
- buffer.position(oldPosition);
+ final byte[] stringBytes = Types.readNullableVariableBlob(buffer,
offset);
return of(StringUtils.fromUtf8(stringBytes));
case ARRAY:
- final ExprType elementType = ExprType.fromByte(buffer.get(offset++));
- switch (elementType) {
+ switch (type.getElementType().getType()) {
case LONG:
- // | expression type (byte) | array element type (byte) | array
length (int) | array bytes |
- final int longArrayLength = buffer.getInt(offset);
- offset += Integer.BYTES;
- if (longArrayLength < 0) {
- return ofLongArray(null);
- }
- final Long[] longs = new Long[longArrayLength];
- for (int i = 0; i < longArrayLength; i++) {
- final byte isNull = buffer.get(offset);
- offset += Byte.BYTES;
- if (isNull == NullHandling.IS_NOT_NULL_BYTE) {
- // | is null (byte) | long bytes |
- longs[i] = buffer.getLong(offset);
- offset += Long.BYTES;
- } else {
- // | is null (byte) |
- longs[i] = null;
- }
- }
- return ofLongArray(longs);
+ return ofLongArray(Types.readNullableLongArray(buffer, offset));
case DOUBLE:
- // | expression type (byte) | array element type (byte) | array
length (int) | array bytes |
- final int doubleArrayLength = buffer.getInt(offset);
- offset += Integer.BYTES;
- if (doubleArrayLength < 0) {
- return ofDoubleArray(null);
- }
- final Double[] doubles = new Double[doubleArrayLength];
- for (int i = 0; i < doubleArrayLength; i++) {
- final byte isNull = buffer.get(offset);
- offset += Byte.BYTES;
- if (isNull == NullHandling.IS_NOT_NULL_BYTE) {
- // | is null (byte) | double bytes |
- doubles[i] = buffer.getDouble(offset);
- offset += Double.BYTES;
- } else {
- // | is null (byte) |
- doubles[i] = null;
- }
- }
- return ofDoubleArray(doubles);
+ return ofDoubleArray(Types.readNullableDoubleArray(buffer,
offset));
case STRING:
- // | expression type (byte) | array element type (byte) | array
length (int) | array bytes |
- final int stringArrayLength = buffer.getInt(offset);
- offset += Integer.BYTES;
- if (stringArrayLength < 0) {
- return ofStringArray(null);
- }
- final String[] stringArray = new String[stringArrayLength];
- for (int i = 0; i < stringArrayLength; i++) {
- final int stringElementLength = buffer.getInt(offset);
- offset += Integer.BYTES;
- if (stringElementLength < 0) {
- // | string length (int) |
- stringArray[i] = null;
- } else {
- // | string length (int) | string bytes |
- final byte[] stringElementBytes = new
byte[stringElementLength];
- final int oldPosition2 = buffer.position();
- buffer.position(offset);
- buffer.get(stringElementBytes, 0, stringElementLength);
- buffer.position(oldPosition2);
- stringArray[i] = StringUtils.fromUtf8(stringElementBytes);
- offset += stringElementLength;
- }
- }
- return ofStringArray(stringArray);
+ return ofStringArray(Types.readNullableStringArray(buffer,
offset));
Review comment:
Nice simplification. (The code below this line that was removed, leaving
only this line.)
--
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]
