Jackie-Jiang commented on code in PR #19586:
URL: https://github.com/apache/pinot/pull/19586#discussion_r4186737378
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ArrayLiteralTransformFunction.java:
##########
@@ -185,6 +274,43 @@ public
ArrayLiteralTransformFunction(List<ExpressionContext> literalContexts) {
_intArrayLiteral = null;
_longArrayLiteral = null;
_floatArrayLiteral = null;
+ _bigDecimalArrayLiteral = null;
+ _stringArrayLiteral = null;
+ _bytesArrayLiteral = null;
+ break;
+ case BIG_DECIMAL:
+ _bigDecimalArrayLiteral = new BigDecimal[literalContexts.size()];
+ for (int i = 0; i < _bigDecimalArrayLiteral.length; i++) {
+ _bigDecimalArrayLiteral[i] =
literalContexts.get(i).getLiteral().getBigDecimalValue();
+ }
+ _intArrayLiteral = null;
+ _longArrayLiteral = null;
+ _floatArrayLiteral = null;
+ _doubleArrayLiteral = null;
+ _stringArrayLiteral = null;
+ _bytesArrayLiteral = null;
+ break;
+ case BOOLEAN:
+ _intArrayLiteral = new int[literalContexts.size()];
+ for (int i = 0; i < _intArrayLiteral.length; i++) {
+ _intArrayLiteral[i] =
literalContexts.get(i).getLiteral().getBooleanValue() ? 1 : 0;
+ }
+ _longArrayLiteral = null;
+ _floatArrayLiteral = null;
+ _doubleArrayLiteral = null;
+ _bigDecimalArrayLiteral = null;
+ _stringArrayLiteral = null;
+ _bytesArrayLiteral = null;
+ break;
+ case TIMESTAMP:
Review Comment:
MAJOR: The new TIMESTAMP and UUID array branches are not reached for
ordinary single-stage SQL literals. `CompileTimeFunctionsInvoker` folds CAST
children before the outer array, and `RequestUtils.getLiteral(Object)` lowers
Timestamp to LONG and UUID to BYTES. Consequently, `ARRAY[CAST(... AS
TIMESTAMP)]` and `ARRAY[CAST(... AS UUID)]` are evaluated as LONG_ARRAY and
BYTES_ARRAY, rather than the advertised logical array types. Please preserve
element type through parsing and add query-level schema/result assertions for
both.
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/CompileTimeFunctionsInvoker.java:
##########
@@ -87,6 +87,9 @@ public static Expression
invokeCompileTimeFunctionExpression(@Nullable Expressio
return expression;
}
String canonicalName =
FunctionRegistry.canonicalize(function.getOperator());
+ if (canonicalName.equals("arrayvalueconstructor") ||
canonicalName.equals("array")) {
Review Comment:
MAJOR: Skipping compile-time folding for the `array` alias regresses `SELECT
array(1,2) FROM testTable`. That call previously folded to an INT_ARRAY
literal. `TransformFunctionFactory` only special-cases `arrayvalueconstructor`,
so `array` now reaches `ScalarTransformFunctionWrapper`, where the
constructor's Object return type maps to ColumnDataType.OBJECT and
`toDataType()` throws. Please handle the alias in the transform factory or
retain folding for it, and add a SQL regression test.
--
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]
