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.

Sign up for GitHub

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

Jump to bottom

Add support for isNaN and datetime related instructions in UDF compiler #593

Merged
merged 10 commits into from
Sep 5, 2020
Merged

Add support for isNaN and datetime related instructions in UDF compiler #593

merged 10 commits into from
Sep 5, 2020

Conversation

wjxiz1992
Copy link
Collaborator

Signed-off-by: Allen Xu [email protected]

@wjxiz1992 wjxiz1992 added the test Only impacts tests label Aug 20, 2020
@wjxiz1992 wjxiz1992 self-assigned this Aug 20, 2020
@tgravescs
Copy link
Collaborator

it would be nice to put in title or description the tests are for the udf compiler.

@wjxiz1992 wjxiz1992 changed the title Add unit tests for datetime related instructions Add unit tests for datetime related instructions in UDF Aug 20, 2020
@wjxiz1992 wjxiz1992 changed the title Add unit tests for datetime related instructions in UDF Add unit tests for datetime related instructions in UDF compiler Aug 20, 2020
@seanprime7 seanprime7 changed the title Add unit tests for datetime related instructions in UDF compiler Add support for isNaN and datetime related instructions in UDF compiler Aug 20, 2020
@seanprime7 seanprime7 mentioned this pull request Aug 20, 2020
Closed
@seanprime7 seanprime7 changed the title Add support for isNaN and datetime related instructions in UDF compiler WIP: Add support for isNaN and datetime related instructions in UDF compiler Aug 20, 2020
@seanprime7

This comment has been minimized.

Sign in to view
This was linked to issues Aug 24, 2020
[FEA] Scala UDF:Support for null Value operands #541
Closed
[FEA] Scala UDF: Support for Date and Time #542
Closed
@seanprime7 seanprime7 added the feature request New feature or request label Aug 24, 2020
@seanprime7

This comment has been minimized.

Sign in to view
@wjxiz1992 wjxiz1992 changed the title WIP: Add support for isNaN and datetime related instructions in UDF compiler Add support for isNaN and datetime related instructions in UDF compiler Aug 25, 2020
@wjxiz1992
Copy link
Collaborator Author

@wjxiz1992 are you planning to add more tests to this PR? If not, I think we can remove WIP and start the review.

By the way, you might want to rebase the PR before starting the review.

Rebased and open for review now.

@seanprime7 seanprime7 changed the title Add support for isNaN and datetime related instructions in UDF compiler WIP: Add support for isNaN and datetime related instructions in UDF compiler Aug 27, 2020
@seanprime7 seanprime7 marked this pull request as draft August 27, 2020 17:28
wjxiz1992 and others added 7 commits August 28, 2020 20:28
@wjxiz1992 @seanprime7
Add unit tests for datetime related instructions
8b36f36 
Signed-off-by: Allen Xu <[email protected]>
@seanprime7
Add support for isNaN
a344b52 
Signed-off-by: Sean Lee <[email protected]>
@seanprime7
Implement date and time ops.
975e681 
We support date and time ops with the following Java/Scala code

* DateTimeFormatter.ofPattern
* LocalDateTime.parse
* LocalDateTime.getYear
* LocalDateTime.getMonthValue
* LocalDateTime.getDayOfMonth
* LocalDateTime.getHour
* LocalDateTime.getMinute
* LocalDateTime.getSecond

Signed-off-by: Sean Lee <[email protected]>
@seanprime7
Support non-default formatting pattern
3a1acbf 
Also modify the tests with various patterns.

Signed-off-by: Sean Lee <[email protected]>
@seanprime7
Add date and time to compatiblity.md
8ef74c0 
Signed-off-by: Sean Lee <[email protected]>
@seanprime7
Single-line spacing between tests
29218bc 
Signed-off-by: Sean Lee <[email protected]>
@seanprime7
Remove redundant calls to udfIsCompiled
ad8385a 
Signed-off-by: Sean Lee <[email protected]>
@seanprime7
Copy link
Contributor

@wjxiz1992 I have rebased this PR since #583 has been merged.

@seanprime7 seanprime7 changed the title WIP: Add support for isNaN and datetime related instructions in UDF compiler Add support for isNaN and datetime related instructions in UDF compiler Aug 29, 2020
@seanprime7 seanprime7 marked this pull request as ready for review August 29, 2020 03:35
@seanprime7
Copy link
Contributor

cc @abellina

This PR is now ready for review.

@abellina abellina self-requested a review September 1, 2020 17:27
@seanprime7
Copy link
Contributor

cc @vinodgro

@abellina can we get this reviewed soon?

@seanprime7
Add check against timezoned format
fc8441e 
The date and time pattern needs to be known at compile time as we need
to check whether the pattern is timezone agnostic.  If it isn't, it
needs to fall back to JVM.

Signed-off-by: Sean Lee <[email protected]>
@abellina
Copy link
Collaborator

