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/09/26 08:21:14 UTC
[GitHub] [spark] zhengruifeng opened a new pull request, #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
zhengruifeng opened a new pull request, #43121:
URL: https://github.com/apache/spark/pull/43121
### What changes were proposed in this pull request?
Correct the group of `ElementAt` and `TryElementAt`, they both support array and map input, so should be in `collection functions`.
Existing category strategy seems like this: if a function should be in `collection functions`, if it support more than one types:
- `Size` supports both array and map, it is in `collection functions`;
- `Reverse` supports both array and string, it is in `collection functions`;
So far, I didn't find other places with incorrect groups.
### Why are the changes needed?
for docs
### Does this PR introduce _any_ user-facing change?
yes, they will be in `collection functions` in SQL references
### 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
[GitHub] [spark] zhengruifeng commented on a diff in pull request #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #43121:
URL: https://github.com/apache/spark/pull/43121#discussion_r1337896202
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -2333,7 +2333,7 @@ case class Get(
b
""",
since = "2.4.0",
- group = "map_funcs")
+ group = "collection_funcs")
Review Comment:
I don't think it means `collection_funcs` is forbidden.
`Concat` works with multiple types, including string, binary and array, so it's not just an array function;
As I mentioned above, it looks like a `collection function` supports collection type (map/array/struct/etc) and some other types.
--
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
[GitHub] [spark] LuciferYang commented on a diff in pull request #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43121:
URL: https://github.com/apache/spark/pull/43121#discussion_r1337197585
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -2333,7 +2333,7 @@ case class Get(
b
""",
since = "2.4.0",
- group = "map_funcs")
+ group = "collection_funcs")
Review Comment:
https://github.com/apache/spark/blob/2aa06fcf1607bbad9e09649e587493032e739e35/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionDescription.java#L110-L116
I think you are right, but from the comments of `ExpressionDescription#group`, it seems we shouldn't directly use `collection_funcs`. Perhaps we should amend this comment. On the other hand, maybe the grouping of other functions needs correction, for instance, should `Concat` be categorized under `array_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
[GitHub] [spark] zhengruifeng commented on pull request #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #43121:
URL: https://github.com/apache/spark/pull/43121#issuecomment-1736756482
thanks all, merged to master
--
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
[GitHub] [spark] zhengruifeng closed pull request #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
URL: https://github.com/apache/spark/pull/43121
--
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
[GitHub] [spark] LuciferYang commented on a diff in pull request #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43121:
URL: https://github.com/apache/spark/pull/43121#discussion_r1337197585
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -2333,7 +2333,7 @@ case class Get(
b
""",
since = "2.4.0",
- group = "map_funcs")
+ group = "collection_funcs")
Review Comment:
https://github.com/apache/spark/blob/2aa06fcf1607bbad9e09649e587493032e739e35/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionDescription.java#L110-L116
From the comments of `ExpressionDescription#group`, it seems we shouldn't directly use `collection_funcs`. Perhaps we should amend this comment. On the other hand, maybe the grouping of other functions needs correction, for instance, should `Concat` be categorized under `array_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
[GitHub] [spark] zhengruifeng commented on pull request #43121: [SPARK-45335][SQL][DOCS] Correct the group of `ElementAt` and `TryElementAt`
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #43121:
URL: https://github.com/apache/spark/pull/43121#issuecomment-1736468116
cc @cloud-fan @HyukjinKwon 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