yashmayya commented on code in PR #18628:
URL: https://github.com/apache/pinot/pull/18628#discussion_r3331091520
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ScalarTransformFunctionWrapper.java:
##########
@@ -482,4 +486,65 @@ private void getNonLiteralValues(ValueBlock valueBlock) {
}
}
}
+
+ private static Object[] transformToObjectValues(TransformFunction
transformFunction, ValueBlock valueBlock) {
+ TransformResultMetadata resultMetadata =
transformFunction.getResultMetadata();
+ DataType dataType = resultMetadata.getDataType();
+ if (resultMetadata.isSingleValue()) {
+ switch (dataType) {
+ case BOOLEAN: {
+ int[] intValues =
transformFunction.transformToIntValuesSV(valueBlock);
+ int numValues = intValues.length;
+ Boolean[] booleanValues = new Boolean[numValues];
+ for (int i = 0; i < numValues; i++) {
+ booleanValues[i] = intValues[i] == 1;
+ }
+ return booleanValues;
+ }
+ case INT:
+ return
ArrayUtils.toObject(transformFunction.transformToIntValuesSV(valueBlock));
+ case LONG:
+ return
ArrayUtils.toObject(transformFunction.transformToLongValuesSV(valueBlock));
+ case FLOAT:
+ return
ArrayUtils.toObject(transformFunction.transformToFloatValuesSV(valueBlock));
+ case DOUBLE:
+ return
ArrayUtils.toObject(transformFunction.transformToDoubleValuesSV(valueBlock));
+ case BIG_DECIMAL:
+ return transformFunction.transformToBigDecimalValuesSV(valueBlock);
+ case TIMESTAMP: {
+ long[] longValues =
transformFunction.transformToLongValuesSV(valueBlock);
+ int numValues = longValues.length;
+ Timestamp[] timestampValues = new Timestamp[numValues];
+ for (int i = 0; i < numValues; i++) {
+ timestampValues[i] = new Timestamp(longValues[i]);
+ }
+ return timestampValues;
+ }
+ case STRING:
+ return transformFunction.transformToStringValuesSV(valueBlock);
+ case BYTES:
+ return transformFunction.transformToBytesValuesSV(valueBlock);
+ default:
+ throw new IllegalStateException("Unsupported data type: " +
dataType);
Review Comment:
JSON isn't handled, so a JSON column or JSON-returning expr passed to an
Object param (e.g. `nullIf(jsonCol, '...')`) hits this default and throws —
even though JSON is stored as STRING. Can we add `case JSON:` next to STRING
(here and in the MV switch)? There's a `jsonSV` test column handy to cover it.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ScalarTransformFunctionWrapper.java:
##########
@@ -482,4 +486,65 @@ private void getNonLiteralValues(ValueBlock valueBlock) {
}
}
}
+
+ private static Object[] transformToObjectValues(TransformFunction
transformFunction, ValueBlock valueBlock) {
+ TransformResultMetadata resultMetadata =
transformFunction.getResultMetadata();
+ DataType dataType = resultMetadata.getDataType();
+ if (resultMetadata.isSingleValue()) {
+ switch (dataType) {
+ case BOOLEAN: {
+ int[] intValues =
transformFunction.transformToIntValuesSV(valueBlock);
+ int numValues = intValues.length;
+ Boolean[] booleanValues = new Boolean[numValues];
+ for (int i = 0; i < numValues; i++) {
+ booleanValues[i] = intValues[i] == 1;
+ }
+ return booleanValues;
+ }
+ case INT:
+ return
ArrayUtils.toObject(transformFunction.transformToIntValuesSV(valueBlock));
+ case LONG:
+ return
ArrayUtils.toObject(transformFunction.transformToLongValuesSV(valueBlock));
+ case FLOAT:
+ return
ArrayUtils.toObject(transformFunction.transformToFloatValuesSV(valueBlock));
+ case DOUBLE:
+ return
ArrayUtils.toObject(transformFunction.transformToDoubleValuesSV(valueBlock));
+ case BIG_DECIMAL:
+ return transformFunction.transformToBigDecimalValuesSV(valueBlock);
+ case TIMESTAMP: {
+ long[] longValues =
transformFunction.transformToLongValuesSV(valueBlock);
+ int numValues = longValues.length;
+ Timestamp[] timestampValues = new Timestamp[numValues];
+ for (int i = 0; i < numValues; i++) {
+ timestampValues[i] = new Timestamp(longValues[i]);
+ }
+ return timestampValues;
+ }
+ case STRING:
+ return transformFunction.transformToStringValuesSV(valueBlock);
+ case BYTES:
+ return transformFunction.transformToBytesValuesSV(valueBlock);
+ default:
+ throw new IllegalStateException("Unsupported data type: " +
dataType);
+ }
+ }
+ switch (dataType) {
+ case INT:
+ return transformFunction.transformToIntValuesMV(valueBlock);
+ case LONG:
+ return transformFunction.transformToLongValuesMV(valueBlock);
+ case FLOAT:
+ return transformFunction.transformToFloatValuesMV(valueBlock);
+ case DOUBLE:
+ return transformFunction.transformToDoubleValuesMV(valueBlock);
+ case BIG_DECIMAL:
+ return transformFunction.transformToBigDecimalValuesMV(valueBlock);
+ case STRING:
+ return transformFunction.transformToStringValuesMV(valueBlock);
+ case BYTES:
+ return transformFunction.transformToBytesValuesMV(valueBlock);
+ default:
+ throw new IllegalStateException("Unsupported multi-value data type: "
+ dataType);
Review Comment:
MV switch is missing BOOLEAN and TIMESTAMP, which the SV switch above
handles — an MV boolean/timestamp child feeding an Object param would throw
here. Intended, or should it mirror the SV cases?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ScalarTransformFunctionWrapper.java:
##########
@@ -482,4 +486,65 @@ private void getNonLiteralValues(ValueBlock valueBlock) {
}
}
}
+
+ private static Object[] transformToObjectValues(TransformFunction
transformFunction, ValueBlock valueBlock) {
Review Comment:
Most of this helper (the numeric/decimal/timestamp/boolean/bytes SV branches
and the whole MV switch) isn't exercised by the tests — only the STRING path
is. Worth a couple more cases, especially one where the Object function returns
null so the null-placeholder path gets hit.
--
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]
