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 MapType in selected operators #984

Merged

kuhushukla merged 20 commits into NVIDIA:branch-0.3 from kuhushukla:getMapValue

Oct 23, 2020

Collaborator

kuhushukla commented Oct 20, 2020

Adds support for getMapValue and whitelists maps wherever required per some targeted queries. Also swicthes Builder to ColumnBuilder.
Needs a ton of testing but this is what I have so far. Very much a WIP.

Need to add tests for - filter and project with operators we now support for maptype and negative test for a couple others as sanity. Also verify the offset and length params to ColumnarMap make sense, initial tests seems to give the right answers but needs more testing. I could not get away with not adding the supported data type whitelisting in filter and project exec. Open to alternative approaches there.

Also for now I have disabled filter to use coalesce, to show how this works for certain targeted queries we want to support. I can modify the approach we take on that as part of this change based on what people think.


          Add support for getMapValue and MapType for certain operators

a43554e

Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>

kuhushukla added the feature request label

kuhushukla added this to the Oct 12 - Oct 23 milestone

kuhushukla self-assigned this

revans2 reviewed

View reviewed changes

Collaborator

revans2 left a comment

I know this is still WIP, but I wanted to make sure that we were going in the right direction.

shims/spark300/src/main/scala/com/nvidia/spark/rapids/shims/spark300/Spark300Shims.scala Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVector.java Outdated Show resolved Hide resolved

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

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

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

kuhushukla added 9 commits

October 20, 2020 16:15


          Add another from for CB and remove getSparkTypeFrom


          has null can be false

d9b31d0


          Fix fields datatype check and add checkInputDataTypes

cab876b


          Add RapidsHostColumnvectorCore

3b385bd


          Merge remote-tracking branch 'refs/remotes/origin-github/branch-0.3' …

e5d55dc

…into getMapValue_main


          Remove debug method

6cfbcd4


          Remove dup code

f340a54


          Remove dup code fixup

b2fbfb3


          clean up

d20b8f8

kuhushukla commented

View reviewed changes

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Outdated Show resolved Hide resolved

kuhushukla commented

View reviewed changes

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVector.java Show resolved Hide resolved

revans2 reviewed

View reviewed changes

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVector.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVector.java Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVectorCore.java Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVectorCore.java Outdated Show resolved Hide resolved

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


          Fix offset and length for ColmunarMap

ed0f191

kuhushukla commented

View reviewed changes

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVector.java Show resolved Hide resolved

kuhushukla added 3 commits

October 21, 2020 13:03


          Cast to hcv core


          Add coalesce iter for maps

ef57f63


          Fix scal style errors and add comment on the new coalesce iter

kuhushukla commented

View reviewed changes

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Outdated Show resolved Hide resolved


          Minor fixups

6b99acd

revans2 previously approved these changes

View reviewed changes

Collaborator

revans2 left a comment

Looks good, I have one nit that would be nice to fix here and several follow on issues that we should file.

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/GpuColumnVector.java Outdated Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVector.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVector.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVectorCore.java Show resolved Hide resolved

sql-plugin/src/main/java/com/nvidia/spark/rapids/RapidsHostColumnVectorCore.java Show resolved Hide resolved


          Fixup

2d513cd

kuhushukla dismissed revans2’s stale review via

2d513cd

October 22, 2020 18:47


          Remove debug

5cc3f48

kuhushukla changed the title ~~[WIP] Add support for MapType in selected operators~~ [REVIEW] Add support for MapType in selected operators

Collaborator Author

kuhushukla commented Oct 22, 2020

build

kuhushukla added the P0 label


          Add back getSparkTypeFrom

82decbb

Collaborator Author

kuhushukla commented Oct 22, 2020

I have pushed a bug fix for python failures seen in CI.

Collaborator Author

kuhushukla commented Oct 22, 2020

build

Collaborator Author

kuhushukla commented Oct 23, 2020

Seeing a bunch of NoSuchMethod found errors :

 Cause: java.lang.NoSuchMethodError: org.apache.spark.sql.execution.datasources.BasicWriteTaskStats.<init>(IIJJ)V�[0m
17:50:27  �[31m  at org.apache.spark.sql.rapids.BasicColumnarWriteTaskStatsTracker.getFinalStats(BasicColumnarWriteStatsTracker.scala:107)�[0m
17:50:27  �[31m  at org.apache.spark.sql.rapids.GpuFileFormatDataWriter.$anonfun$commit$1(GpuFileFormatDataWriter.scala:82)�[0m
17:50:27  �[31m  at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:237)�[0m
17:50:27  �[31m  at scala.collection.immutable.List.foreach(List.scala:392)�[0m
17:50:27  �[31m  at scala.collection.TraversableLike.map(TraversableLike.scala:237)�[0m
17:50:27  �[31m  at scala.collection.TraversableLike.map$(TraversableLike.scala:230)�[0m
17:50:27  �[31m  at scala.collection.immutable.List.map(List.scala:298)�[0m
17:50:27  �[31m  at org.apache.spark.sql.rapids.GpuFileFormatDataWriter.commit(GpuFileFormatDataWriter.scala:82)�[0m
17:50:27  �[31m  at org.apache.spark.sql.rapids.GpuFileFormatWriter$.$anonfun$executeTask$1(GpuFileFormatWriter.scala:298)�[0m
17:50:27  �[31m  at org.apache.spark.util.Utils$.tryWithSafeFinallyAndFailureCallbacks(Utils.scala:1460)�[0m

this seems related to #1009

Collaborator Author

kuhushukla commented Oct 23, 2020

I will try again once #1009 is in.

revans2 previously approved these changes

View reviewed changes

Collaborator

revans2 left a comment

Looks good to me. Just need to get the build to pass.

kuhushukla mentioned this pull request

Change Hash Aggregate to allow pass-through on MapType #1016

Merged


          Merge remote-tracking branch 'refs/remotes/origin-github/branch-0.3' …

b966ffb

…into getMapValue_main

Collaborator Author

kuhushukla commented Oct 23, 2020

build

Collaborator Author

kuhushukla commented Oct 23, 2020

Oh well I was bit by an intermittent failure in test_broadcast_nested_loop_join_special_case. Rerunning...

Collaborator Author

kuhushukla commented Oct 23, 2020

build


          Add isSupported for maps to 310 shim

81a4111

Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>

kuhushukla dismissed revans2’s stale review via

81a4111

October 23, 2020 20:51

Collaborator Author

kuhushukla commented Oct 23, 2020

build

revans2 approved these changes

View reviewed changes

Collaborator Author

kuhushukla commented Oct 23, 2020

I'm going to merge this now.

kuhushukla merged commit 27f6a0a into NVIDIA:branch-0.3

jlowe changed the title ~~[REVIEW] Add support for MapType in selected operators~~ Add support for MapType in selected operators

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


          Add support for MapType in selected operators (NVIDIA#984)

51c7f3c

* Add support for getMapValue and MapType for certain operators
Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>

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


          Add support for MapType in selected operators (NVIDIA#984)

c8c041f

* Add support for getMapValue and MapType for certain operators
Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>

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


          Add support for MapType in selected operators (NVIDIA#984)

0b7061e

* Add support for getMapValue and MapType for certain operators
Signed-off-by: Kuhu Shukla <kuhus@nvidia.com>

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


          Fix issue where SparkRapidsAdaptor could be shut down but not through…

7ff429b

… RmmSpark (NVIDIA#984)

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>

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

Labels

feature request P0