MaxGekk commented on code in PR #57183:
URL: https://github.com/apache/spark/pull/57183#discussion_r3577268124
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -1089,6 +1153,11 @@ case class UnixNanos(child: Expression)
// scalastyle:off line.contains.tab
@ExpressionDescription(
usage = "_FUNC_(date) - Returns the year component of the date/timestamp.",
+ arguments = """
+ Arguments:
+ * date - The date or timestamp to extract the year from.
Review Comment:
The description says "The date **or timestamp** to extract the year from",
but the type line just below reads "An expression that evaluates to a date." —
narrower than what's accepted. `GetDateField` accepts a timestamp via implicit
cast; I confirmed `year`/`month`/`quarter`/`dayofmonth` (Column API) and
`year`/`dayofyear`/`weekofyear` (SQL) all evaluate on a `TIMESTAMP` argument.
So the type phrase should match the prose:
```suggestion
* date - The date or timestamp to extract the year from.
An expression that evaluates to a date or timestamp.
```
The same narrowing is in the other `GetDateField` extract functions
(`quarter`, `month`, `dayofmonth`, `dayofweek`/`weekday`, `dayofyear`) —
`hour`/`minute`/`second` already say "timestamp", so this aligns them.
(Non-blocking.)
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Between.scala:
##########
@@ -30,7 +30,11 @@ import org.apache.spark.sql.internal.SQLConf
arguments = """
Arguments:
* input - An expression that is being compared with lower and upper
bound.
+ An expression that evaluates to a boolean, numeric, string, binary,
date, timestamp, time,
+ interval, array, or struct.
* lower - Lower bound of the between check.
+ An expression that evaluates to a boolean, numeric, string, binary,
date, timestamp, time,
+ interval, array, or struct.
* upper - Upper bound of the between check.
Review Comment:
`upper` gets a description but no type line, while its siblings `input` and
`lower` each got "An expression that evaluates to a boolean, numeric, string,
..." — and all three must be the same comparable type. Adding the matching type
line keeps the block internally consistent:
```suggestion
* upper - Upper bound of the between check.
An expression that evaluates to a boolean, numeric, string, binary,
date, timestamp, time,
interval, array, or struct.
```
The same one-arg-missing-its-type-line pattern appears in `arrays_zip` (`a2`
at `collectionOperations.scala:296`, while `a1` is typed) and the tuple-sketch
aggregates (`lgNomEntries`/`mode` untyped under typed `key`/`summary`; the
`lgNomEntries` continuation line is also under-indented). Worth a sweep for
consistency — or a brief note where a type line is intentionally omitted for a
constant/variadic arg. (Non-blocking.)
--
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]
