Support ElementAt #2260

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

Merged

wjxiz1992 merged 24 commits into NVIDIA:branch-0.6 from wjxiz1992:elementat

May 15, 2021

Collaborator

wjxiz1992 commented Apr 25, 2021 •

edited

Loading

This PR is to close #2060. This gives initial support for element_at.

Note apache/spark#30297 introduce different behaviors for element_at when invalid index or key is given to lhs with ansi mode enabled and disabled.
A follow-up issue is created to track the difference.

For select element_at(Array(1,2,3), 4):

Spark 3.0.2 + spark.sql.ansi.enabled=true:

+-----------------------------+
|element_at(array(1, 2, 3), 4)|
+-----------------------------+
|                         null|
+-----------------------------+

Spark 3.1.1 + spark.sql.ansi.enabled=true:

java.lang.ArrayIndexOutOfBoundsException: Invalid index: 4, numElements: 3

For select element_at(map(1, 'a', 2, 'b'), 3):

Spark 3.0.2 + spark.sql.ansi.enabled=true:

+------------------------------+
|element_at(map(1, a, 2, b), 3)|
+------------------------------+
|                          null|
+------------------------------+

Spark 3.1.1 + spark.sql.ansi.enabled=true:

java.util.NoSuchElementException: Key 3 does not exist.

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

wjxiz1992 requested review from firestarman and sperlingxx

April 25, 2021 13:00

wjxiz1992 self-assigned this

sameerz added the feature request label

sameerz added this to the Apr 26 - May 7 milestone

sperlingxx reviewed

View reviewed changes

integration_tests/src/main/python/array_test.py Outdated Show resolved Hide resolved

sperlingxx reviewed

View reviewed changes

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

firestarman reviewed

View reviewed changes

docs/supported_ops.md Show resolved Hide resolved

firestarman reviewed

View reviewed changes

integration_tests/src/main/python/array_test.py Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved

wjxiz1992 linked an issue

that may be closed by this pull request

[FEA] Support element_at function on GPU #2060

Closed

revans2 requested changes

View reviewed changes

docs/supported_ops.md Show resolved Hide resolved

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved

wjxiz1992 mentioned this pull request

[FEA] elementAt, getArrayItem and getMapValue have different behaviors since Spark-3.1.1 when honoring AnsiMode #2272

Closed

3 tasks

wjxiz1992 force-pushed the elementat branch from d65c2da to 1b8cea3 Compare

April 28, 2021 12:22

wjxiz1992 marked this pull request as ready for review

April 28, 2021 13:07

wjxiz1992 force-pushed the elementat branch from 65a4073 to 94a4f55 Compare

April 28, 2021 13:40

wjxiz1992 requested review from firestarman, revans2 and sperlingxx

April 29, 2021 01:33

firestarman reviewed

View reviewed changes

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

Collaborator

firestarman commented Apr 30, 2021

build

firestarman previously approved these changes

View reviewed changes

revans2 reviewed

View reviewed changes

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala Outdated Show resolved Hide resolved

sperlingxx reviewed

View reviewed changes

Collaborator

sperlingxx left a comment •

edited

Loading

I found that ElementAt is located at collectionOperations.scala instead of complexTypeExtractors.scala in Spark. Shall we move GpuElementAt into collectionOperations.scala to follow the structure of Spark SQL ?

Collaborator

revans2 commented May 5, 2021

I found that ElementAt is located at collectionOperations.scala instead of complexTypeExtractors.scala in Spark. Shall we move GpuElementAt into collectionOperations.scala to follow the structure of Spark SQL ?

For most expressions I think it is good to try and mirror where Spark has them so yes.

wjxiz1992 dismissed firestarman’s stale review via

f3f4391

May 6, 2021 08:17

wjxiz1992 added 5 commits

May 6, 2021 16:40


          Initial support for elementAt

fd3967d


          temp work saving

4e42d39


          draft element_at

6b83aad


          Support element_at

3b6425f

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


          debug for array of array

fc5f6fc

wjxiz1992 commented

View reviewed changes

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/collectionOperations.scala

    
            @@ -0,0 +1,119 @@
          
              /*

Collaborator Author

wjxiz1992 May 14, 2021

This file is moved from rapids to sql, because:

to match the path for this file in Spark
to be able to call private [sql] class like AbstractDataType, ArrayType.simpleString


          resolve comments

d4b761f

Collaborator

firestarman commented May 14, 2021

LGTM

Collaborator

firestarman commented May 14, 2021

build

Collaborator Author

wjxiz1992 commented May 14, 2021 •

edited

Loading

CI failing due to a window related PR, this PR has been reverted.
Need to wait until the new cuDF jar is ready, then build again.

Collaborator

revans2 commented May 14, 2021

build

revans2 reviewed

View reviewed changes

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated

    
              object GetArrayItemUtil {

                def evalColumnar(array: GpuColumnVector, ordinal: Scalar, dataType: DataType,

                                 zeroIndexed: Boolean): ColumnVector = {

Collaborator

revans2 May 14, 2021

I asked for a common implementation because I thought it could be combined cleanly. This is not combined cleanly. At a minimum we need some documentation to explain exactly what is happening because it is not obvious from just the API. However, I am inclined to admin that my request was a bad one because the corner cases are handled differently enough that it is not simple to combine them, and perhaps we should leave these separate.

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeExtractors.scala Outdated Show resolved Hide resolved


          revert refactor

ae6f8d0

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

Collaborator Author

wjxiz1992 commented May 14, 2021

build

wjxiz1992 requested a review from revans2

May 14, 2021 13:48


          final fix

03110ab

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

revans2 previously approved these changes

View reviewed changes


          Merge remote-tracking branch 'origin/branch-0.6' into HEAD

968682a

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

wjxiz1992 dismissed revans2’s stale review via

968682a

May 15, 2021 05:18

wjxiz1992 requested a review from firestarman

May 15, 2021 05:18

Collaborator Author

wjxiz1992 commented May 15, 2021

build

firestarman previously approved these changes

View reviewed changes


          use columnVectorFromNull

56982cd

wjxiz1992 dismissed firestarman’s stale review via

56982cd

May 15, 2021 05:52

Collaborator Author

wjxiz1992 commented May 15, 2021

build


          refine

0ce0d35

Collaborator Author

wjxiz1992 commented May 15, 2021

build

firestarman reviewed

View reviewed changes

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/collectionOperations.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/collectionOperations.scala Show resolved Hide resolved


          indentation

0f55a04

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

firestarman approved these changes

View reviewed changes

Collaborator

firestarman commented May 15, 2021

build

wjxiz1992 merged commit 247b758 into NVIDIA:branch-0.6

wjxiz1992 deleted the elementat branch

May 15, 2021 15:20

nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request


          Support ElementAt (NVIDIA#2260)

2ffb875

Support ElementAt.

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

nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request


          Support ElementAt (NVIDIA#2260)

44e2619

Support ElementAt.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request