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.

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

Add in array_join support #11420

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Add in array_join support #11420

merged 1 commit into from
Sep 4, 2024

Conversation

revans2
Copy link
Collaborator

@revans2 revans2 commented Sep 3, 2024

This adds in support for the array_join expression.

This closes #5223

But I will file a follow on issue to deal with rapidsai/cudf#12766 when it is fixed.

Note that this does not handle a null replacement that is not a Scalar. I can add it in without too much work. It is just that I have to special case that even more because there are no CUDF APIs to handle it.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2
Copy link
Collaborator Author

revans2 commented Sep 3, 2024

build

case class GpuArrayJoin(override val children : Seq[Expression])
extends GpuExpression with ShimExpression {

private val array = children(0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Typically do not use universal keyword as variable name. Instead of array we can use input or array_data or something similar.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just following exactly what Spark did for the naming.

https://github.com/apache/spark/blob/c5293ecb017b55ff661ea05353e4463a08d0073c/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L2121

array is technically not a keyword in scala or the code would not compile. It is not even the name of a class in scala. That is Array. But I am really nit picking at this point.

I did it really to match what Spark was doing for clarity.

@revans2 revans2 merged commit d2abcd9 into NVIDIA:branch-24.10 Sep 4, 2024
46 checks passed
@sameerz sameerz added the feature request New feature or request label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Support array_join
4 participants