MaxGekk commented on code in PR #56205:
URL: https://github.com/apache/spark/pull/56205#discussion_r3335785179
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala:
##########
Review Comment:
Thanks @uros-db. I'd lean against adding `private[sql]` here, because these
methods already live in an internal package, so the marker would be redundant
and inconsistent with the surrounding code.
- `SparkDateTimeUtils` (and `DateTimeUtils`) sit in
`org.apache.spark.sql.catalyst.util`, and the whole
`org.apache.spark.sql.catalyst` namespace is treated as non-public by Spark's
own build. From `project/SparkBuild.scala`:
```scala
// Skip actual catalyst, but include the subproject.
// Catalyst is not public API and contains quasiquotes which break
scaladoc.
```
and the unidoc source filter explicitly drops it:
```scala
.map(_.filterNot(_.getCanonicalPath.contains("org/apache/spark/sql/catalyst")))
```
So nothing under `catalyst` is published as API docs or treated as a
stable contract, regardless of the `def` visibility modifier (and note
`private[sql]` wouldn't actually narrow much, since `catalyst.util` is already
under `org.apache.spark.sql`).
- Every sibling in this same trait/object is a plain public `def` —
`parseTimestampString`, `stringToTimestamp`,
`stringToTimestampWithoutTimeZone`, `stringToTimestampAnsi`, etc. — and so are
the SPARK-57033 helpers this PR delegates to (`instantToTimestampNanos`,
`localDateTimeToTimestampNanos`, `truncateNanosWithinMicroToPrecision`).
Singling out only the new nanos entry points with `private[sql]` would break
that consistency without buying real protection.
You're right that "package-private" in the PR description is imprecise
wording, though — I'll reword it to "internal `catalyst.util` APIs (not public
API)" so reviewers aren't misled.
--
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]
