siddharthteotia commented on a change in pull request #6728:
URL: https://github.com/apache/incubator-pinot/pull/6728#discussion_r604762020
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java
##########
@@ -634,7 +754,7 @@ public PinotDataType getSingleValueType() {
}
public static PinotDataType getPinotDataType(FieldSpec fieldSpec) {
- FieldSpec.DataType dataType = fieldSpec.getDataType();
+ DataType dataType = fieldSpec.getDataType();
Review comment:
The semantics look confusing w.r.t the use of INTEGER_ARRAY v/s
PRIMITIVE_INT_ARRAY
FieldSpec for a MV column is mapped to INTEGER_ARRAY and not
PRIMITIVE_INT_ARRAY for PinotDataType
However, ColumnDataType is mapped to PRIMITIVE_INT_ARRAY
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
##########
@@ -127,62 +126,41 @@ public static Expression createNewLiteralExpression() {
return expression;
}
- public static Expression getLiteralExpression(String value) {
Review comment:
Why remove this?
Will it default to `getLiteralExpression (Object )` ?
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
##########
@@ -127,62 +126,41 @@ public static Expression createNewLiteralExpression() {
return expression;
}
- public static Expression getLiteralExpression(String value) {
+ public static Expression getLiteralExpression(long value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setStringValue(value);
+ expression.getLiteral().setLongValue(value);
return expression;
}
- public static Expression getLiteralExpression(byte[] value) {
+ public static Expression getLiteralExpression(double value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setStringValue(BytesUtils.toHexString(value));
+ expression.getLiteral().setDoubleValue(value);
return expression;
}
- public static Expression getLiteralExpression(Integer value) {
- return getLiteralExpression(value.longValue());
- }
-
- public static Expression getLiteralExpression(Long value) {
+ public static Expression getLiteralExpression(String value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setLongValue(value);
+ expression.getLiteral().setStringValue(value);
return expression;
}
- public static Expression getLiteralExpression(Float value) {
- return getLiteralExpression(value.doubleValue());
- }
-
- public static Expression getLiteralExpression(Double value) {
+ public static Expression getLiteralExpression(byte[] value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setDoubleValue(value);
+ expression.getLiteral().setStringValue(BytesUtils.toHexString(value));
return expression;
}
public static Expression getLiteralExpression(Object object) {
- if (object instanceof Integer) {
- return RequestUtils.getLiteralExpression((Integer) object);
- }
- if (object instanceof Long) {
- return RequestUtils.getLiteralExpression((Long) object);
- }
- if (object instanceof Float) {
- return RequestUtils.getLiteralExpression((Float) object);
- }
- if (object instanceof Double) {
- return RequestUtils.getLiteralExpression((Double) object);
- }
- if (object instanceof String) {
- return RequestUtils.getLiteralExpression((String) object);
+ if (object instanceof Integer || object instanceof Long) {
+ return RequestUtils.getLiteralExpression(((Number) object).longValue());
}
- if (object instanceof SqlLiteral) {
Review comment:
We need to handle the case for SqlLiteral right ?
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
##########
@@ -127,62 +126,41 @@ public static Expression createNewLiteralExpression() {
return expression;
}
- public static Expression getLiteralExpression(String value) {
+ public static Expression getLiteralExpression(long value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setStringValue(value);
+ expression.getLiteral().setLongValue(value);
return expression;
}
- public static Expression getLiteralExpression(byte[] value) {
+ public static Expression getLiteralExpression(double value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setStringValue(BytesUtils.toHexString(value));
+ expression.getLiteral().setDoubleValue(value);
return expression;
}
- public static Expression getLiteralExpression(Integer value) {
- return getLiteralExpression(value.longValue());
- }
-
- public static Expression getLiteralExpression(Long value) {
+ public static Expression getLiteralExpression(String value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setLongValue(value);
+ expression.getLiteral().setStringValue(value);
return expression;
}
- public static Expression getLiteralExpression(Float value) {
- return getLiteralExpression(value.doubleValue());
- }
-
- public static Expression getLiteralExpression(Double value) {
+ public static Expression getLiteralExpression(byte[] value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setDoubleValue(value);
+ expression.getLiteral().setStringValue(BytesUtils.toHexString(value));
return expression;
}
public static Expression getLiteralExpression(Object object) {
- if (object instanceof Integer) {
- return RequestUtils.getLiteralExpression((Integer) object);
- }
- if (object instanceof Long) {
- return RequestUtils.getLiteralExpression((Long) object);
- }
- if (object instanceof Float) {
- return RequestUtils.getLiteralExpression((Float) object);
- }
- if (object instanceof Double) {
- return RequestUtils.getLiteralExpression((Double) object);
- }
- if (object instanceof String) {
- return RequestUtils.getLiteralExpression((String) object);
+ if (object instanceof Integer || object instanceof Long) {
+ return RequestUtils.getLiteralExpression(((Number) object).longValue());
}
- if (object instanceof SqlLiteral) {
- return RequestUtils.getLiteralExpression((SqlLiteral) object);
+ if (object instanceof Float || object instanceof Double) {
+ return RequestUtils.getLiteralExpression(((Number)
object).doubleValue());
}
if (object instanceof byte[]) {
return RequestUtils.getLiteralExpression((byte[]) object);
}
- throw new SqlCompilationException(
Review comment:
We should throw the exception
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java
##########
@@ -127,62 +126,41 @@ public static Expression createNewLiteralExpression() {
return expression;
}
- public static Expression getLiteralExpression(String value) {
+ public static Expression getLiteralExpression(long value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setStringValue(value);
+ expression.getLiteral().setLongValue(value);
return expression;
}
- public static Expression getLiteralExpression(byte[] value) {
+ public static Expression getLiteralExpression(double value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setStringValue(BytesUtils.toHexString(value));
+ expression.getLiteral().setDoubleValue(value);
return expression;
}
- public static Expression getLiteralExpression(Integer value) {
- return getLiteralExpression(value.longValue());
- }
-
- public static Expression getLiteralExpression(Long value) {
+ public static Expression getLiteralExpression(String value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setLongValue(value);
+ expression.getLiteral().setStringValue(value);
return expression;
}
- public static Expression getLiteralExpression(Float value) {
- return getLiteralExpression(value.doubleValue());
- }
-
- public static Expression getLiteralExpression(Double value) {
+ public static Expression getLiteralExpression(byte[] value) {
Expression expression = createNewLiteralExpression();
- expression.getLiteral().setDoubleValue(value);
+ expression.getLiteral().setStringValue(BytesUtils.toHexString(value));
return expression;
}
public static Expression getLiteralExpression(Object object) {
- if (object instanceof Integer) {
- return RequestUtils.getLiteralExpression((Integer) object);
- }
- if (object instanceof Long) {
- return RequestUtils.getLiteralExpression((Long) object);
- }
- if (object instanceof Float) {
- return RequestUtils.getLiteralExpression((Float) object);
- }
- if (object instanceof Double) {
- return RequestUtils.getLiteralExpression((Double) object);
- }
- if (object instanceof String) {
- return RequestUtils.getLiteralExpression((String) object);
+ if (object instanceof Integer || object instanceof Long) {
+ return RequestUtils.getLiteralExpression(((Number) object).longValue());
}
- if (object instanceof SqlLiteral) {
- return RequestUtils.getLiteralExpression((SqlLiteral) object);
+ if (object instanceof Float || object instanceof Double) {
+ return RequestUtils.getLiteralExpression(((Number)
object).doubleValue());
}
if (object instanceof byte[]) {
return RequestUtils.getLiteralExpression((byte[]) object);
}
- throw new SqlCompilationException(
- new IllegalArgumentException("Unsupported Literal value type - " +
object.getClass()));
+ return RequestUtils.getLiteralExpression(object.toString());
Review comment:
I think this will undo the performance improvement done in
https://github.com/apache/incubator-pinot/pull/6314
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java
##########
@@ -84,41 +82,19 @@ public void init(@Nonnull List<TransformFunction>
arguments, @Nonnull Map<String
}
_jsonFieldTransformFunction = firstArgument;
_jsonPath = ((LiteralTransformFunction) arguments.get(1)).getLiteral();
- _resultsType = ((LiteralTransformFunction)
arguments.get(2)).getLiteral().toUpperCase();
- boolean isSingleValue = !_resultsType.toUpperCase().endsWith("_ARRAY");
+ String resultsType = ((LiteralTransformFunction)
arguments.get(2)).getLiteral().toUpperCase();
+ boolean isSingleValue = !resultsType.endsWith("_ARRAY");
try {
- FieldSpec.DataType fieldType =
FieldSpec.DataType.valueOf(_resultsType.split("_ARRAY")[0]);
-
+ DataType dataType =
+ DataType.valueOf(isSingleValue ? resultsType :
resultsType.substring(0, resultsType.length() - 6));
if (arguments.size() == 4) {
- String defaultValue = ((LiteralTransformFunction)
arguments.get(3)).getLiteral();
- switch (fieldType) {
- case INT:
- _defaultValue = Double.valueOf(defaultValue).intValue();
- break;
- case LONG:
- _defaultValue = Double.valueOf(defaultValue).longValue();
- break;
- case FLOAT:
- _defaultValue = Double.valueOf(defaultValue).floatValue();
- break;
- case DOUBLE:
- _defaultValue = Double.valueOf(defaultValue);
- break;
- case BOOLEAN:
- case STRING:
- _defaultValue = defaultValue;
- break;
- case BYTES:
- throw new UnsupportedOperationException(String.format(
- "Unsupported results type: BYTES for 'jsonExtractScalar' Udf.
Supported types are:
INT/LONG/FLOAT/DOUBLE/STRING/INT_ARRAY/LONG/FLOAT_ARRAY/DOUBLE_ARRAY/STRING_ARRAY",
- _resultsType));
- }
+ _defaultValue = dataType.convert(((LiteralTransformFunction)
arguments.get(3)).getLiteral());
}
- _resultMetadata = new TransformResultMetadata(fieldType, isSingleValue,
false);
+ _resultMetadata = new TransformResultMetadata(dataType, isSingleValue,
false);
} catch (Exception e) {
- throw new UnsupportedOperationException(String.format(
- "Unsupported results type: %s for 'jsonExtractScalar' Udf. Supported
types are:
INT/LONG/FLOAT/DOUBLE/STRING/INT_ARRAY/LONG/FLOAT_ARRAY/DOUBLE_ARRAY/STRING_ARRAY",
- _resultsType));
+ throw new IllegalStateException(String.format(
+ "Unsupported results type: %s for 'jsonExtractScalar' Udf. Supported
types are:
INT/LONG/FLOAT/DOUBLE/STRING/INT_ARRAY/LONG_ARRAY/FLOAT_ARRAY/DOUBLE_ARRAY/STRING_ARRAY",
Review comment:
(nit) Udf isn't really needed here right. This is an in-built function
just like other inbuilt aggregation and transform functions. It is not a UDF
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java
##########
@@ -490,82 +491,13 @@ public static ResultTable
renderResultTableWithoutOrdering(List<Object[]> rows,
return columnToIndexMap;
}
- /**
- * Converts a value into the given data type. (Broker side)
- * <p>Actual value type can be different with data type passed in, but they
must be type compatible.
- */
- public static Serializable convertValueToType(Object value,
DataSchema.ColumnDataType dataType) {
- switch (dataType) {
- // Single-value column
- case INT:
- return ((Number) value).intValue();
- case LONG:
- return ((Number) value).longValue();
- case FLOAT:
- return ((Number) value).floatValue();
- case DOUBLE:
- return ((Number) value).doubleValue();
- // NOTE: Return hex-encoded String for BYTES columns for
backward-compatibility
- // TODO: Revisit to see whether we should return byte[] instead
- case BYTES:
- return ((ByteArray) value).toHexString();
-
- // Multi-value column
- case LONG_ARRAY:
- // LONG_ARRAY type covers INT_ARRAY and LONG_ARRAY
- if (value instanceof int[]) {
- int[] ints = (int[]) value;
- int length = ints.length;
- long[] longs = new long[length];
- for (int i = 0; i < length; i++) {
- longs[i] = ints[i];
- }
- return longs;
- } else {
- return (long[]) value;
- }
- case DOUBLE_ARRAY:
- // DOUBLE_ARRAY type covers INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY and
DOUBLE_ARRAY
- if (value instanceof int[]) {
- int[] ints = (int[]) value;
- int length = ints.length;
- double[] doubles = new double[length];
- for (int i = 0; i < length; i++) {
- doubles[i] = ints[i];
- }
- return doubles;
- } else if (value instanceof long[]) {
- long[] longs = (long[]) value;
- int length = longs.length;
- double[] doubles = new double[length];
- for (int i = 0; i < length; i++) {
- doubles[i] = longs[i];
- }
- return doubles;
- } else if (value instanceof float[]) {
- float[] floats = (float[]) value;
- int length = floats.length;
- double[] doubles = new double[length];
- for (int i = 0; i < length; i++) {
- doubles[i] = floats[i];
- }
- return doubles;
- } else {
- return (double[]) value;
- }
-
- default:
- // For STRING, INT_ARRAY, FLOAT_ARRAY and STRING_ARRAY, no need to
format
- return (Serializable) value;
- }
- }
-
/**
* Formats a value into a {@code String} (single-value column) or {@code
String[]} (multi-value column) based on the
* data type. (Broker side)
* <p>Actual value type can be different with data type passed in, but they
must be type compatible.
*/
- public static Serializable getFormattedValue(Object value,
DataSchema.ColumnDataType dataType) {
+ @Deprecated
+ public static Serializable getFormattedValue(Object value, ColumnDataType
dataType) {
Review comment:
Because PQL endpoint has been deprecated right?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
