Skip to content

chore: Enable plan stability suite for native_datafusion scans#2877

Merged
mbutrovich merged 4 commits into
apache:mainfrom
andygrove:native-datafusion-plan-stability
Dec 11, 2025
Merged

chore: Enable plan stability suite for native_datafusion scans#2877
mbutrovich merged 4 commits into
apache:mainfrom
andygrove:native-datafusion-plan-stability

Conversation

@andygrove
Copy link
Copy Markdown
Member

@andygrove andygrove commented Dec 11, 2025 •
edited
Loading

Which issue does this PR close?

Closes #2672

Rationale for this change

Fix a bug where we were not falling back to Spark for native_datafusion scans with subqueries.

What changes are included in this PR?

  • Fix fallback logic for native_datafusion in CometScanRule
  • Update golden files

How are these changes tested?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 11, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.32%. Comparing base (f09f8af) to head (ed01dd8).
⚠️ Report is 1167 commits behind head on main.

Files with missing lines Patch % Lines
...ala/org/apache/spark/sql/comet/CometScanExec.scala 25.00% 0 Missing and 3 partials ⚠️
...n/scala/org/apache/comet/rules/CometScanRule.scala 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #2877      +/-   ##
============================================
+ Coverage     56.12%   59.32%   +3.20%     
- Complexity      976     1377     +401     
============================================
  Files           119      167      +48     
  Lines         11743    15298    +3555     
  Branches       2251     2533     +282     
============================================
+ Hits           6591     9076    +2485     
- Misses         4012     4934     +922     
- Partials       1140     1288     +148

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mbutrovich
Copy link
Copy Markdown
Contributor

mbutrovich commented Dec 11, 2025

Does this close #2672?

@andygrove andygrove changed the title chore: Enable plan stability suite for native_datafusion for Spark 3.4 chore: Enable plan stability suite for native_datafusion scans Dec 11, 2025
andygrove
andygrove commented Dec 11, 2025
View reviewed changes
Comment thread spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala
Comment on lines +166 to +167
fallbackReasons += "Native DataFusion scan does not support subqueries/dynamic pruning"
return withInfos(scanExec, fallbackReasons.toSet)
Copy link
Copy Markdown
Member Author

@andygrove andygrove Dec 11, 2025

Choose a reason for hiding this comment

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

I am going to to follow up with another PR to clean up fallback handling in this file. I just followed the existing pattern for now.

@andygrove
Copy link
Copy Markdown
Member Author

andygrove commented Dec 11, 2025

Does this close #2672?

Yes! I updated the PR description to add this.

@andygrove andygrove marked this pull request as ready for review December 11, 2025 16:01
@andygrove andygrove mentioned this pull request Dec 11, 2025
Merged
mbutrovich
mbutrovich approved these changes Dec 11, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mbutrovich mbutrovich left a comment

Choose a reason for hiding this comment

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

Thanks for making sense of the scan rules, @andygrove!

Comment thread ...esources/tpcds-plan-stability/approved-plans-v1_4-spark3_5/q1.native_datafusion/extended.txt
: : +- CometColumnarToRow
: : +- CometColumnarExchange
: : +- HashAggregate
: : +- HashAggregate
Copy link
Copy Markdown
Contributor

@mbutrovich mbutrovich Dec 11, 2025

Choose a reason for hiding this comment

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

I'm curious about some of these HashAggregate fallbacks.

@mbutrovich mbutrovich merged commit 2d3172d into apache:main Dec 11, 2025
135 of 139 checks passed
@andygrove andygrove deleted the native-datafusion-plan-stability branch December 12, 2025 16:02
@andygrove andygrove mentioned this pull request Jan 23, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbutrovich mbutrovich mbutrovich approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[native_datafusion] Subquery has not finished

3 participants

@andygrove @codecov-commenter @mbutrovich