Skip to content

Conversation

@andygrove
Copy link
Member

@andygrove andygrove commented Sep 29, 2025

Which issue does this PR close?

Closes #2469

Includes changes from #2505, so we should merge that PR first.

Rationale for this change

We were no longer running the plan stability tests if the default scan was explicitly set to native_comet, native_datafusion, or native_iceberg_compat, and this was always the case in CI, therefore we were not running these tests at all.

What changes are included in this PR?

How are these changes tested?

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.92%. Comparing base (f09f8af) to head (ee7102e).
⚠️ Report is 567 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2492      +/-   ##
============================================
+ Coverage     56.12%   58.92%   +2.80%     
- Complexity      976     1457     +481     
============================================
  Files           119      146      +27     
  Lines         11743    13550    +1807     
  Branches       2251     2356     +105     
============================================
+ Hits           6591     7985    +1394     
- Misses         4012     4343     +331     
- Partials       1140     1222      +82     

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

@andygrove andygrove force-pushed the plan-stability-regression branch from 1eda3a6 to 6166b4e Compare September 30, 2025 16:33
@andygrove
Copy link
Member Author

I'm seeing a difference in the explain plan (but not the simplified plan) for tpc-ds q9 with Spark 3.5

expected:<...m#35, count#36, sum#[37, count#38]
 but was:<...m#35, count#36, sum#[15, count#37]

@andygrove
Copy link
Member Author

I'm seeing a difference in the explain plan (but not the simplified plan) for tpc-ds q9 with Spark 3.5

expected:<...m#35, count#36, sum#[37, count#38]
 but was:<...m#35, count#36, sum#[15, count#37]

I cannot reproduce this issue locally 😞

The expected plan has:

(18) CometHashAggregate
Input [5]: [count#34, sum#35, count#36, sum#37, count#38]
Keys: []
Functions [3]: [count(1), avg(UnscaledValue(ss_ext_discount_amt#31)), avg(UnscaledValue(ss_net_paid#32))]

The actual plan has:

(18) CometHashAggregate
Input [5]: [count#34, sum#35, count#36, sum#15, count#37]
Keys: []
Functions [3]: [count(1), avg(UnscaledValue(ss_ext_discount_amt#31)), avg(UnscaledValue(ss_net_paid#32))]

@andygrove
Copy link
Member Author

I'm seeing a difference in the explain plan (but not the simplified plan) for tpc-ds q9 with Spark 3.5

expected:<...m#35, count#36, sum#[37, count#38]
 but was:<...m#35, count#36, sum#[15, count#37]

I cannot reproduce this issue locally 😞

The expected plan has:

(18) CometHashAggregate
Input [5]: [count#34, sum#35, count#36, sum#37, count#38]
Keys: []
Functions [3]: [count(1), avg(UnscaledValue(ss_ext_discount_amt#31)), avg(UnscaledValue(ss_net_paid#32))]

The actual plan has:

(18) CometHashAggregate
Input [5]: [count#34, sum#35, count#36, sum#15, count#37]
Keys: []
Functions [3]: [count(1), avg(UnscaledValue(ss_ext_discount_amt#31)), avg(UnscaledValue(ss_net_paid#32))]

I can reproduce this now

@andygrove andygrove changed the title fix: Fix regression with plan stability tests in CI [WIP] fix: Fix regression with plan stability tests in CI Oct 1, 2025
@andygrove andygrove marked this pull request as ready for review October 1, 2025 14:10
@andygrove andygrove marked this pull request as draft October 2, 2025 15:24
@andygrove
Copy link
Member Author

This PR is too large to review. I am going to create smaller PRs to build up to this.

@andygrove andygrove closed this Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TPC-DS Plan Stability Suites Not Running in CI

2 participants