HyukjinKwon commented on code in PR #57079:
URL: https://github.com/apache/spark/pull/57079#discussion_r3550558552
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/First.scala:
##########
@@ -36,6 +36,12 @@ import org.apache.spark.sql.types._
usage = """
_FUNC_(expr[, isIgnoreNull]) - Returns the first value of `expr` for a
group of rows.
If `isIgnoreNull` is true, returns only non-null values.""",
+ arguments = """
+ Arguments:
+ * expr - The expression to return the first value of.
+ An expression of any type.
+ * isIgnoreNull - Whether to ignore null values and return only the first
non-null value.
Review Comment:
`isIgnoreNull` must be a boolean literal (`FirstLast.validateIgnoreNullExpr`
accepts only `Literal(b: Boolean, BooleanType)`), so it warrants a type line
like the other documented flags.
```suggestion
* isIgnoreNull - Whether to ignore null values and return only the
first non-null value.
An expression that evaluates to a boolean. Must be a constant.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Last.scala:
##########
@@ -35,6 +35,12 @@ import org.apache.spark.sql.types._
usage = """
_FUNC_(expr[, isIgnoreNull]) - Returns the last value of `expr` for a
group of rows.
If `isIgnoreNull` is true, returns only non-null values""",
+ arguments = """
+ Arguments:
+ * expr - The expression to return the last value of.
+ An expression of any type.
+ * isIgnoreNull - Whether to ignore null values and return only the last
non-null value.
Review Comment:
Same as `first`: `isIgnoreNull` is a boolean constant.
```suggestion
* isIgnoreNull - Whether to ignore null values and return only the
last non-null value.
An expression that evaluates to a boolean. Must be a constant.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/AnyValue.scala:
##########
@@ -34,6 +34,12 @@ import org.apache.spark.sql.types._
usage = """
_FUNC_(expr[, isIgnoreNull]) - Returns some value of `expr` for a group of
rows.
If `isIgnoreNull` is true, returns only non-null values.""",
+ arguments = """
+ Arguments:
+ * expr - The expression to return some value of for a group of rows.
+ An expression of any type.
+ * isIgnoreNull - Whether to return only non-null values.
Review Comment:
Same: `isIgnoreNull` is a boolean constant.
```suggestion
* isIgnoreNull - Whether to return only non-null values.
An expression that evaluates to a boolean. Must be a constant.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Mode.scala:
##########
@@ -233,6 +233,13 @@ case class Mode(
_FUNC_() WITHIN GROUP (ORDER BY col) - Returns the most frequent value for
the values within `col` (specified in ORDER BY clause). NULL values are ignored.
If all the values are NULL, or there are 0 rows, returns NULL. When
multiple values have the same greatest frequency only one value will be
returned.
The value will be chosen based on sort direction. Return the smallest
value if sort direction is asc or the largest value if sort direction is desc
from multiple values with the same frequency.""",
+ arguments = """
+ Arguments:
+ * col - The column to compute the most frequent value of.
+ An expression of any type.
+ * deterministic - Whether to return a deterministic result when there
are multiple most
+ frequent values.
Review Comment:
`deterministic` is enforced foldable-boolean (Mode.scala:275-283). Adding
the type line here; also fixes the continuation indent of the wrapped
description.
```suggestion
* deterministic - Whether to return a deterministic result when there
are multiple most
frequent values.
An expression that evaluates to a boolean. Must be a constant.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1044,6 +1060,12 @@ case class MapSort(base: Expression)
elements for double/float type. Null elements will be placed at the
beginning of the returned
array in ascending order or at the end of the returned array in
descending order.
""",
+ arguments = """
+ Arguments:
+ * array - The array to sort.
+ An expression that evaluates to an array.
+ * ascendingOrder - Whether to sort in ascending order; false sorts in
descending order.
Review Comment:
`ascendingOrder` is enforced foldable-boolean
(collectionOperations.scala:1092-1099).
```suggestion
* ascendingOrder - Whether to sort in ascending order; false sorts in
descending order.
An expression that evaluates to a boolean. Must be a constant.
```
--
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]
