AnkitaAdvitot commented on code in PR #19586:
URL: https://github.com/apache/pinot/pull/19586#discussion_r4118586351
##########
pinot-common/src/test/java/org/apache/pinot/common/request/context/LiteralContextTest.java:
##########
@@ -276,10 +277,70 @@ public Object[][] arrayBackedValues() {
{DataType.DOUBLE, new double[]{1.0, 2.0}, new double[]{1.0, 2.0}, new
double[]{1.0, 3.0}},
{DataType.STRING, new String[]{"one", "two"}, new String[]{"one",
"two"}, new String[]{"one", "three"}},
{DataType.BYTES, new byte[]{1, 2}, new byte[]{1, 2}, new byte[]{1, 3}},
- {DataType.BYTES, new byte[][]{{1}, {2}}, new byte[][]{{1}, {2}}, new
byte[][]{{1}, {3}}}
+ {DataType.BYTES, new byte[][]{{1}, {2}}, new byte[][]{{1}, {2}}, new
byte[][]{{1}, {3}}},
+ {DataType.BOOLEAN, new boolean[]{true, false}, new boolean[]{true,
false}, new boolean[]{true, true}},
Review Comment:
Reordered the cases in `LiteralContextTest.arrayBackedValues()` and the
corresponding test methods to match the standard production order
(`BIG_DECIMAL, BOOLEAN, TIMESTAMP, STRING, BYTES, UUID`), and added
`testTimestampLiteral` for full branch coverage.
##########
pinot-common/src/main/java/org/apache/pinot/common/request/context/LiteralContext.java:
##########
@@ -169,8 +168,8 @@ private static PinotDataType getPinotDataType(DataType
type, @Nullable Object va
boolean singleValue = !value.getClass().isArray();
switch (type) {
case BOOLEAN:
- Preconditions.checkState(singleValue, "Boolean array is not
supported");
- return PinotDataType.BOOLEAN;
+ return singleValue ? PinotDataType.BOOLEAN
Review Comment:
Extended `ArrayLiteralTransformFunction` to support `BOOLEAN`,
`BIG_DECIMAL`, `TIMESTAMP`, and `UUID` in both constructors (`LiteralContext`
and `List<ExpressionContext>`), implemented `transformToBigDecimalValuesMV`,
and added cross-type MV conversions. In addition, prevented
`CompileTimeFunctionsInvoker` from folding `arrayValueConstructor` into
unsupported Thrift literals, ensuring the logical array types are preserved and
evaluated by `ArrayLiteralTransformFunction`. Added unit tests in
`ArrayLiteralTransformFunctionTest` and an end-to-end query regression test in
`TransformQueriesTest.testArrayLiteralQueries`.
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionUtils.java:
##########
@@ -91,6 +91,7 @@ private FunctionUtils() {
put(Timestamp[].class, ColumnDataType.TIMESTAMP_ARRAY);
put(String[].class, ColumnDataType.STRING_ARRAY);
put(byte[][].class, ColumnDataType.BYTES_ARRAY);
+ put(UUID[].class, ColumnDataType.UUID_ARRAY);
Review Comment:
Added `UUID_ARRAY` in `FunctionUtils.getRelDataType` to return `ARRAY<UUID>`
so planner and executor agree. In `ScalarTransformFunctionWrapper`, implemented
`transformToBytesValuesMV` (supporting `BYTES_ARRAY` and `UUID_ARRAY`) as well
as `transformToBigDecimalValuesMV`, and added `UUID_ARRAY` deserialization in
`getNonLiteralValues`. Added unit tests in `FunctionUtilsTest` and
`ScalarTransformFunctionWrapperTest`.
--
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]
