nooneuse commented on code in PR #67469:
URL: https://github.com/apache/doris/pull/67469#discussion_r3931379147
##########
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)) {
+ throw new AnalysisException(getName() + " requires lg_max_k to be
a constant integer: " + this.toSql());
+ }
+ long value = ((IntegerLikeLiteral) lgMaxK).getLongValue();
+ if (value < MIN_LG_MAX_K || value > MAX_LG_MAX_K) {
Review Comment:
Thanks for identifying this. I agree that StateCombinator currently forwards
only checkLegalityBeforeTypeCoercion(), so nested post-rewrite validation is
not executed for the generated _state form.
I am not changing the generic StateCombinator behavior in this PR.
Forwarding post-rewrite checks or introducing an aggregate-combinator
capability would affect all aggregate functions and should be handled as a
separate FE change, rather than adding a DataSketches-specific workaround to
the generic combinator framework.
The BE still validates the [7, 21] range whenever a non-null row is
evaluated, so invalid values cannot create a populated state. For empty or
all-null input, no sketch is processed and the result remains an empty state.
This PR keeps the direct two-argument aggregate fully validated by FE and
limits its AggState change to allowing valid typed states to be consumed by
_merge and _union.
--
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]
