Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEA] Convert Timestamp/Timezone tests/checks to be per operator instead of generic #6832

Closed
revans2 opened this issue Oct 18, 2022 · 0 comments · Fixed by #9719
Closed

[FEA] Convert Timestamp/Timezone tests/checks to be per operator instead of generic #6832

revans2 opened this issue Oct 18, 2022 · 0 comments · Fixed by #9719
Assignees
Labels
feature request New feature or request

Comments

@revans2
Copy link
Collaborator

revans2 commented Oct 18, 2022

Is your feature request related to a problem? Please describe.
Currently we have checks for time zones in a few places that disable timestamp support if the timezone is not what we want.

case TimestampType =>
TypeChecks.areTimestampsSupported(ZoneId.systemDefault()) &&
TypeChecks.areTimestampsSupported(SQLConf.get.sessionLocalTimeZone)

def checkTimeZoneId(sessionZoneId: ZoneId): Unit = {
// Both of the Spark session time zone and JVM's default time zone should be UTC.
if (!TypeChecks.areTimestampsSupported(sessionZoneId)) {
willNotWorkOnGpu("Only UTC zone id is supported. " +
s"Actual session local zone id: $sessionZoneId")
}
val defaultZoneId = ZoneId.systemDefault()
if (!TypeChecks.areTimestampsSupported(defaultZoneId)) {
willNotWorkOnGpu(s"Only UTC zone id is supported. Actual default zone id: $defaultZoneId")
}
}

In preparation to be able to support more time zones and doing it on a per operator basis we need to move these checks from the generic places that they are now, to individual expressions and operators.

This is going to be a lot of refactoring, but hopefully it will not be too difficult. The hard part will be testing this everywhere. Some operators that do not actually process the data, only send it along, will probably be allowed to work with timestamps.

We should pay special attention to Cast, because it does need a timezone, but only for a subset of the casts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
3 participants