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

Support arithmetic operators on ANSI interval types #5020

Merged
merged 13 commits into from
Apr 8, 2022
Merged

Support arithmetic operators on ANSI interval types #5020

merged 13 commits into from
Apr 8, 2022

Conversation

res-life
Copy link
Collaborator

@res-life res-life commented Mar 23, 2022 •
edited
Loading

Contributes #4151

Support arithmetic operators on ANSI interval types(year month interval and day time interval)

  • Abs: abs(interval)
  • UnaryMinus: -interval
  • Add: interval + interval
  • Subtract: interval - interval
  • UnaryPositive: +interval

Signed-off-by: Chong Gao res_life@163.com

Support arithmetic operators on ANSI interval types
5621197 
Signed-off-by: Chong Gao <res_life@163.com>
@res-life
Copy link
Collaborator Author

build

@res-life
Copy link
Collaborator Author

res-life commented Mar 23, 2022 •
edited
Loading

TODO:

  • add test case for year-month type [done]
  • support multiple operator (interval * number) will move to another PR
  • support divide operator (interval / number) will move to another PR

@sameerz sameerz added the audit_3.3.0 Audit related tasks for 3.3.0 label Mar 23, 2022
@sameerz sameerz added this to the Mar 21 - Apr 1 milestone Mar 23, 2022
@sameerz
Copy link
Collaborator

sameerz commented Mar 23, 2022

@res-life please target to branch-22.06.

@res-life res-life changed the base branch from branch-22.04 to branch-22.06 March 24, 2022 01:32
@res-life
Copy link
Collaborator Author

build

Support arithmetic operators on year month interval types
70af6a0 
Signed-off-by: Chong Gao <res_life@163.com>
@res-life
Copy link
Collaborator Author

build

@res-life res-life marked this pull request as ready for review March 25, 2022 06:44
firestarman
firestarman reviewed Mar 25, 2022
View reviewed changes
integration_tests/src/main/python/arithmetic_ops_test.py Outdated Show resolved Hide resolved
integration_tests/src/main/python/arithmetic_ops_test.py Outdated Show resolved Hide resolved
integration_tests/src/main/python/arithmetic_ops_test.py Outdated Show resolved Hide resolved
integration_tests/src/main/python/arithmetic_ops_test.py Outdated Show resolved Hide resolved
sql-plugin/src/main/330+/scala/com/nvidia/spark/rapids/shims/Spark33XShims.scala Show resolved Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/arithmetic.scala Show resolved Hide resolved
sql-plugin/src/main/311until330-all/scala/com/nvidia/spark/rapids/shims/GpuTypeShims.scala Outdated Show resolved Hide resolved
sql-plugin/src/main/330+/scala/com/nvidia/spark/rapids/shims/GpuTypeShims.scala Outdated Show resolved Hide resolved
sql-plugin/src/main/330+/scala/com/nvidia/spark/rapids/shims/GpuTypeShims.scala Outdated Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/TypeChecks.scala Outdated Show resolved Hide resolved
Chong Gao added 2 commits March 31, 2022 17:13
Refactor
5082292
Merge branch 22.06
f08163f 
Signed-off-by: Chong Gao <res_life@163.com>
@res-life
Copy link
Collaborator Author

build

@res-life
Copy link
Collaborator Author

build

@firestarman
Copy link
Collaborator

LGTM

@res-life res-life requested review from revans2 and jlowe April 1, 2022 11:59
@res-life
Copy link
Collaborator Author

res-life commented Apr 1, 2022

@revans2 @jlowe help review.

Refactor
8f72074 
Signed-off-by: Chong Gao <res_life@163.com>
@sameerz sameerz modified the milestones: Mar 21 - Apr 1, Apr 4 - Apr 15 Apr 4, 2022
@jlowe jlowe mentioned this pull request Apr 4, 2022
Merged
@res-life res-life mentioned this pull request Apr 6, 2022
Closed
@res-life
Copy link
Collaborator Author

res-life commented Apr 6, 2022

CI failed due to: #5147

@res-life
Copy link
Collaborator Author

res-life commented Apr 6, 2022 •
edited
Loading

Spark supports reading/writing dataframes with ANSI intervals from/to parquet files from 330
https://issues.apache.org/jira/browse/SPARK-36825

And Pyspark supports DayTimeIntervalType from 330
https://issues.apache.org/jira/browse/SPARK-37279

Although these operators are from Spark320, we add GPU support into 330 for Pyspark test convenient purpose, is this ok?

BTW: Pyspark does not support year-month interval currently.

@jlowe
Copy link
Member

jlowe commented Apr 6, 2022

Although these operators are from Spark320, we add GPU support into 330 for Pyspark test convenient purpose, is this ok?

It's OK as long as the plugin consistently handles these types in Spark 3.2. We don't have to support a type Spark supports as long as we need to fallback to the CPU properly in those cases. However I do find it a little weird that we have all the necessary code to support interval types in the plugin but chose not to for Spark 3.2. Not having a nice way to test it helps explain the context for why we support interval types in 3.3 but not in 3.2. Thanks!

@res-life
Copy link
Collaborator Author

res-life commented Apr 7, 2022

build

@res-life
Copy link
Collaborator Author

res-life commented Apr 8, 2022

build

@res-life res-life mentioned this pull request Apr 8, 2022
Merged
@res-life
Copy link
Collaborator Author

res-life commented Apr 8, 2022 •
edited
Loading

Check Markdown links failed:

FILE: docs/spark-qualification-tool.md
[:heavy_multiplication_x:] https://repo1.maven.org/maven2/com/nvidia/rapids-4-spark-tools_2.12/22.04.0/ → Status: 404

@jlowe
Copy link
Member

jlowe commented Apr 8, 2022

Markdown link failure is a known issue with downloads page updated to the not-quite-yet-released 22.04 artifacts.

@jlowe jlowe merged commit 09da145 into NVIDIA:branch-22.06 Apr 8, 2022
@res-life res-life deleted the interval-arithmetic branch April 8, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@firestarman firestarman firestarman left review comments

@jlowe jlowe jlowe approved these changes

@revans2 revans2 Awaiting requested review from revans2

Assignees
No one assigned
Labels
audit_3.3.0 Audit related tasks for 3.3.0
Projects
None yet
Milestone
Apr 4 - Apr 15
Development

Successfully merging this pull request may close these issues.
4 participants
@res-life @sameerz @firestarman @jlowe