AnkitaAdvitot commented on code in PR #19586:
URL: https://github.com/apache/pinot/pull/19586#discussion_r4203841359
##########
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:
Handled both:
1. Updated `TransformFunctionFactory` to recognize the `array` alias
alongside `arrayValueConstructor` and route it to
`ArrayLiteralTransformFunction`.
2. Updated `CompileTimeFunctionsInvoker` to retain compile-time folding for
both `array` and `arrayvalueconstructor` when the element data types have
native Thrift literal representations (`INT`, `LONG`, `FLOAT`, `DOUBLE`,
`STRING`, `BYTES`), so `SELECT array(1, 2) FROM testTable` folds to an
`INT_ARRAY` literal as before.
3. Added regression tests in `CalciteSqlCompilerTest` and query-level
execution tests in `TransformQueriesTest`.
##########
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:
Addressed:
1. In `CompileTimeFunctionsInvoker`, when encountering array constructors
(`arrayvalueconstructor` or `array`), child operands that are `CAST(... AS
TIMESTAMP)` or `CAST(... AS UUID)` are no longer folded into `LONG` or `BYTES`
Thrift literals (which would lower logical type information); their inner
expressions are compiled, but the cast expression node is preserved and outer
array folding is skipped.
2. In `ArrayLiteralTransformFunction`, added support for resolving child
`CAST` expressions into `LiteralContext` instances preserving logical
`DataType.TIMESTAMP` and `DataType.UUID`, which now properly populates
`_dataType` and results in `TIMESTAMP_ARRAY` and `UUID_ARRAY`.
3. Added query-level schema and formatted result assertions for
`ARRAY[CAST(... AS TIMESTAMP)]`, `ARRAY[CAST(... AS UUID)]`, `array(CAST(... AS
TIMESTAMP))`, and `array(CAST(... AS UUID))` in `TransformQueriesTest`, along
with unit tests in `CalciteSqlCompilerTest` and
`ArrayLiteralTransformFunctionTest`.
--
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]
