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

support interval.microseconds for range window TimeStampType #2525

Merged
merged 3 commits into from
Jun 1, 2021

Conversation

wbo4958
Copy link
Collaborator

@wbo4958 wbo4958 commented May 27, 2021

  1. This change supports interval.microseconds for TimeStampType.

2.fix a range window on DateType bug.

When the order-by column type is DateType, the value bounded type
should be IntegerType instead of CalendarIntervalType.
In other words, Interval is not working when order-by column is DateType.

Signed-off-by: Bobby Wang wbo4958@gmail.com

1. This change supports interval.microseconds for TimeStampType.

2.fix a range window on DateType bug.

  When the order-by column type is DateType, the value bounded type
  should be IntegerType instead of CalendarIntervalType.
  In other words, Interval is not working when order-by column is DateType.

Signed-off-by: Bobby Wang <wbo4958@gmail.com>
@wbo4958
Copy link
Collaborator Author

wbo4958 commented May 27, 2021

build

@sameerz sameerz added the feature request New feature or request label May 27, 2021
@wbo4958
Copy link
Collaborator Author

wbo4958 commented May 27, 2021

build

@wbo4958 wbo4958 changed the title support interval.microseconds for TimeStampType support range window interval.microseconds for TimeStampType May 28, 2021
@wbo4958 wbo4958 changed the title support range window interval.microseconds for TimeStampType support interval.microseconds for TimeStampType range window May 28, 2021
@wbo4958 wbo4958 requested a review from revans2 May 28, 2021 02:17
@wbo4958 wbo4958 changed the title support interval.microseconds for TimeStampType range window support interval.microseconds for range window TimeStampType May 28, 2021
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit

ci.microseconds)
} catch {
case e: ArithmeticException =>
willNotWorkOnGpu("interval total microseconds has overflow")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see a better message. It is not clear how the user can fix this or what exactly has happened.

Perhaps something more like

willNotWorkOnGpu(s"windows over timestamps are converted to microseconds and $ci is too large to fit")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx @revans2. Done

@wbo4958
Copy link
Collaborator Author

wbo4958 commented May 28, 2021

build

@revans2 revans2 merged commit c1ac13f into NVIDIA:branch-21.06 Jun 1, 2021
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@wbo4958 wbo4958 deleted the interval-microseconds branch June 15, 2021 02:23
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
Development

Successfully merging this pull request may close these issues.

3 participants