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
Fix scala tests when AQE is enabled
b68daae 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1
fix broadcasthashjoin tests
2e86fa0 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1
fix indentation
853ce84 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@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 <nartal@nvidia.com>
@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 <nartal@nvidia.com>
@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 <andygrove@nvidia.com>

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

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* 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 <tgraves@nvidia.com>

* 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 <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

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

* Fix scala tests when AQE is enabled

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

* fix broadcasthashjoin tests

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

* fix indentation

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

* addressed review comments

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

* addressed review comments

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

* addressed review comments

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

* 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 <jlowe@nvidia.com>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

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

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

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

* Move GpuParquetScan to shim

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move scan overrides into shim

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Rename GpuParquetScan object to match

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Add tests for v2 datasources

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move OrcScan into shims

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Fixes

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Fix imports

Signed-off-by: Thomas Graves <tgraves@apache.org>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* 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 <bobby@apache.org>

* Addressed review comments

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

* Updated patch for other shims

* changelog generator

Signed-off-by: Peixin Li <pxli@nyu.edu>

* only filter out labels for issue

* add nightly workflow on github actions

Co-authored-by: Andy Grove <andygrove@users.noreply.github.com>
Co-authored-by: Thomas Graves <tgraves@apache.org>
Co-authored-by: Thomas Graves <tgraves@nvidia.com>
Co-authored-by: Niranjan Artal <50492963+nartal1@users.noreply.github.com>
Co-authored-by: Jason Lowe <jlowe@nvidia.com>
Co-authored-by: Robert (Bobby) Evans <bobby@apache.org>
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 <andygrove@nvidia.com>

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

Signed-off-by: Andy Grove <andygrove@nvidia.com>

* 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 <tgraves@nvidia.com>

* 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 <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

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

* Fix scala tests when AQE is enabled

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

* fix broadcasthashjoin tests

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

* fix indentation

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

* addressed review comments

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

* addressed review comments

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

* addressed review comments

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

* 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 <jlowe@nvidia.com>

* Update parameter name for clarity

Signed-off-by: Jason Lowe <jlowe@nvidia.com>

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

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

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

* Move GpuParquetScan to shim

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move scan overrides into shim

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Rename GpuParquetScan object to match

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Add tests for v2 datasources

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Move OrcScan into shims

Signed-off-by: Thomas Graves <tgraves@apache.org>

* Fixes

Signed-off-by: Thomas Graves <tgraves@nvidia.com>

* Fix imports

Signed-off-by: Thomas Graves <tgraves@apache.org>

Co-authored-by: Thomas Graves <tgraves@nvidia.com>

* 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 <bobby@apache.org>

* Addressed review comments

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

* Updated patch for other shims

* changelog generator

Signed-off-by: Peixin Li <pxli@nyu.edu>

* only filter out labels for issue

* add nightly workflow on github actions

* fix format

Co-authored-by: Andy Grove <andygrove@users.noreply.github.com>
Co-authored-by: Thomas Graves <tgraves@apache.org>
Co-authored-by: Thomas Graves <tgraves@nvidia.com>
Co-authored-by: Niranjan Artal <50492963+nartal1@users.noreply.github.com>
Co-authored-by: Jason Lowe <jlowe@nvidia.com>
Co-authored-by: Robert (Bobby) Evans <bobby@apache.org>
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 <nartal@nvidia.com>

* fix broadcasthashjoin tests

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

* fix indentation

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

* addressed review comments

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

* addressed review comments

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
@nartal1
Fix unit tests when AQE is enabled (NVIDIA#558)
3361fbe 
* Fix scala tests when AQE is enabled

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

* fix broadcasthashjoin tests

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

* fix indentation

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

* addressed review comments

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

* addressed review comments

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

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
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 <70000568+nvauto@users.noreply.github.com>

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
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