abellina commented Sep 4, 2020 •
edited
Loading

In order for the time functions implemented here to work, the time type must become a LocalDateTime. I do not see timezone issues at that point (using getHour vs Hour while setting a timestamp when I change my timezone produces identical values). So I am +1 on this.

Pending CI!

@abellina
Copy link
Collaborator

abellina commented Sep 4, 2020

build

@abellina abellina merged commit 7aa1e72 into NVIDIA:branch-0.2 Sep 5, 2020
@wjxiz1992 wjxiz1992 deleted the new-feature-tests branch December 18, 2020 03:48
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@wjxiz1992 @seanprime7 @abellina
Add support for isNaN and datetime related instructions in UDF compil…
9c8b925 
…er (NVIDIA#593)

* Add unit tests for datetime related instructions

Signed-off-by: Allen Xu <[email protected]>

* Add support for isNaN

Signed-off-by: Sean Lee <[email protected]>

* Implement date and time ops.

We support date and time ops with the following Java/Scala code

* DateTimeFormatter.ofPattern
* LocalDateTime.parse
* LocalDateTime.getYear
* LocalDateTime.getMonthValue
* LocalDateTime.getDayOfMonth
* LocalDateTime.getHour
* LocalDateTime.getMinute
* LocalDateTime.getSecond

Signed-off-by: Sean Lee <[email protected]>

* Support non-default formatting pattern

Also modify the tests with various patterns.

Signed-off-by: Sean Lee <[email protected]>

* Add date and time to compatiblity.md

Signed-off-by: Sean Lee <[email protected]>

* Single-line spacing between tests

Signed-off-by: Sean Lee <[email protected]>

* Remove redundant calls to udfIsCompiled

Signed-off-by: Sean Lee <[email protected]>

* Update udf-compiler/src/main/scala/com/nvidia/spark/udf/Instruction.scala

Co-authored-by: Alessandro Bellina <[email protected]>

* Add a test case with zoned datetime string

Signed-off-by: Sean Lee <[email protected]>

* Add check against timezoned format

The date and time pattern needs to be known at compile time as we need
to check whether the pattern is timezone agnostic.  If it isn't, it
needs to fall back to JVM.

Signed-off-by: Sean Lee <[email protected]>

Co-authored-by: Allen Xu <[email protected]>
Co-authored-by: Sean Lee <[email protected]>
Co-authored-by: Sean Lee <[email protected]>
Co-authored-by: Alessandro Bellina <[email protected]>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@wjxiz1992 @seanprime7 @abellina
Add support for isNaN and datetime related instructions in UDF compil…
cd9b705 
…er (NVIDIA#593)

* Add unit tests for datetime related instructions

Signed-off-by: Allen Xu <[email protected]>

* Add support for isNaN

Signed-off-by: Sean Lee <[email protected]>

* Implement date and time ops.

We support date and time ops with the following Java/Scala code

* DateTimeFormatter.ofPattern
* LocalDateTime.parse
* LocalDateTime.getYear
* LocalDateTime.getMonthValue
* LocalDateTime.getDayOfMonth
* LocalDateTime.getHour
* LocalDateTime.getMinute
* LocalDateTime.getSecond

Signed-off-by: Sean Lee <[email protected]>

* Support non-default formatting pattern

Also modify the tests with various patterns.

Signed-off-by: Sean Lee <[email protected]>

* Add date and time to compatiblity.md

Signed-off-by: Sean Lee <[email protected]>

* Single-line spacing between tests

Signed-off-by: Sean Lee <[email protected]>

* Remove redundant calls to udfIsCompiled

Signed-off-by: Sean Lee <[email protected]>

* Update udf-compiler/src/main/scala/com/nvidia/spark/udf/Instruction.scala

Co-authored-by: Alessandro Bellina <[email protected]>

* Add a test case with zoned datetime string

Signed-off-by: Sean Lee <[email protected]>

* Add check against timezoned format

The date and time pattern needs to be known at compile time as we need
to check whether the pattern is timezone agnostic.  If it isn't, it
needs to fall back to JVM.

Signed-off-by: Sean Lee <[email protected]>

Co-authored-by: Allen Xu <[email protected]>
Co-authored-by: Sean Lee <[email protected]>
Co-authored-by: Sean Lee <[email protected]>
Co-authored-by: Alessandro Bellina <[email protected]>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
@nvauto
Merge pull request NVIDIA#593 from NVIDIA/bot-auto-merge-branch-22.10
2edae41 
[auto-merge] bot-auto-merge-branch-22.10 to branch-22.12 [skip ci] [bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanprime7 seanprime7 seanprime7 left review comments

@abellina abellina abellina approved these changes

Assignees

@wjxiz1992 wjxiz1992

Labels
feature request New feature or request test Only impacts tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Scala UDF: Support for Date and Time [FEA] Scala UDF:Support for null Value operands
4 participants
@wjxiz1992 @tgravescs @seanprime7 @abellina