You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "sarutak (via GitHub)" <gi...@apache.org> on 2025/09/11 14:24:23 UTC
[PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
sarutak opened a new pull request, #8:
URL: https://github.com/apache/spark-connect-rust/pull/8
### What changes were proposed in this pull request?
This PR aims to fix `sum_distinct`.
In the current spark-connect-rust, `sum_distinct` doesn't work correctly.
If we run the following code,
```
let df = spark
.sql("SELECT * FROM VALUES (1), (2), (3), (1), (2), (3) AS data(value)")
.await?;
df.select([sum_distinct(col("value")).alias("sum")])
.show(None, None, None)
.await?;
```
We will get the following error.
```
Error: AnalysisException("[UNRESOLVED_ROUTINE] Cannot resolve routine `sum_distinct` on search path [`system`.`builtin`, `system`.`session`, `spark_catalog`.`default`]. SQLSTATE: 42883")
```
### Why are the changes needed?
Bug fix.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Added new tests.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #8:
URL: https://github.com/apache/spark-connect-rust/pull/8#issuecomment-3356028393
This PR has already approved and the last change is just inserting blank lines.
I'll merge this into `master`
Thank you for the review, @viirya !
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #8:
URL: https://github.com/apache/spark-connect-rust/pull/8#discussion_r2392130583
##########
crates/connect/src/functions/mod.rs:
##########
@@ -1108,7 +1108,19 @@ gen_func!(stddev, [col: Column], "Alias for stddev_samp.");
gen_func!(stddev_pop, [col: Column], "Returns population standard deviation of the expression in a group.");
gen_func!(stddev_samp, [col: Column], "Returns the unbiased sample standard deviation of the expression in a group.");
gen_func!(sum, [col: Column], "Returns the sum of all values in the expression.");
-gen_func!(sum_distinct, [col: Column], "Returns the sum of distinct values in the expression.");
+/// "Returns the sum of distinct values in the expression."
+pub fn sum_distinct(col: impl Into<Column>) -> Column {
Review Comment:
I wonder why the args are not the same as `count_distinct`?
rust
```rust
pub fn count_distinct<I>(col: impl Into<Column>, cols: Option<I>) -> Column
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak closed pull request #8: [SPARK-53554] Fix `sum_distinct`
URL: https://github.com/apache/spark-connect-rust/pull/8
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #8:
URL: https://github.com/apache/spark-connect-rust/pull/8#discussion_r2392130583
##########
crates/connect/src/functions/mod.rs:
##########
@@ -1108,7 +1108,19 @@ gen_func!(stddev, [col: Column], "Alias for stddev_samp.");
gen_func!(stddev_pop, [col: Column], "Returns population standard deviation of the expression in a group.");
gen_func!(stddev_samp, [col: Column], "Returns the unbiased sample standard deviation of the expression in a group.");
gen_func!(sum, [col: Column], "Returns the sum of all values in the expression.");
-gen_func!(sum_distinct, [col: Column], "Returns the sum of distinct values in the expression.");
+/// "Returns the sum of distinct values in the expression."
+pub fn sum_distinct(col: impl Into<Column>) -> Column {
Review Comment:
I wonder why the args are not the same as `count_distinct`?
rust
```rust
pub fn count_distinct<I>(col: impl Into<Column>, cols: Option<I>) -> Column
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #8:
URL: https://github.com/apache/spark-connect-rust/pull/8#issuecomment-3350601122
@viirya Could you help review this if possible? Thanks.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #8:
URL: https://github.com/apache/spark-connect-rust/pull/8#discussion_r2392132336
##########
crates/connect/src/functions/mod.rs:
##########
@@ -1108,7 +1108,19 @@ gen_func!(stddev, [col: Column], "Alias for stddev_samp.");
gen_func!(stddev_pop, [col: Column], "Returns population standard deviation of the expression in a group.");
gen_func!(stddev_samp, [col: Column], "Returns the unbiased sample standard deviation of the expression in a group.");
gen_func!(sum, [col: Column], "Returns the sum of all values in the expression.");
-gen_func!(sum_distinct, [col: Column], "Returns the sum of distinct values in the expression.");
+/// "Returns the sum of distinct values in the expression."
+pub fn sum_distinct(col: impl Into<Column>) -> Column {
+ Column::from(spark::Expression {
+ expr_type: Some(spark::expression::ExprType::UnresolvedFunction(
+ spark::expression::UnresolvedFunction {
+ function_name: "sum".to_string(),
+ arguments: VecExpression::from_iter(vec![col]).into(),
+ is_distinct: true,
+ is_user_defined_function: false,
+ },
+ )),
+ })
+}
Review Comment:
```suggestion
/// "Returns the sum of distinct values in the expression."
pub fn sum_distinct(col: impl Into<Column>) -> Column {
Column::from(spark::Expression {
expr_type: Some(spark::expression::ExprType::UnresolvedFunction(
spark::expression::UnresolvedFunction {
function_name: "sum".to_string(),
arguments: VecExpression::from_iter(vec![col]).into(),
is_distinct: true,
is_user_defined_function: false,
},
)),
})
}
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-53554] Fix `sum_distinct` [spark-connect-rust]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #8:
URL: https://github.com/apache/spark-connect-rust/pull/8#issuecomment-3352897707
@viirya Thank you for taking a look at this PR!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org