xiangfu0 commented on code in PR #19990:
URL: https://github.com/apache/pinot/pull/19990#discussion_r4366215185
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateTypeArgumentRewriteRule.java:
##########
@@ -158,7 +186,24 @@ private static String getDataTypeArgument(AggregateCall
aggCall, RelNode input)
return "UUID";
default:
throw new IllegalArgumentException(
- String.format("Unsupported data type for %s(dataColumn,
timeColumn): %s", name, dataType));
+ String.format("Unsupported data type for %s(%s): %s", name,
arguments, dataType));
+ }
+ }
+
+ /// The result type of ANY_VALUE follows its data argument, so an explicit
data type argument must be the one this
+ /// rule supplies for it.
+ private static void checkAnyValueDataTypeArgument(String name, List<Integer>
argList, RelNode input) {
Review Comment:
Worth flagging for users: the explicit 2-arg form behaves differently per
engine. Single-stage happily reads a convertible explicit type
(`ANY_VALUE(intCol, 'STRING')` -> "10"), but here the multi-stage rule rejects
anything other than the inferred type. It's intentional and documented in the
PR, just an easy surprise — maybe a one-line note in the function docs.
Non-blocking.
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java:
##########
@@ -58,19 +62,62 @@
/// ```
public class AnyValueAggregationFunction extends
BaseSingleInputAggregationFunction<Object, Comparable<?>> {
private static final DataType[] DATA_TYPE_VALUES = DataType.values();
- // Result type is determined at runtime based on input expression type
+ // Data type argument, or null when the result type is determined at runtime
from the input blocks
+ @Nullable
+ private final DataType _dataType;
private ColumnDataType _resultType;
public AnyValueAggregationFunction(List<ExpressionContext> arguments,
boolean nullHandlingEnabled) {
- super(verifySingleArgument(arguments, "ANY_VALUE"), nullHandlingEnabled);
- _resultType = null;
+ super(getValueArgument(arguments), nullHandlingEnabled);
+ if (arguments.size() == 2) {
+ ExpressionContext dataTypeArgument = arguments.get(1);
+ Preconditions.checkArgument(dataTypeArgument.getType() ==
ExpressionContext.Type.LITERAL,
+ "ANY_VALUE expects the 2nd argument to be literal, got: %s",
dataTypeArgument.getType());
+ _dataType =
DataType.valueOf(dataTypeArgument.getLiteral().getStringValue().toUpperCase());
Review Comment:
Tiny robustness nit: if a user writes the 2-arg form directly with a bad
type name (e.g. `ANY_VALUE(col, 'FOO')`), `DataType.valueOf` throws a raw
`IllegalArgumentException`. The optimizer-supplied path is always valid, so
this only hits hand-written queries — but wrapping it as a
`BadQueryRequestException` would surface it as a clean 400 instead of a server
error. Non-blocking.
--
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]
