You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2026/10/05 17:16:22 UTC
Re: [PR] Complete SQL array literal support for logical types (#19338) [pinot]
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org