You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhengruifeng (via GitHub)" <gi...@apache.org> on 2023/10/10 10:07:46 UTC
[PR] [SPARK-45483][CONNECT] Correct the function groups in connect.functions [spark]
zhengruifeng opened a new pull request, #43309:
URL: https://github.com/apache/spark/pull/43309
### What changes were proposed in this pull request?
Correct the function groups in connect.functions
### Why are the changes needed?
to be consistent with https://github.com/apache/spark/commit/17da43803fd4c405fda00ffc2c7f4ff835ab24aa
### Does this PR introduce _any_ user-facing change?
yes, will changes the scaladoc (when it is available)
### How was this patch tested?
ci
### 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-45483][CONNECT] Correct the function groups in connect.functions [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #43309:
URL: https://github.com/apache/spark/pull/43309#discussion_r1353866402
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -8350,6 +8363,7 @@ object functions {
* function name that follows the SQL identifier syntax (can be quoted, can be qualified)
* @param cols
* the expression parameters of function
+ * @group normal_funcs
Review Comment:
I personally feel `calling_funcs` is a bit confusing, so let's follow `def expr` 's group
--
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-45483][CONNECT] Correct the function groups in connect.functions [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43309:
URL: https://github.com/apache/spark/pull/43309#discussion_r1352448420
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -8350,6 +8363,7 @@ object functions {
* function name that follows the SQL identifier syntax (can be quoted, can be qualified)
* @param cols
* the expression parameters of function
+ * @group normal_funcs
Review Comment:
I means to add a new group. If no need, `normal_funcs` is OK too.
--
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-45483][CONNECT] Correct the function groups in connect.functions [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43309:
URL: https://github.com/apache/spark/pull/43309#discussion_r1352226335
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -8350,6 +8363,7 @@ object functions {
* function name that follows the SQL identifier syntax (can be quoted, can be qualified)
* @param cols
* the expression parameters of function
+ * @group normal_funcs
Review Comment:
How about add a new group `calling_funcs`?
--
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-45483][CONNECT] Correct the function groups in connect.functions [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #43309:
URL: https://github.com/apache/spark/pull/43309#issuecomment-1758721600
thank you all for reviews
--
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-45483][CONNECT] Correct the function groups in connect.functions [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #43309:
URL: https://github.com/apache/spark/pull/43309#discussion_r1352309818
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -8350,6 +8363,7 @@ object functions {
* function name that follows the SQL identifier syntax (can be quoted, can be qualified)
* @param cols
* the expression parameters of function
+ * @group normal_funcs
Review Comment:
we don't have `calling_funcs` after https://github.com/apache/spark/commit/17da43803fd4c405fda00ffc2c7f4ff835ab24aa
because I feel it is more like to `def expr(expr: String): Column` which is in group `normal_funcs`
--
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-45483][CONNECT] Correct the function groups in connect.functions [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #43309: [SPARK-45483][CONNECT] Correct the function groups in connect.functions
URL: https://github.com/apache/spark/pull/43309
--
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