clintropolis commented on code in PR #14249:
URL: https://github.com/apache/druid/pull/14249#discussion_r1239581927
##########
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchApproxCountDistinctSqlAggregator.java:
##########
@@ -21,24 +21,29 @@
import org.apache.calcite.sql.SqlAggFunction;
import org.apache.calcite.sql.SqlFunctionCategory;
-import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.type.InferTypes;
-import org.apache.calcite.sql.type.OperandTypes;
-import org.apache.calcite.sql.type.ReturnTypes;
import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.druid.query.aggregation.AggregatorFactory;
import
org.apache.druid.query.aggregation.post.FinalizingFieldAccessPostAggregator;
import org.apache.druid.sql.calcite.aggregation.Aggregation;
import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
import java.util.Collections;
public class HllSketchApproxCountDistinctSqlAggregator extends
HllSketchBaseSqlAggregator implements SqlAggregator
{
public static final String NAME = "APPROX_COUNT_DISTINCT_DS_HLL";
-
- private static final SqlAggFunction FUNCTION_INSTANCE = new
HllSketchApproxCountDistinctSqlAggFunction();
+ private static final SqlAggFunction FUNCTION_INSTANCE =
+ OperatorConversions.aggregatorBuilder(NAME)
+ .operandTypes(SqlTypeFamily.ANY,
SqlTypeFamily.NUMERIC, SqlTypeFamily.STRING)
Review Comment:
not a big deal, but does this lose the nice signature that provides the
parameter names in the validation error? similar comment on other changes that
drop the `OperandTypes.sequence`.
I personally find the more verbose `OperandTypes.or` form of using
`operandTypeChecker` to be a bit easier to understand the shape of the function
at a glance than trying to piece it together from looking at all of
`operandTypes`, `literalOperands` and `requiredOperandCount`, but i guess it
isn't a big deal and is also consistent with what is allowed when using the
builder to spit out `SqlFunction`.
--
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]
