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

Fallback to CPU when datasource v2 enables RuntimeFiltering #3456

Merged
merged 2 commits into from
Sep 15, 2021

Conversation

wbo4958
Copy link
Collaborator

@wbo4958 wbo4958 commented Sep 13, 2021

This PR is to fix #3093

@wbo4958
Copy link
Collaborator Author

wbo4958 commented Sep 13, 2021

build

@revans2 revans2 changed the title Fallback to CPU when datasource v2 enables DPP Fallback to CPU when datasource v2 enables RuntimeFiltering Sep 13, 2021
revans2
revans2 previously approved these changes Sep 13, 2021
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just a few nits

override def tagPlanForGpu(): Unit = {
// Only Scan with SupportsRuntimeFiltering can support DPP.
if (!p.runtimeFilters.isEmpty) {
willNotWorkOnGpu("The DPP has not been supported")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we update the comment to be a little more clear? Perhaps something like.

"Runtime filtering (DPP) on datasource V2 is not supported"

@wbo4958
Copy link
Collaborator Author

wbo4958 commented Sep 13, 2021

build

1 similar comment
@wbo4958
Copy link
Collaborator Author

wbo4958 commented Sep 14, 2021

build

revans2
revans2 previously approved these changes Sep 14, 2021
GpuOrcScanBase.tagSupport(this)
// we are being overly cautious and that Orc does not support this yet
if (a.isInstanceOf[SupportsRuntimeFiltering]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

here and elsewhere: Does calling tagSupport(this) on L542 make sense if this is true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Per my understanding. Plugin is trying best to explain all the reasons that won't run on GPU at a time.

@wbo4958
Copy link
Collaborator Author

wbo4958 commented Sep 14, 2021

build

@wbo4958 wbo4958 merged commit 492c15f into NVIDIA:branch-21.10 Sep 15, 2021
@wbo4958 wbo4958 deleted the batchscan-fallback branch September 15, 2021 01:09
@sameerz sameerz added this to the Sep 13 - Sep 24 milestone Sep 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GpuBatchScanExec needs to be changed to match the change in SPARK-35779
5 participants