support GpuConcat on ArrayType #2379

sperlingxx · 2021-05-10T10:08:22Z

Current PR is to support GpuConcat on ArrayType, which is required in issue #2013.
In addition, this PR also introduces some refinement on the implementation of string concatenation.

Signed-off-by: sperlingxx <[email protected]>

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala

jlowe · 2021-05-10T23:16:24Z

build

Signed-off-by: sperlingxx <[email protected]>

sperlingxx · 2021-05-11T09:29:08Z

I found another small problem of cuDF implementation during running tests on Array of String locally. And I've put up a PR to fix it.

jlowe · 2021-05-11T14:54:38Z

build

sperlingxx · 2021-05-13T01:45:41Z

build

gerashegalov

LGTM

gerashegalov · 2021-05-13T02:06:55Z

sql-plugin/src/main/scala/com/nvidia/spark/rapids/collectionOperations.scala

+  private def stringConcat(batch: ColumnarBatch): GpuColumnVector = {
+    val rows = batch.numRows()
+
+    withResource(ArrayBuffer[ColumnVector]()) { buffer =>


nit: here and later: ArrayBuffer.empty[ColumnVector] for readability.

Signed-off-by: sperlingxx <[email protected]>

sperlingxx · 2021-05-13T03:15:56Z

build

sql-plugin/src/main/scala/com/nvidia/spark/rapids/collectionOperations.scala

…erations.scala Co-authored-by: Jason Lowe <[email protected]>

sperlingxx · 2021-05-13T14:25:03Z

build

…erations.scala Co-authored-by: Jason Lowe <[email protected]>

jlowe · 2021-05-13T15:01:57Z

build

revans2

Mostly nits but there are some corner cases that string concat missed and that this also is missing.

I tested the string concat code and it works if we have asserts disabled. If we enable them it fails. I don't know about list concat. There is probably also an issue with no columns being passed in. I am fine with letting this slide for now because you have to go out of your way to disable expression folding for it to be an issue, but it might be good to just fall back to the CPU if there are no children. That way we don't have to ever worry about it.

sql-plugin/src/main/scala/com/nvidia/spark/rapids/collectionOperations.scala

Signed-off-by: sperlingxx <[email protected]>

sperlingxx · 2021-05-17T07:59:36Z

build

sperlingxx · 2021-05-17T08:08:59Z

Still would like to see us prevent against the empty concat when folding is disabled, but it is such a rare corner case it is not important enough to hold this up over it.

I added missing support of empty concat. And I also found that cuDF concat can be simply bypassed in single column concat.

The base branch was changed.

integration_tests/src/main/python/collection_ops_test.py

sperlingxx · 2021-05-19T03:08:30Z

build

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

integration_tests/src/main/python/collection_ops_test.py

Signed-off-by: sperlingxx <[email protected]>

sperlingxx · 2021-05-20T09:04:16Z

build

revans2

Looks good

tgravescs · 2021-05-20T12:53:12Z

integration_tests/src/main/python/collection_ops_test.py

+                f.concat(s1, f.col('b')),
+                f.concat(f.col('a'), s2),
+                f.concat(f.lit(None).cast('string'), f.col('b')),
+                f.concat(f.col('a'), f.lit(None).cast('string')),


do we have an all nulls test case? I guess this line might hit it if col a can generate nulls.

similar for arrays, do we want an all nulls test case for arrays

To be frank, I am not sure whether there exists rows consisting of all null fields among test data for concat_array , with default random seed.

But I think the all null case may be not that special, since the output row will be null only if one input field is null.

Closes NVIDIA#2013 Support GpuConcat on ArrayType. And introduce some refinement for GpuConcat on StringType. Signed-off-by: sperlingxx <[email protected]>

support concat Arrays

34cd85a

Signed-off-by: sperlingxx <[email protected]>

jlowe reviewed May 10, 2021

View reviewed changes

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

update auto-generated docs

8763aae

Signed-off-by: sperlingxx <[email protected]>

jlowe previously approved these changes May 11, 2021

View reviewed changes

sameerz linked an issue May 11, 2021 that may be closed by this pull request

[FEA] Support concatenating ArrayType columns #2013

Closed

sameerz added the feature request New feature or request label May 12, 2021

sameerz added this to the May 10 - May 21 milestone May 12, 2021

gerashegalov previously approved these changes May 13, 2021

View reviewed changes

fix

52bfccd

Signed-off-by: sperlingxx <[email protected]>

sperlingxx dismissed stale reviews from gerashegalov and jlowe via 52bfccd May 13, 2021 03:14

jlowe reviewed May 13, 2021

View reviewed changes

sperlingxx and others added 2 commits May 13, 2021 22:23

Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/collectionOp…

fb74f1d

…erations.scala Co-authored-by: Jason Lowe <[email protected]>

Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/collectionOp…

bee46ad

…erations.scala Co-authored-by: Jason Lowe <[email protected]>

Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/collectionOp…

b6a3853

…erations.scala Co-authored-by: Jason Lowe <[email protected]>

jlowe previously approved these changes May 13, 2021

View reviewed changes

revans2 requested changes May 13, 2021

View reviewed changes

revans2 reviewed May 13, 2021

View reviewed changes

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

sperlingxx added 2 commits May 17, 2021 11:02

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

0d76e0c

refine && merge

ce02997

Signed-off-by: sperlingxx <[email protected]>

sperlingxx dismissed revans2’s stale review via ce02997 May 17, 2021 07:30

sperlingxx requested a review from revans2 May 18, 2021 01:03

jlowe previously approved these changes May 18, 2021

View reviewed changes

pxLi changed the base branch from branch-0.6 to branch-21.06 May 19, 2021 01:11

wjxiz1992 reviewed May 19, 2021

View reviewed changes

integration_tests/src/main/python/collection_ops_test.py Show resolved Hide resolved

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

14493e5

sperlingxx requested a review from jlowe May 19, 2021 03:13

revans2 reviewed May 19, 2021

View reviewed changes

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

revans2 reviewed May 19, 2021

View reviewed changes

integration_tests/src/main/python/collection_ops_test.py Show resolved Hide resolved

support GpuScalar

37cccf1

Signed-off-by: sperlingxx <[email protected]>

sperlingxx requested a review from revans2 May 20, 2021 09:57

revans2 approved these changes May 20, 2021

View reviewed changes

tgravescs reviewed May 20, 2021

View reviewed changes

sperlingxx merged commit c694fdc into NVIDIA:branch-21.06 May 20, 2021

sperlingxx deleted the concat_array branch December 2, 2021 02:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support GpuConcat on ArrayType #2379

support GpuConcat on ArrayType #2379

sperlingxx commented May 10, 2021 •

edited

Loading

jlowe commented May 10, 2021

sperlingxx commented May 11, 2021

jlowe commented May 11, 2021

sperlingxx commented May 13, 2021

gerashegalov left a comment

gerashegalov May 13, 2021

sperlingxx May 13, 2021

sperlingxx commented May 13, 2021

sperlingxx commented May 13, 2021

jlowe commented May 13, 2021

revans2 left a comment

sperlingxx commented May 17, 2021

sperlingxx commented May 17, 2021

sperlingxx commented May 19, 2021

sperlingxx commented May 20, 2021

revans2 left a comment

tgravescs May 20, 2021

tgravescs May 20, 2021

sperlingxx May 20, 2021 •

edited

Loading

sperlingxx May 20, 2021

support GpuConcat on ArrayType #2379

support GpuConcat on ArrayType #2379

Conversation

sperlingxx commented May 10, 2021 • edited Loading

jlowe commented May 10, 2021

sperlingxx commented May 11, 2021

jlowe commented May 11, 2021

sperlingxx commented May 13, 2021

gerashegalov left a comment

Choose a reason for hiding this comment

gerashegalov May 13, 2021

Choose a reason for hiding this comment

sperlingxx May 13, 2021

Choose a reason for hiding this comment

sperlingxx commented May 13, 2021

sperlingxx commented May 13, 2021

jlowe commented May 13, 2021

revans2 left a comment

Choose a reason for hiding this comment

sperlingxx commented May 17, 2021

sperlingxx commented May 17, 2021

sperlingxx commented May 19, 2021

sperlingxx commented May 20, 2021

revans2 left a comment

Choose a reason for hiding this comment

tgravescs May 20, 2021

Choose a reason for hiding this comment

tgravescs May 20, 2021

Choose a reason for hiding this comment

sperlingxx May 20, 2021 • edited Loading

Choose a reason for hiding this comment

sperlingxx May 20, 2021

Choose a reason for hiding this comment

sperlingxx commented May 10, 2021 •

edited

Loading

sperlingxx May 20, 2021 •

edited

Loading