Skip to content

Fix literal type handling to be SQL compatible #11749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Jackie-Jiang wants to merge 2 commits into from
Closed

Fix literal type handling to be SQL compatible #11749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d0263b
Fix literal type handling to be SQL compatible
Jackie-Jiang Oct 6, 2023
adaab9f
Do not rely on column name lookup in selection reducer
Jackie-Jiang Oct 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
277 changes: 168 additions & 109 deletions pinot-common/src/main/java/org/apache/pinot/common/request/context/LiteralContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,179 +18,232 @@
*/
package org.apache.pinot.common.request.context;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.sql.Timestamp;
import java.util.Objects;
import javax.annotation.Nullable;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.pinot.common.request.Literal;
import org.apache.pinot.common.utils.PinotDataType;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.utils.BigDecimalUtils;
import org.apache.pinot.spi.utils.BytesUtils;
import org.apache.pinot.spi.utils.CommonConstants.NullValuePlaceHolder;


/**
* The {@code LiteralContext} class represents a literal in the query.
* <p>This includes both value and type information. We translate thrift literal to this representation in server.
* Currently, only Boolean literal is correctly encoded in thrift and passed in.
* All integers are encoded as LONG in thrift, and the other numerical types are encoded as DOUBLE.
* The remaining types are encoded as STRING.
*/
public class LiteralContext {
// TODO: Support all of the types for sql.
private final FieldSpec.DataType _type;
private final DataType _type;
private final Object _value;
private final BigDecimal _bigDecimalValue;

private static BigDecimal getBigDecimalValue(FieldSpec.DataType type, Object value) {
switch (type) {
case BIG_DECIMAL:
return (BigDecimal) value;
case BOOLEAN:
return PinotDataType.BOOLEAN.toBigDecimal(value);
case TIMESTAMP:
return PinotDataType.TIMESTAMP.toBigDecimal(Timestamp.valueOf(value.toString()));
default:
if (type.isNumeric()) {
return new BigDecimal(value.toString());
}
return BigDecimal.ZERO;
}
}
/**
* A transient field used for the type conversion, and is not included in {@link #equals} and {@link #hashCode}.
*/
private final transient PinotDataType _pinotDataType;
Copy link
Copy Markdown
Contributor

@gortiz gortiz Oct 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why transient? This class is not serializable, right?

Copy link
Copy Markdown
Contributor Author

@Jackie-Jiang Jackie-Jiang Oct 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Added transient for documentation purpose and future proof because it is not included in equals() and hashCode(). Certain tools such as EqualsVerifier won't pass if a non-transient field is not used.


@VisibleForTesting
static Pair<FieldSpec.DataType, Object> inferLiteralDataTypeAndValue(String literal) {
// Try to interpret the literal as number
try {
Number number = NumberUtils.createNumber(literal);
if (number instanceof BigDecimal || number instanceof BigInteger) {
return ImmutablePair.of(FieldSpec.DataType.BIG_DECIMAL, new BigDecimal(literal));
} else {
return ImmutablePair.of(FieldSpec.DataType.STRING, literal);
}
} catch (Exception e) {
// Ignored
}
private Boolean _booleanValue;
private Integer _intValue;
private Long _longValue;
private Float _floatValue;
private Double _doubleValue;
private BigDecimal _bigDecimalValue;
private String _stringValue;
private byte[] _bytesValue;

// Try to interpret the literal as TIMESTAMP
try {
Timestamp timestamp = Timestamp.valueOf(literal);
return ImmutablePair.of(FieldSpec.DataType.TIMESTAMP, timestamp);
} catch (Exception e) {
// Ignored
}
return ImmutablePair.of(FieldSpec.DataType.STRING, literal);
public LiteralContext(DataType type, Object value) {
_type = type;
_value = value;
_pinotDataType = getPinotDataType(type);
}

public LiteralContext(Literal literal) {
Preconditions.checkState(literal.getFieldValue() != null,
"Field value cannot be null for field:" + literal.getSetField());
switch (literal.getSetField()) {
case BOOL_VALUE:
_type = FieldSpec.DataType.BOOLEAN;
_value = literal.getFieldValue();
_bigDecimalValue = PinotDataType.BOOLEAN.toBigDecimal(_value);
_type = DataType.BOOLEAN;
_value = literal.getBoolValue();
break;
case INT_VALUE:
_type = FieldSpec.DataType.INT;
_type = DataType.INT;
_value = literal.getIntValue();
_bigDecimalValue = new BigDecimal((int) _value);
break;
case LONG_VALUE:
// TODO: Remove this special handling after broker uses INT type for integer literals.
// https://github.com/apache/pinot/pull/11751
// _type = DataType.LONG;
// _value = literal.getLongValue();
long longValue = literal.getLongValue();
if (longValue == (int) longValue) {
_type = FieldSpec.DataType.INT;
if (longValue >= Integer.MIN_VALUE && longValue <= Integer.MAX_VALUE) {
_type = DataType.INT;
_value = (int) longValue;
} else {
_type = FieldSpec.DataType.LONG;
_type = DataType.LONG;
_value = longValue;
}
_bigDecimalValue = new BigDecimal(longValue);
break;
case DOUBLE_VALUE:
String stringValue = literal.getFieldValue().toString();
Number floatingNumber = NumberUtils.createNumber(stringValue);
if (floatingNumber instanceof Float) {
_type = FieldSpec.DataType.FLOAT;
_value = floatingNumber;
} else {
_type = FieldSpec.DataType.DOUBLE;
_value = literal.getDoubleValue();
}
_bigDecimalValue = new BigDecimal(stringValue);
case FLOAT_VALUE:
_type = DataType.FLOAT;
_value = Float.intBitsToFloat(literal.getFloatValue());
break;
case STRING_VALUE:
Pair<FieldSpec.DataType, Object> typeAndValue =
inferLiteralDataTypeAndValue(literal.getFieldValue().toString());
_type = typeAndValue.getLeft();
if (_type == FieldSpec.DataType.BIG_DECIMAL) {
_bigDecimalValue = (BigDecimal) typeAndValue.getRight();
} else if (_type == FieldSpec.DataType.TIMESTAMP) {
_bigDecimalValue = PinotDataType.TIMESTAMP.toBigDecimal(typeAndValue.getRight());
} else {
_bigDecimalValue = BigDecimal.ZERO;
}
_value = literal.getFieldValue().toString();
case DOUBLE_VALUE:
_type = DataType.DOUBLE;
_value = literal.getDoubleValue();
break;
case BIG_DECIMAL_VALUE:
_type = FieldSpec.DataType.BIG_DECIMAL;
_bigDecimalValue = BigDecimalUtils.deserialize(literal.getBigDecimalValue());
_value = _bigDecimalValue;
_type = DataType.BIG_DECIMAL;
_value = BigDecimalUtils.deserialize(literal.getBigDecimalValue());
break;
case STRING_VALUE:
_type = DataType.STRING;
_value = literal.getStringValue();
break;
case BINARY_VALUE:
_type = FieldSpec.DataType.BYTES;
_type = DataType.BYTES;
_value = literal.getBinaryValue();
_bigDecimalValue = BigDecimal.ZERO;
break;
case NULL_VALUE:
_type = FieldSpec.DataType.UNKNOWN;
_type = DataType.UNKNOWN;
_value = null;
_bigDecimalValue = BigDecimal.ZERO;
break;
default:
throw new UnsupportedOperationException("Unsupported data type:" + literal.getSetField());
throw new IllegalStateException("Unsupported field type: " + literal.getSetField());
}
_pinotDataType = getPinotDataType(_type);
}

private static PinotDataType getPinotDataType(DataType type) {
switch (type) {
case BOOLEAN:
return PinotDataType.BOOLEAN;
case INT:
return PinotDataType.INTEGER;
case LONG:
return PinotDataType.LONG;
case FLOAT:
return PinotDataType.FLOAT;
case DOUBLE:
return PinotDataType.DOUBLE;
case BIG_DECIMAL:
return PinotDataType.BIG_DECIMAL;
case STRING:
return PinotDataType.STRING;
case BYTES:
return PinotDataType.BYTES;
case UNKNOWN:
return null;
default:
throw new IllegalStateException("Unsupported data type: " + type);
}
}

public FieldSpec.DataType getType() {
public DataType getType() {
return _type;
}

@Nullable
public Object getValue() {
return _value;
}

public boolean getBooleanValue() {
Boolean booleanValue = _booleanValue;
if (booleanValue == null) {
booleanValue = _pinotDataType != null && _pinotDataType.toBoolean(_value);
_booleanValue = booleanValue;
}
return booleanValue;
}

public int getIntValue() {
return _bigDecimalValue.intValue();
Integer intValue = _intValue;
if (intValue == null) {
try {
intValue = _pinotDataType != null ? _pinotDataType.toInt(_value) : NullValuePlaceHolder.INT;
} catch (NumberFormatException e) {
// Pinot uses int to represent BOOLEAN value, so we need to handle the case when the value is the string
// representation of a BOOLEAN value.
String stringValue = (String) _value;
if (stringValue.equalsIgnoreCase("true")) {
intValue = 1;
} else if (stringValue.equalsIgnoreCase("false")) {
intValue = 0;
} else {
throw new IllegalArgumentException("Invalid int value: " + _value);
}
}
_intValue = intValue;
}
return intValue;
}

public long getLongValue() {
Long longValue = _longValue;
if (longValue == null) {
try {
longValue = _pinotDataType != null ? _pinotDataType.toLong(_value) : NullValuePlaceHolder.LONG;
} catch (NumberFormatException e) {
// Pinot uses long to represent TIMESTAMP value, so we need to handle the case when the value is the string
// representation of a TIMESTAMP value.
try {
longValue = Timestamp.valueOf((String) _value).getTime();
} catch (IllegalArgumentException e1) {
throw new IllegalArgumentException("Invalid long value: " + _value);
}
}
_longValue = longValue;
}
return longValue;
}

public float getFloatValue() {
Float floatValue = _floatValue;
if (floatValue == null) {
floatValue = _pinotDataType != null ? _pinotDataType.toFloat(_value) : NullValuePlaceHolder.FLOAT;
_floatValue = floatValue;
}
return floatValue;
}

public double getDoubleValue() {
return _bigDecimalValue.doubleValue();
Double doubleValue = _doubleValue;
if (doubleValue == null) {
doubleValue = _pinotDataType != null ? _pinotDataType.toDouble(_value) : NullValuePlaceHolder.DOUBLE;
_doubleValue = doubleValue;
}
return doubleValue;
}

public BigDecimal getBigDecimalValue() {
return _bigDecimalValue;
BigDecimal bigDecimalValue = _bigDecimalValue;
if (bigDecimalValue == null) {
bigDecimalValue = _pinotDataType != null ? _pinotDataType.toBigDecimal(_value) : NullValuePlaceHolder.BIG_DECIMAL;
_bigDecimalValue = bigDecimalValue;
}
return bigDecimalValue;
}

public String getStringValue() {
return String.valueOf(_value);
String stringValue = _stringValue;
if (stringValue == null) {
stringValue = _pinotDataType != null ? _pinotDataType.toString(_value) : NullValuePlaceHolder.STRING;
_stringValue = stringValue;
}
return stringValue;
}

@Nullable
public Object getValue() {
return _value;
public byte[] getBytesValue() {
byte[] bytesValue = _bytesValue;
if (bytesValue == null) {
bytesValue = _pinotDataType != null ? _pinotDataType.toBytes(_value) : NullValuePlaceHolder.BYTES;
_bytesValue = bytesValue;
}
return bytesValue;
}

// This ctor is only used for special handling in subquery.
public LiteralContext(FieldSpec.DataType type, Object value) {
_type = type;
if (type == FieldSpec.DataType.UNKNOWN) {
_value = null;
} else {
_value = value;
}
_bigDecimalValue = getBigDecimalValue(type, value);
public boolean isNull() {
return _value == null;
}

@Override
Expand All @@ -212,7 +265,13 @@ public boolean equals(Object o) {

@Override
public String toString() {
// TODO: print out the type.
return '\'' + String.valueOf(_value) + '\'';
switch (_type) {
case STRING:
return "'" + _value + "'";
case BYTES:
return "'" + BytesUtils.toHexString((byte[]) _value) + "'";
default:
return String.valueOf(_value);
}
}
}
3 changes: 3 additions & 0 deletions pinot-common/src/main/java/org/apache/pinot/common/request/context/RequestContextUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ public static FilterContext getFilter(FunctionContext filterFunction) {
}
}

// TODO: Pass the literal context into the Predicate so that we can read the value based on the data type. Currently
// literal context doesn't support float, and we cannot differentiate explicit string literal and literal
// without explicit type, so we always convert the literal into string.
private static String getStringValue(ExpressionContext expressionContext) {
if(expressionContext.getType() != ExpressionContext.Type.LITERAL){
throw new BadQueryRequestException(
Expand Down
12 changes: 6 additions & 6 deletions pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,9 @@ public double toDouble(Object value) {

@Override
public BigDecimal toBigDecimal(Object value) {
return BigDecimal.valueOf((Float) value);
// Use string representation of the value to create BigDecimal to avoid getting the exact floating-point value.
// new BigDecimal(123.45f) -> 123.4499969482421875
return new BigDecimal(value.toString());
}

@Override
Expand Down Expand Up @@ -446,11 +448,9 @@ public double toDouble(Object value) {

@Override
public BigDecimal toBigDecimal(Object value) {
// Note:
// - BigDecimal.valueOf(double): uses the canonical String representation of the double value passed
// in to instantiate the BigDecimal object.
// - new BigDecimal(double): attempts to represent the double value as accurately as possible.
return BigDecimal.valueOf((Double) value);
// Use string representation of the value to create BigDecimal to avoid getting the exact floating-point value.
// new BigDecimal(123.45) -> 123.4500000000000028421709430404007434844970703125
return new BigDecimal(value.toString());
}

@Override
Expand Down
Loading