yashmayya commented on code in PR #19991:
URL: https://github.com/apache/pinot/pull/19991#discussion_r4382037336
##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/AggregateFunctionRewriteOptimizer.java:
##########
@@ -134,6 +139,31 @@ private static void addDataTypeArgument(Function function,
String functionName,
functionName, fieldSpec.isSingleValueField() ? "" : "multi-value ",
fieldSpec.getDataType()));
}
+ /// Appends the default tie reducer when missing, then the data column's
logical type when it is non-numeric.
+ private static void addModeDataTypeArgument(Function function, FieldSpec
fieldSpec) {
+ if (!fieldSpec.isSingleValueField()) {
+ return;
+ }
+ String dataType;
+ switch (fieldSpec.getDataType()) {
+ case STRING:
+ case BOOLEAN:
Review Comment:
This is still open at 269a7ebdc4. `addModeDataTypeArgument` still adds the
type argument for BOOLEAN and TIMESTAMP, so `MODE(boolCol)` breaks on old
servers during a rolling upgrade. Can you limit the rewrite to STRING and JSON
here, or put BOOLEAN and TIMESTAMP behind an off-by-default config like
`inferAnyValueType`?
--
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]
