Jackie-Jiang commented on code in PR #19586:
URL: https://github.com/apache/pinot/pull/19586#discussion_r4108816794
##########
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:
Could we move BOOLEAN after BIG_DECIMAL so this switch follows INT, LONG,
FLOAT, DOUBLE, BIG_DECIMAL, BOOLEAN, TIMESTAMP, STRING, UUID? The separate
BYTES validation can stay above the switch.
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##########
@@ -359,6 +361,20 @@ public static Object arrayValueConstructor(Object... arr) {
}
return bytesArr;
}
+ if (clazz == Timestamp.class) {
Review Comment:
Could we keep the branches in the usual type order? That would put
BIG_DECIMAL before BOOLEAN and TIMESTAMP before STRING and BYTES, with UUID
last.
##########
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:
Could we order these new cases consistently with the production type order:
BIG_DECIMAL, BOOLEAN, TIMESTAMP, then the existing STRING and BYTES cases, and
UUID last?
--
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]
