This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 9aab260c2b3c [SPARK-53868][SQL] Only use signature with Expression[]
of `visitAggregateFunction` in V2ExpressionSQBuilder
9aab260c2b3c is described below
commit 9aab260c2b3c1a36347fbd97c32993cd01f58b8c
Author: alekjarmov <[email protected]>
AuthorDate: Sun Oct 12 12:47:00 2025 +0800
[SPARK-53868][SQL] Only use signature with Expression[] of
`visitAggregateFunction` in V2ExpressionSQBuilder
### What changes were proposed in this pull request?
https://github.com/apache/spark/pull/50143 This PR introduced
`visitAggregateFunction` with `inputs: Array[Expression]`, but it's not the
only usage of `visitAggregateFunction` for example here
https://github.com/apache/spark/blob/6eb4d3c9d38f6849b0acfcffdbadce03c8f49ac6/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java#L134
the old API is still used but it no longer checked if the function is
supported.
This means that if some dialect did not support one of `(MIN, MAX, COUNT,
SUM, AVG)` it would not be blocked, but as of now all the dialects have them in
the `supportedFunctions` so this is not a behavioral change.
### Why are the changes needed?
To unify the API in case in the future if some dialect does not support an
aggregate function of `(MIN, MAX, COUNT, SUM, AVG)` it should be blocked.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing tests.
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #52573 from alekjarmov/block-aggregates.
Lead-authored-by: alekjarmov <[email protected]>
Co-authored-by: Alek Jarmov <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
---
.../sql/connector/util/V2ExpressionSQLBuilder.java | 30 +++++++++++++---------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git
a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
index 2bc994acaf33..877cf3dd765c 100644
---
a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
+++
b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java
@@ -131,22 +131,17 @@ public class V2ExpressionSQLBuilder {
default -> visitUnexpectedExpr(expr);
};
} else if (expr instanceof Min min) {
- return visitAggregateFunction("MIN", false,
- expressionsToStringArray(min.children()));
+ return visitAggregateFunction("MIN", false, min.children());
} else if (expr instanceof Max max) {
- return visitAggregateFunction("MAX", false,
- expressionsToStringArray(max.children()));
+ return visitAggregateFunction("MAX", false, max.children());
} else if (expr instanceof Count count) {
- return visitAggregateFunction("COUNT", count.isDistinct(),
- expressionsToStringArray(count.children()));
+ return visitAggregateFunction("COUNT", count.isDistinct(),
count.children());
} else if (expr instanceof Sum sum) {
- return visitAggregateFunction("SUM", sum.isDistinct(),
- expressionsToStringArray(sum.children()));
- } else if (expr instanceof CountStar) {
- return visitAggregateFunction("COUNT", false, new String[]{"*"});
+ return visitAggregateFunction("SUM", sum.isDistinct(), sum.children());
+ } else if (expr instanceof CountStar countStar) {
+ return visitAggregateFunction("COUNT", false, countStar.children());
} else if (expr instanceof Avg avg) {
- return visitAggregateFunction("AVG", avg.isDistinct(),
- expressionsToStringArray(avg.children()));
+ return visitAggregateFunction("AVG", avg.isDistinct(), avg.children());
} else if (expr instanceof GeneralAggregateFunc f) {
if (f.orderingWithinGroups().length == 0) {
return visitAggregateFunction(f.name(), f.isDistinct(), f.children());
@@ -282,8 +277,19 @@ public class V2ExpressionSQLBuilder {
return joinArrayToString(inputs, ", ", funcName + "(", ")");
}
+ /**
+ * Builds SQL for an aggregate function.
+ *
+ * In V2ExpressionSQLBuilder, always use this override (with Expression[])
+ * instead of the String[] version, as the String[] version does not validate
+ * whether the function is supported in JDBC dialects.
+ */
protected String visitAggregateFunction(
String funcName, boolean isDistinct, Expression[] inputs) {
+ // CountStar has no children but should return with a star
+ if (funcName.equals("COUNT") && inputs == Expression.EMPTY_EXPRESSION) {
+ return visitAggregateFunction(funcName, isDistinct, new String[]{"*"});
+ }
return visitAggregateFunction(funcName, isDistinct,
expressionsToStringArray(inputs));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
