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

Update HashAggregateSuite to work with AQE #469

Merged
merged 2 commits into from
Jul 30, 2020

Conversation

nartal1
Copy link
Collaborator

@nartal1 nartal1 commented Jul 30, 2020

Signed-off-by: Niranjan Artal nartal@nvidia.com

Updated tests and verified locally. This closes #453.

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1 nartal1 added the test Only impacts tests label Jul 30, 2020
@nartal1 nartal1 added this to the Jul 20 - Jul 31 milestone Jul 30, 2020
@nartal1 nartal1 self-assigned this Jul 30, 2020
@revans2
Copy link
Collaborator

revans2 commented Jul 30, 2020

build

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

gpuPlan match {
case WholeStageCodegenExec(GpuColumnarToRowExec(plan, _)) =>
assert(plan.children.head.isInstanceOf[GpuHashAggregateExec])
assert(gpuPlan.find(_.isInstanceOf[SortAggregateExec]).isEmpty)
assert(gpuPlan.children.forall(exec => exec.isInstanceOf[GpuExec]))

case a: AdaptiveSparkPlanExec =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be good to revisit creating helper utilities for inspecting plans, that can work both with adaptive and non-adaptive plans so we don't end up with lots of duplicate code in tests. I don't think it has to be part of this PR though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @andygrove . I have created a follow-on issue for creating helper utilities #476

@nartal1
Copy link
Collaborator Author

nartal1 commented Jul 30, 2020

build

Copy link
Contributor

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM

@revans2 revans2 merged commit 70cb788 into NVIDIA:branch-0.2 Jul 30, 2020
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* Update HashAggregateSuite to work with AQE

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* Update HashAggregateSuite to work with AQE

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
pxLi pushed a commit to pxLi/spark-rapids that referenced this pull request May 12, 2022
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Only impacts tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Update HashAggregatesSuite to work with AQE
3 participants