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.

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

Fix unit tests when AQE is enabled #558

Merged
merged 7 commits into from
Aug 18, 2020
Merged

Fix unit tests when AQE is enabled #558

merged 7 commits into from
Aug 18, 2020

Conversation

nartal1
Copy link
Collaborator

@nartal1 nartal1 commented Aug 14, 2020

This fixes #452 and also BroadcastHashJoinSuite when AQE is enabled.

nartal1 added 3 commits August 13, 2020 15:14
@nartal1
Fix scala tests when AQE is enabled
b68daae 
Signed-off-by: Niranjan Artal <[email protected]>
@nartal1
fix broadcasthashjoin tests
2e86fa0 
Signed-off-by: Niranjan Artal <[email protected]>
@nartal1
fix indentation
853ce84 
Signed-off-by: Niranjan Artal <[email protected]>
@nartal1 nartal1 added the test Only impacts tests label Aug 14, 2020
@nartal1 nartal1 self-assigned this Aug 14, 2020
@nartal1 nartal1 changed the title [REVIEW]Fix unit tests when AQE is enabled Fix unit tests when AQE is enabled Aug 14, 2020
@nartal1
Copy link
Collaborator Author

nartal1 commented Aug 14, 2020

build

@sameerz sameerz added this to the Aug 17 - Aug 28 milestone Aug 17, 2020
@nartal1
addressed review comments
4dff1f1 
Signed-off-by: Niranjan Artal <[email protected]>
@nartal1
Copy link
Collaborator Author

nartal1 commented Aug 18, 2020

Thanks @andygrove for reviewing. I think I have addressed all the review comments. Please take another look.

andygrove
andygrove reviewed Aug 18, 2020
View reviewed changes
tests/src/test/scala/com/nvidia/spark/rapids/BroadcastHashJoinSuite.scala Outdated
"GpuBuildLeft")
assert(ShimLoader.getSparkShims.getBuildSide(res2.get.asInstanceOf[HashJoin]).toString ==
"GpuBuildRight")
val initialPlan2 = plan2.queryExecution.executedPlan
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here re removing intermediate variable

