HyukjinKwon commented on code in PR #57079:
URL: https://github.com/apache/spark/pull/57079#discussion_r3550380252
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:
##########
@@ -593,6 +608,13 @@ case class BinaryPredicate(override val prettyName:
String, left: Expression, ri
Returns NULL if either input expression is NULL. Otherwise, returns False.
Both left or right must be of STRING or BINARY type.
""",
+ arguments = """
+ Arguments:
+ * left - The expression to search within.
+ An expression that evaluates to a string.
Review Comment:
`contains` accepts binary as well as string —
`StringBinaryPredicateExpressionBuilderBase.build` routes `(BinaryType,
BinaryType)` to `BinaryPredicate`, and the usage above says "Both left or right
must be of STRING or BINARY type". Same on `right` (616), and on `startswith`
(663/665) and `endswith` (719/721) in this file.
```suggestion
An expression that evaluates to a string or binary.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -18403,8 +18940,10 @@ def contains(left: "ColumnOrName", right:
"ColumnOrName") -> Column:
----------
left : :class:`~pyspark.sql.Column` or str
The input column or strings to check, may be NULL.
+ A column that evaluates to a string.
Review Comment:
`contains` accepts binary too — the docstring says "Both left or right must
be of STRING or BINARY type" and the example passes `to_binary(...)`. Same on
`right` (18946), and on `startswith` (18629/18632) and `endswith` (18589/18592).
```suggestion
A column that evaluates to a string or binary.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala:
##########
@@ -263,6 +267,11 @@ case class MinutesOfTime(child: Expression)
sub-microsecond digits.
If `expr` is a TIME type (since 4.1.0), it returns the minute of the
time-of-day.
""",
+ arguments = """
+ Arguments:
+ * expr - The expression to extract the minute component from.
+ An expression that evaluates to a timestamp.
Review Comment:
`minute` accepts the TIME type as well as timestamp — the usage two lines up
says "If `expr` is a TIME type (since 4.1.0), it returns the minute of the
time-of-day", and `MinuteExpressionBuilder` dispatches `TimeType` to
`MinutesOfTime` (`inputTypes = Seq(AnyTimeType)`). Same on `hour` (345) and
`second` (442).
```suggestion
An expression that evaluates to a timestamp or time.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -10408,6 +10626,7 @@ def minute(col: "ColumnOrName") -> Column:
----------
col : :class:`~pyspark.sql.Column` or column name
target date/time/timestamp column to work on.
+ A column that evaluates to a timestamp.
Review Comment:
Same as the Scala side: `minute` accepts time as well as timestamp (the
docstring notes "Added support for time type" and "target date/time/timestamp
column"). Same on `hour` (10545) and `second` (10713).
```suggestion
A column that evaluates to a timestamp or time.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -449,6 +456,12 @@ object JsonToStructs {
// scalastyle:off line.size.limit
@ExpressionDescription(
usage = "_FUNC_(expr[, options]) - Returns a JSON string with a given struct
value",
+ arguments = """
+ Arguments:
+ * expr - The struct value to convert to a JSON string.
+ An expression that evaluates to an array or struct.
Review Comment:
`to_json` also accepts map and variant — `StructsToJson.checkInputDataTypes`
accepts `StructType | MapType | ArrayType | VariantType` (map input is a common
use). "array or struct" drops both.
```suggestion
An expression that evaluates to a struct, array, map, or variant.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -21952,6 +22583,7 @@ def to_json(col: "ColumnOrName", options:
Optional[Mapping[str, str]] = None) ->
----------
col : :class:`~pyspark.sql.Column` or str
name of column containing a struct, an array, a map, or a variant
object.
+ A column that evaluates to an array or struct.
Review Comment:
The docstring's first line already says "a StructType, ArrayType, MapType or
a VariantType" and the param prose says "a struct, an array, a map, or a
variant object" — so this type line should match rather than drop map and
variant.
```suggestion
A column that evaluates to a struct, array, map, or variant.
```
--
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]
