HyukjinKwon commented on code in PR #57079:
URL: https://github.com/apache/spark/pull/57079#discussion_r3551296811
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numberFormatExpressions.scala:
##########
@@ -223,6 +237,13 @@ case class TryToNumber(left: Expression, right: Expression)
'hex': a string in the hexadecimal format.
'utf-8': the input binary is decoded to UTF-8 string.
""",
+ arguments = """
+ Arguments:
+ * expr - The expression to convert to a string.
+ An expression that evaluates to a decimal, timestamp, or time.
Review Comment:
Still drops binary. `ToCharacterBuilder.build` routes a `BinaryType` input
to `Base64`/`Hex`/`Decode`, and the examples below use `x'537061726b2053514c'`
('base64'/'hex') and `encode('abc', 'utf-8')`.
```suggestion
An expression that evaluates to a decimal, timestamp, time, or
binary.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -17310,8 +17814,10 @@ def to_char(col: "ColumnOrName", format:
"ColumnOrName") -> Column:
----------
col : :class:`~pyspark.sql.Column` or str
Input column or strings.
+ A column that evaluates to a decimal, timestamp, or time.
Review Comment:
Same as the Scala side: `to_char` accepts binary too (the docstring says "If
`col` is a binary ...").
```suggestion
A column that evaluates to a decimal, timestamp, time, or binary.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TryEval.scala:
##########
@@ -58,6 +58,13 @@ case class TryEval(child: Expression) extends
UnaryExpression {
@ExpressionDescription(
usage = "_FUNC_(expr1, expr2) - Returns the sum of `expr1`and `expr2` and
the result is null on overflow. " +
"The acceptable input types are the same with the `+` operator.",
+ arguments = """
+ Arguments:
+ * expr1 - The first addend.
+ An expression that evaluates to a numeric, interval, date, timestamp,
or time.
+ * expr2 - The second addend.
+ An expression that evaluates to a numeric or interval.
Review Comment:
`expr1` was widened but `expr2` wasn't. Addition is commutative —
`BinaryArithmeticWithDatetimeResolver` handles `(interval, DateType)`,
`(DayTimeInterval, TimeType)`, etc., so `expr2` accepts the same set as `expr1`.
```suggestion
An expression that evaluates to a numeric, interval, date,
timestamp, or time.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TryEval.scala:
##########
@@ -219,6 +247,13 @@ case class TrySubtract(left: Expression, right:
Expression, replacement: Express
@ExpressionDescription(
usage = "_FUNC_(expr1, expr2) - Returns `expr1`*`expr2` and the result is
null on overflow. " +
"The acceptable input types are the same with the `*` operator.",
+ arguments = """
+ Arguments:
+ * expr1 - The first factor.
+ An expression that evaluates to a numeric or interval.
+ * expr2 - The second factor.
+ An expression that evaluates to a numeric.
Review Comment:
`expr2` should match `expr1` — `MultiplyInterval` handles both `(interval,
numeric)` and `(numeric, interval)`
(BinaryArithmeticWithDatetimeResolver.scala:149-150), so `numeric * interval`
is valid.
```suggestion
An expression that evaluates to a numeric or interval.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -553,7 +555,9 @@ def try_add(left: "ColumnOrName", right: "ColumnOrName") ->
Column:
Parameters
----------
left : :class:`~pyspark.sql.Column` or column name
+ A column that evaluates to a numeric, interval, date, timestamp, or
time.
right : :class:`~pyspark.sql.Column` or column name
+ A column that evaluates to a numeric or interval.
Review Comment:
Same asymmetry as the Scala side: `try_add`'s second operand accepts the
same types as the first (addition is commutative).
```suggestion
A column that evaluates to a numeric, interval, date, timestamp, or
time.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -17356,8 +17862,10 @@ def to_varchar(col: "ColumnOrName", format:
"ColumnOrName") -> Column:
----------
col : :class:`~pyspark.sql.Column` or str
Input column or strings.
+ A column that evaluates to a decimal, timestamp, or time.
Review Comment:
Same: `to_varchar` accepts binary.
```suggestion
A column that evaluates to a decimal, timestamp, time, or binary.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -803,9 +812,11 @@ def try_multiply(left: "ColumnOrName", right:
"ColumnOrName") -> Column:
Parameters
----------
left : :class:`~pyspark.sql.Column` or column name
- multiplicand
+ multiplicand.
+ A column that evaluates to a numeric or interval.
right : :class:`~pyspark.sql.Column` or column name
- multiplier
+ multiplier.
+ A column that evaluates to a numeric.
Review Comment:
Same asymmetry: `try_multiply`'s second operand also accepts an interval.
```suggestion
A column that evaluates to a numeric or interval.
```
--
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]
