nooneuse commented on code in PR #67469:
URL: https://github.com/apache/doris/pull/67469#discussion_r3921935407
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/DataSketchesHllUnionAgg.java:
##########
@@ -39,27 +40,43 @@
/** datasketches_hll_union_agg agg function. */
public class DataSketchesHllUnionAgg extends NotNullableAggregateFunction
- implements UnaryExpression, ExplicitlyCastableSignature,
FunctionTrait, RollUpTrait {
+ implements ExplicitlyCastableSignature, FunctionTrait, RollUpTrait {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(DoubleType.INSTANCE).args(StringType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(VarcharType.SYSTEM_DEFAULT),
-
FunctionSignature.ret(DoubleType.INSTANCE).args(VarBinaryType.INSTANCE)
+
FunctionSignature.ret(DoubleType.INSTANCE).args(VarBinaryType.INSTANCE),
+
FunctionSignature.ret(DoubleType.INSTANCE).args(StringType.INSTANCE,
IntegerType.INSTANCE),
Review Comment:
Okay
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/DataSketchesHllUnionAgg.java:
##########
@@ -73,6 +90,27 @@ public void checkLegalityBeforeTypeCoercion() {
throw new AnalysisException(getName()
+ " function's argument should be of STRING/VARCHAR/VARBINARY
type, but was " + inputType);
}
+ if (arity() == 2
+ && (!getArgument(1).isConstant() ||
!getArgumentType(1).isIntegralType())) {
+ throw new AnalysisException(getName()
+ + " requires lg_max_k to be a constant integer: " +
this.toSql());
+ }
+ }
+
+ @Override
+ public void checkLegalityAfterRewrite() {
+ if (arity() == 1) {
+ return;
+ }
+ Expression lgMaxK = getArgument(1);
+ if (!(lgMaxK instanceof IntegerLikeLiteral)) {
Review Comment:
Thanks for pointing this out. I tested the exact query on the current PR head
with Nereids enabled and fallback disabled:
EXPLAIN SELECT DATASKETCHES_HLL_UNION_AGG(
DISTINCT sk, CAST(8 AS BIGINT))
FROM (SELECT CAST(NULL AS STRING) AS sk) t;
The query is planned successfully, and the partial aggregate receives the
normalized integer literal 8. I also verified CAST(4 + 4 AS BIGINT), abs(-8),
and nested integral casts.
Although FoldConstantRuleOnFE skips folding after entering an already-bound
DISTINCT aggregate, these constant child expressions are analyzed and folded
before that point. Therefore checkLegalityAfterRewrite sees an
IntegerLikeLiteral in these cases.
I could not reproduce the reported rejection on the reviewed head. If there
is
another query shape or session setting that leaves the expression as a Cast,
please provide the complete reproduction.
--
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]
