MaxGekk commented on a change in pull request #28856:
URL: https://github.com/apache/spark/pull/28856#discussion_r442366706
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -2623,8 +2623,16 @@ object Sequence {
// about a month length in days and a day length in microseconds
val intervalStepInMicros =
Review comment:
The current implementation is strange mix, it seems. There are following
options:
1. Step is an interval of (months, days, micros):
1. If start point is TimestampType, we should convert it to local
date-time in the session time zone, and add the interval by time components.
The intermediate local timestamps should be converted back to micros using the
session time zone but we should keep adding the interval to local timestamp
"accumulator".
2. The same for dates - convert `start` to a local date. Time zone
shouldn't involved here.
2. If the step is a duration in micros or days (this is not our case)
1. start is TimestampType, we shouldn't convert it to local timestamp,
and just add micros to instants. So, time zone will be not involved here.
2. start is DateType, just add number of days. The same as for
timestamps, time zone is not involved here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
