MaxGekk commented on code in PR #56997:
URL: https://github.com/apache/spark/pull/56997#discussion_r3577341136
##########
docs/sparkr-migration-guide.md:
##########
@@ -26,6 +26,10 @@ Note that this migration guide describes the items specific
to SparkR.
Many items of SQL migration can be applied when migrating SparkR to higher
versions.
Please refer [Migration Guide: SQL, Datasets and
DataFrame](sql-migration-guide.html).
+## Upgrading from SparkR 4.0 to 4.3
+
+ - In Spark 4.3, `TimeType` support was added. But SparkR does not support
`TimeType` and creating or collecting `TimeType` column would raise an error.
Review Comment:
Two small prose fixes here: "`TimeType` column" is missing an article, and
"would raise an error" (conditional) reads inconsistently with the sibling
data-types notes, which use the present tense ("raises an error"). Pluralizing
and switching to present tense also matches the wording in
`docs/sql-ref-datatypes.md` ("Creating and collecting `TimeType` columns"):
```suggestion
- In Spark 4.3, `TimeType` support was added. But SparkR does not support
`TimeType`, and creating or collecting `TimeType` columns raises an error.
```
(Non-blocking.)
##########
docs/sql-ref-datatypes.md:
##########
@@ -49,6 +49,7 @@ Spark SQL and DataFrames support the following data types:
- `TimeType(precision)`: Represents values comprising values of fields hour,
minute and second with the number of decimal digits `precision` following the
decimal point in the seconds field, without a time-zone.
The range of values is from `00:00:00` to `23:59:59` for min precision `0`,
and to `23:59:59.999999999` for max precision `9`. The default precision is `6`.
- Note: Apache Hive has no TIME type, so `TimeType` is not supported in
Hive SerDe interop. Storing it in a Hive SerDe table (including `INSERT
OVERWRITE DIRECTORY ... STORED AS`) or passing it to a Hive UDF/UDAF/UDTF
raises an error rather than silently converting the value.
+ - Note: SparkR's JVM serializer handles only the legacy java.sql datetime
types, so `TimeType` is not supported in SparkR as it is backed by
java.time.LocalTime. Creating and collecting `TimeType` columns would raise an
error.
Review Comment:
Minor: "would raise an error" (conditional) is inconsistent with the
adjacent Hive note directly above ("...raises an error rather than silently
converting the value"), which uses the present tense. Matching it reads more
definitively:
```suggestion
- Note: SparkR's JVM serializer handles only the legacy java.sql
datetime types, so `TimeType` is not supported in SparkR as it is backed by
java.time.LocalTime. Creating and collecting `TimeType` columns raises an error.
```
(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]