andygrove
andygrove reviewed Aug 18, 2020
View reviewed changes
tests/src/test/scala/com/nvidia/spark/rapids/BroadcastHashJoinSuite.scala Outdated
@@ -35,13 +37,14 @@ class BroadcastHashJoinSuite extends SparkQueryCompareTestSuite {
val df5 = df4.join(df3, Seq("longs"), "inner")

val plan = df5.queryExecution.executedPlan
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, just one more nit. It would be better to get the reference to the executedPlan after calling collect rather than before. The goal is to get the final plan, so it would be clearer in this order. It isn't obvious to people looking this code that the executedPlan gets updated by the call to collect. Other than this, LGTM.

@nartal1
addressed review comments
e169f4d 
Signed-off-by: Niranjan Artal <[email protected]>
@nartal1
Copy link
Collaborator Author

nartal1 commented Aug 18, 2020

build

@nartal1 nartal1 merged commit a4e86f6 into NVIDIA:branch-0.2 Aug 18, 2020
pxLi added a commit to pxLi/spark-rapids that referenced this pull request Aug 24, 2020
@pxLi @andygrove
@tgravescs @nartal1 @jlowe @revans2
Local changelog test (#32)
b609d99 
* explicitly disable AQE in one test (NVIDIA#567)

Signed-off-by: Andy Grove <[email protected]>

* Enable using spark-submit to convert from csv to parquet (NVIDIA#565)

Signed-off-by: Andy Grove <[email protected]>

* xfail the Spark 3.1.0 integration tests that fail  (NVIDIA#580)

* xfail GpuTimeSub, arithmetic ops, and full outer join failures on 3.1.0

Signed-off-by: Thomas Graves <[email protected]>

* xfail the rest of the 3.1.0 tests and enable 3.1.0 unit tests in the
jenkins builds

Signed-off-by: Thomas Graves <[email protected]>

Co-authored-by: Thomas Graves <[email protected]>

* Fix unit tests when AQE is enabled (NVIDIA#558)

* Fix scala tests when AQE is enabled

Signed-off-by: Niranjan Artal <[email protected]>

* fix broadcasthashjoin tests

Signed-off-by: Niranjan Artal <[email protected]>

* fix indentation

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* Update buffer store to return compressed batches directly, add compression NVTX ranges (NVIDIA#572)

* Update buffer store to return compressed batches directly, add compression NVTX ranges

Signed-off-by: Jason Lowe <[email protected]>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <[email protected]>

* xfail the tpch spark 3.1.0 tests that fail (NVIDIA#588)

Signed-off-by: Thomas Graves <[email protected]>

Co-authored-by: Thomas Graves <[email protected]>

* Move GpuParquetScan/GpuOrcScan into Shim (NVIDIA#590)

* Move GpuParquetScan to shim

Signed-off-by: Thomas Graves <[email protected]>

* Move scan overrides into shim

Signed-off-by: Thomas Graves <[email protected]>

* Rename GpuParquetScan object to match

Signed-off-by: Thomas Graves <[email protected]>

* Add tests for v2 datasources

Signed-off-by: Thomas Graves <[email protected]>

* Move OrcScan into shims

Signed-off-by: Thomas Graves <[email protected]>

* Fixes

Signed-off-by: Thomas Graves <[email protected]>

* Fix imports

Signed-off-by: Thomas Graves <[email protected]>

Co-authored-by: Thomas Graves <[email protected]>

* Filter nulls from joins where possible to improve performance. (NVIDIA#594)

* Filter nulls from joins where possible to improve performance.

Signed-off-by: Robert (Bobby) Evans <[email protected]>

* Addressed review comments

Signed-off-by: Robert (Bobby) Evans <[email protected]>

* Updated patch for other shims

* changelog generator

Signed-off-by: Peixin Li <[email protected]>

* only filter out labels for issue

* add nightly workflow on github actions

Co-authored-by: Andy Grove <[email protected]>
Co-authored-by: Thomas Graves <[email protected]>
Co-authored-by: Thomas Graves <[email protected]>
Co-authored-by: Niranjan Artal <[email protected]>
Co-authored-by: Jason Lowe <[email protected]>
Co-authored-by: Robert (Bobby) Evans <[email protected]>
pxLi added a commit to pxLi/spark-rapids that referenced this pull request Aug 24, 2020
@pxLi @andygrove
@tgravescs @nartal1 @jlowe @revans2
Local changelog test (#33)
bbbeadb 
* explicitly disable AQE in one test (NVIDIA#567)

Signed-off-by: Andy Grove <[email protected]>

* Enable using spark-submit to convert from csv to parquet (NVIDIA#565)

Signed-off-by: Andy Grove <[email protected]>

* xfail the Spark 3.1.0 integration tests that fail  (NVIDIA#580)

* xfail GpuTimeSub, arithmetic ops, and full outer join failures on 3.1.0

Signed-off-by: Thomas Graves <[email protected]>

* xfail the rest of the 3.1.0 tests and enable 3.1.0 unit tests in the
jenkins builds

Signed-off-by: Thomas Graves <[email protected]>

Co-authored-by: Thomas Graves <[email protected]>

* Fix unit tests when AQE is enabled (NVIDIA#558)

* Fix scala tests when AQE is enabled

Signed-off-by: Niranjan Artal <[email protected]>

* fix broadcasthashjoin tests

Signed-off-by: Niranjan Artal <[email protected]>

* fix indentation

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* Update buffer store to return compressed batches directly, add compression NVTX ranges (NVIDIA#572)

* Update buffer store to return compressed batches directly, add compression NVTX ranges

Signed-off-by: Jason Lowe <[email protected]>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <[email protected]>

* xfail the tpch spark 3.1.0 tests that fail (NVIDIA#588)

Signed-off-by: Thomas Graves <[email protected]>

Co-authored-by: Thomas Graves <[email protected]>

* Move GpuParquetScan/GpuOrcScan into Shim (NVIDIA#590)

* Move GpuParquetScan to shim

Signed-off-by: Thomas Graves <[email protected]>

* Move scan overrides into shim

Signed-off-by: Thomas Graves <[email protected]>

* Rename GpuParquetScan object to match

Signed-off-by: Thomas Graves <[email protected]>

* Add tests for v2 datasources

Signed-off-by: Thomas Graves <[email protected]>

* Move OrcScan into shims

Signed-off-by: Thomas Graves <[email protected]>

* Fixes

Signed-off-by: Thomas Graves <[email protected]>

* Fix imports

Signed-off-by: Thomas Graves <[email protected]>

Co-authored-by: Thomas Graves <[email protected]>

* Filter nulls from joins where possible to improve performance. (NVIDIA#594)

* Filter nulls from joins where possible to improve performance.

Signed-off-by: Robert (Bobby) Evans <[email protected]>

* Addressed review comments

Signed-off-by: Robert (Bobby) Evans <[email protected]>

* Updated patch for other shims

* changelog generator

Signed-off-by: Peixin Li <[email protected]>

* only filter out labels for issue

* add nightly workflow on github actions

* fix format

Co-authored-by: Andy Grove <[email protected]>
Co-authored-by: Thomas Graves <[email protected]>
Co-authored-by: Thomas Graves <[email protected]>
Co-authored-by: Niranjan Artal <[email protected]>
Co-authored-by: Jason Lowe <[email protected]>
Co-authored-by: Robert (Bobby) Evans <[email protected]>
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@nartal1
Fix unit tests when AQE is enabled (NVIDIA#558)
3593107 
* Fix scala tests when AQE is enabled

Signed-off-by: Niranjan Artal <[email protected]>

* fix broadcasthashjoin tests

Signed-off-by: Niranjan Artal <[email protected]>

* fix indentation

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@nartal1
Fix unit tests when AQE is enabled (NVIDIA#558)
3361fbe 
* Fix scala tests when AQE is enabled

Signed-off-by: Niranjan Artal <[email protected]>

* fix broadcasthashjoin tests

Signed-off-by: Niranjan Artal <[email protected]>

* fix indentation

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>

* addressed review comments

Signed-off-by: Niranjan Artal <[email protected]>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
@nvauto
Update submodule cudf to 006b2547801684239ce3bfe89648205306b4a72a (NV…
43e3532 
…IDIA#558)

Signed-off-by: spark-rapids automation <[email protected]>

Signed-off-by: spark-rapids automation <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andygrove andygrove andygrove approved these changes

Assignees

@nartal1 nartal1

Labels
test Only impacts tests
Projects
None yet
Milestone
Aug 17 - Aug 28
Development

Successfully merging this pull request may close these issues.

[FEA] Update HashSortOptimizerSuite to work with AQE
3 participants
@nartal1 @andygrove @sameerz