fix: Remove `COMET_SHUFFLE_FALLBACK_TO_COLUMNAR` hack by andygrove · Pull Request #1736 · apache/datafusion-comet

andygrove · 2025-05-13T17:38:41Z

Which issue does this PR close?

Part of #1254

Rationale for this change

The config COMET_SHUFFLE_FALLBACK_TO_COLUMNAR was added as a hack so that we could avoid fixing some failing Spark SQL tests. However, the failing tests are real issues and only show up now because more queries are running natively.

What changes are included in this PR?

This PR removes the hack and updates the diff files to either fix or ignore failing tests. Some of these tests will be enabled later as part of #1254.

How are these changes tested?

codecov-commenter · 2025-05-13T20:13:14Z

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 59.40%. Comparing base (f09f8af) to head (c105513).
Report is 247 commits behind head on main.

Files with missing lines	Patch %	Lines
...he/comet/rules/EliminateRedundantTransitions.scala	0.00%	1 Missing and 2 partials ⚠️
...pache/spark/sql/comet/CometColumnarToRowExec.scala	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1736      +/-   ##
============================================
+ Coverage     56.12%   59.40%   +3.27%     
- Complexity      976     1151     +175     
============================================
  Files           119      130      +11     
  Lines         11743    12661     +918     
  Branches       2251     2375     +124     
============================================
+ Hits           6591     7521     +930     
+ Misses         4012     3930      -82     
- Partials       1140     1210      +70

☔ 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 · 2025-05-30T20:01:02Z

There are now many more test failures than when this PR was first created.

coderfender · 2025-06-02T04:10:19Z

Working on this

coderfender · 2025-06-02T04:11:40Z

Working on this

…ck-hack

…nto remove-shuffle-fallback-hack

…ck-hack

…allback-hack

…ck-hack

andygrove · 2025-06-06T16:38:04Z

The remaining failure is related to exchange reuse in TPC-DS q44.

2025-06-06T00:48:02.3079684Z OUTPUT: TakeOrderedAndProject(limit=100, orderBy=[rnk#18761 ASC NULLS FIRST], output=[rnk#18761,best_performing#18767,worst_performing#18768])
2025-06-06T00:48:02.3081088Z +- Project [rnk#18761, i_product_name#18894 AS best_performing#18767, i_product_name#18905 AS worst_performing#18768]
2025-06-06T00:48:02.3082152Z    +- BroadcastHashJoin [item_sk#18762], [i_item_sk#18796], Inner, BuildRight, false
2025-06-06T00:48:02.3082940Z       :- Project [rnk#18761, item_sk#18762, i_product_name#18894]
2025-06-06T00:48:02.3083811Z       :  +- BroadcastHashJoin [item_sk#18757], [i_item_sk#722], Inner, BuildRight, false
2025-06-06T00:48:02.3084580Z       :     :- Project [item_sk#18757, rnk#18761, item_sk#18762]
2025-06-06T00:48:02.3085314Z       :     :  +- BroadcastHashJoin [rnk#18761], [rnk#18766], Inner, BuildRight, false
2025-06-06T00:48:02.3086015Z       :     :     :- Project [item_sk#18757, rnk#18761]
2025-06-06T00:48:02.3086766Z       :     :     :  +- Filter ((rnk#18761 < 11) AND isnotnull(item_sk#18757))
2025-06-06T00:48:02.3088247Z       :     :     :     +- Window [rank(rank_col#18758) windowspecdefinition(rank_col#18758 ASC NULLS FIRST, specifiedwindowframe(RowFrame, unboundedpreceding$(), currentrow$())) AS rnk#18761], [rank_col#18758 ASC NULLS FIRST]
2025-06-06T00:48:02.3089876Z       :     :     :        +- WindowGroupLimit [rank_col#18758 ASC NULLS FIRST], rank(rank_col#18758), 10, Final
2025-06-06T00:48:02.3090689Z       :     :     :           +- Sort [rank_col#18758 ASC NULLS FIRST], false, 0
2025-06-06T00:48:02.3091724Z       :     :     :              +- Filter (isnotnull(rank_col#18758) AND (cast(rank_col#18758 as decimal(13,7)) > (0.9 * ReusedSubquery Subquery subquery#18765, [id=#147283])))
2025-06-06T00:48:02.3092900Z       :     :     :                 :  +- ReusedSubquery Subquery subquery#18765, [id=#147283]
2025-06-06T00:48:02.3093962Z       :     :     :                 +- HashAggregate(keys=[ss_item_sk#701], functions=[avg(UnscaledValue(ss_net_profit#721))], output=[item_sk#18757, rank_col#18758])
2025-06-06T00:48:02.3094946Z       :     :     :                    +- CometColumnarToRow
2025-06-06T00:48:02.3095489Z       :     :     :                       +- ShuffleQueryStage 0
2025-06-06T00:48:02.3096562Z       :     :     :                          +- CometExchange hashpartitioning(ss_item_sk#701, 1), ENSURE_REQUIREMENTS, CometNativeShuffle, [plan_id=146851]
2025-06-06T00:48:02.3097956Z       :     :     :                             +- !CometHashAggregate [ss_item_sk#701, ss_net_profit#721], Partial, [ss_item_sk#701], [partial_avg(UnscaledValue(ss_net_profit#721))]
2025-06-06T00:48:02.3099165Z       :     :     :                                +- CometProject [ss_item_sk#701, ss_net_profit#721], [ss_item_sk#701, ss_net_profit#721]
2025-06-06T00:48:02.3100381Z       :     :     :                                   +- CometFilter [ss_item_sk#701, ss_store_sk#706, ss_net_profit#721], (isnotnull(ss_store_sk#706) AND (ss_store_sk#706 = 4))
2025-06-06T00:48:02.3103650Z       :     :     :                                      +- CometScan parquet spark_catalog.default.store_sales[ss_item_sk#701,ss_store_sk#706,ss_net_profit#721] Batched: true, DataFilters: [isnotnull(ss_store_sk#706), (ss_store_sk#706 = 4)], Format: CometParquet, Location: InMemoryFileIndex(1 paths)[file:/__w/datafusion-comet/datafusion-comet/tpcds-sf-1/store_sales], PartitionFilters: [], PushedFilters: [IsNotNull(ss_store_sk), EqualTo(ss_store_sk,4)], ReadSchema: struct<ss_item_sk:int,ss_store_sk:int,ss_net_profit:decimal(7,2)>
2025-06-06T00:48:02.3106642Z       :     :     +- BroadcastQueryStage 4
2025-06-06T00:48:02.3107602Z       :     :        +- BroadcastExchange HashedRelationBroadcastMode(List(cast(input[1, int, false] as bigint)),false), [plan_id=147448]
2025-06-06T00:48:02.3108617Z       :     :           +- *(2) Project [item_sk#18762, rnk#18766]
2025-06-06T00:48:02.3109338Z       :     :              +- *(2) Filter ((rnk#18766 < 11) AND isnotnull(item_sk#18762))
2025-06-06T00:48:02.3110827Z       :     :                 +- Window [rank(rank_col#18763) windowspecdefinition(rank_col#18763 DESC NULLS LAST, specifiedwindowframe(RowFrame, unboundedpreceding$(), currentrow$())) AS rnk#18766], [rank_col#18763 DESC NULLS LAST]
2025-06-06T00:48:02.3112485Z       :     :                    +- WindowGroupLimit [rank_col#18763 DESC NULLS LAST], rank(rank_col#18763), 10, Final
2025-06-06T00:48:02.3113319Z       :     :                       +- *(1) Sort [rank_col#18763 DESC NULLS LAST], false, 0
2025-06-06T00:48:02.3114412Z       :     :                          +- *(1) Filter (isnotnull(rank_col#18763) AND (cast(rank_col#18763 as decimal(13,7)) > (0.9 * Subquery subquery#18765, [id=#147283])))
2025-06-06T00:48:02.3115373Z       :     :                             :  +- Subquery subquery#18765, [id=#147283]
2025-06-06T00:48:02.3115975Z       :     :                             :     +- AdaptiveSparkPlan isFinalPlan=true
2025-06-06T00:48:02.3116693Z                                                    +- == Final Plan ==
2025-06-06T00:48:02.3117643Z                                                       *(1) HashAggregate(keys=[ss_store_sk#18830], functions=[avg(UnscaledValue(ss_net_profit#18845))], output=[rank_col#18759])
2025-06-06T00:48:02.3118606Z                                                       +- *(1) CometColumnarToRow
2025-06-06T00:48:02.3119179Z                                                          +- ShuffleQueryStage 0
2025-06-06T00:48:02.3120162Z                                                             +- CometExchange hashpartitioning(ss_store_sk#18830, 1), ENSURE_REQUIREMENTS, CometNativeShuffle, [plan_id=147498]
2025-06-06T00:48:02.3121750Z                                                                +- !CometHashAggregate [ss_store_sk#18830, ss_net_profit#18845], Partial, [ss_store_sk#18830], [partial_avg(UnscaledValue(ss_net_profit#18845))]
2025-06-06T00:48:02.3123117Z                                                                   +- CometProject [ss_store_sk#18830, ss_net_profit#18845], [ss_store_sk#18830, ss_net_profit#18845]
2025-06-06T00:48:02.3124510Z                                                                      +- CometFilter [ss_addr_sk#18829, ss_store_sk#18830, ss_net_profit#18845], ((isnotnull(ss_store_sk#18830) AND (ss_store_sk#18830 = 4)) AND isnull(ss_addr_sk#18829))
2025-06-06T00:48:02.3128544Z                                                                         +- CometScan parquet spark_catalog.default.store_sales[ss_addr_sk#18829,ss_store_sk#18830,ss_net_profit#18845] Batched: true, DataFilters: [isnotnull(ss_store_sk#18830), (ss_store_sk#18830 = 4), isnull(ss_addr_sk#18829)], Format: CometParquet, Location: InMemoryFileIndex(1 paths)[file:/__w/datafusion-comet/datafusion-comet/tpcds-sf-1/store_sales], PartitionFilters: [], PushedFilters: [IsNotNull(ss_store_sk), EqualTo(ss_store_sk,4), IsNull(ss_addr_sk)], ReadSchema: struct<ss_addr_sk:int,ss_store_sk:int,ss_net_profit:decimal(7,2)>
2025-06-06T00:48:02.3131704Z                                                    +- == Initial Plan ==
2025-06-06T00:48:02.3132670Z                                                       HashAggregate(keys=[ss_store_sk#18830], functions=[avg(UnscaledValue(ss_net_profit#18845))], output=[rank_col#18759])
2025-06-06T00:48:02.3134048Z                                                       +- CometExchange hashpartitioning(ss_store_sk#18830, 1), ENSURE_REQUIREMENTS, CometNativeShuffle, [plan_id=146421]
2025-06-06T00:48:02.3135539Z                                                          +- !CometHashAggregate [ss_store_sk#18830, ss_net_profit#18845], Partial, [ss_store_sk#18830], [partial_avg(UnscaledValue(ss_net_profit#18845))]
2025-06-06T00:48:02.3139421Z                                                             +- CometProject [ss_store_sk#18830, ss_net_profit#18845], [ss_store_sk#18830, ss_net_profit#18845]
2025-06-06T00:48:02.3140970Z                                                                +- CometFilter [ss_addr_sk#18829, ss_store_sk#18830, ss_net_profit#18845], ((isnotnull(ss_store_sk#18830) AND (ss_store_sk#18830 = 4)) AND isnull(ss_addr_sk#18829))
2025-06-06T00:48:02.3144752Z                                                                   +- CometScan parquet spark_catalog.default.store_sales[ss_addr_sk#18829,ss_store_sk#18830,ss_net_profit#18845] Batched: true, DataFilters: [isnotnull(ss_store_sk#18830), (ss_store_sk#18830 = 4), isnull(ss_addr_sk#18829)], Format: CometParquet, Location: InMemoryFileIndex(1 paths)[file:/__w/datafusion-comet/datafusion-comet/tpcds-sf-1/store_sales], PartitionFilters: [], PushedFilters: [IsNotNull(ss_store_sk), EqualTo(ss_store_sk,4), IsNull(ss_addr_sk)], ReadSchema: struct<ss_addr_sk:int,ss_store_sk:int,ss_net_profit:decimal(7,2)>
2025-06-06T00:48:02.3148516Z       :     :                             +- *(1) HashAggregate(keys=[ss_item_sk#18775], functions=[avg(UnscaledValue(ss_net_profit#18795))], output=[item_sk#18762, rank_col#18763])
2025-06-06T00:48:02.3149536Z       :     :                                +- *(1) ColumnarToRow
2025-06-06T00:48:02.3150077Z       :     :                                   +- ShuffleQueryStage 1
2025-06-06T00:48:02.3151311Z       :     :                                      +- ReusedExchange [ss_item_sk#18775, sum#18915, count#18916L], CometExchange hashpartitioning(ss_item_sk#701, 1), ENSURE_REQUIREMENTS, CometNativeShuffle, [plan_id=146851]
2025-06-06T00:48:02.3152516Z       :     +- CometColumnarToRow
2025-06-06T00:48:02.3152984Z       :        +- BroadcastQueryStage 2
2025-06-06T00:48:02.3153616Z       :           +- CometBroadcastExchange [i_item_sk#722, i_product_name#18894]
2025-06-06T00:48:02.3155540Z       :              +- CometProject [i_item_sk#722, i_product_name#18894], [i_item_sk#722, staticinvoke(class org.apache.spark.sql.catalyst.util.CharVarcharCodegenUtils, StringType, readSidePadding, i_product_name#743, 50, true, false, true) AS i_product_name#18894]
2025-06-06T00:48:02.3157539Z       :                 +- CometFilter [i_item_sk#722, i_product_name#743], isnotnull(i_item_sk#722)
2025-06-06T00:48:02.3160081Z       :                    +- CometScan parquet spark_catalog.default.item[i_item_sk#722,i_product_name#743] Batched: true, DataFilters: [isnotnull(i_item_sk#722)], Format: CometParquet, Location: InMemoryFileIndex(1 paths)[file:/__w/datafusion-comet/datafusion-comet/tpcds-sf-1/item], PartitionFilters: [], PushedFilters: [IsNotNull(i_item_sk)], ReadSchema: struct<i_item_sk:int,i_product_name:string>
2025-06-06T00:48:02.3162411Z       +- ColumnarToRow
2025-06-06T00:48:02.3162821Z          +- BroadcastQueryStage 3
2025-06-06T00:48:02.3163722Z             +- ReusedExchange [i_item_sk#18796, i_product_name#18905], CometBroadcastExchange [i_item_sk#722, i_product_name#18894]

Note the C2R around a reused exchange:

2025-06-06T00:48:02.3162411Z       +- ColumnarToRow
2025-06-06T00:48:02.3162821Z          +- BroadcastQueryStage 3
2025-06-06T00:48:02.3163722Z             +- ReusedExchange [i_item_sk#18796, i_product_name#18905],

andygrove · 2025-06-06T19:16:52Z

Test now pass.

…ck-hack

andygrove · 2025-06-08T14:39:02Z

replaced with #1865

Remove COMET_SHUFFLE_FALLBACK_TO_COLUMNAR config

b3cf832

andygrove added 3 commits May 30, 2025 11:20

upmerge

cdf0999

format

81b3384

Specify -Dsbt.log.noformat=true in sbt CI runs

8626f51

andygrove mentioned this pull request May 30, 2025

Panic in Comet shuffle when building structs #1823

Closed

Merge branch 'sbt-log-noformat' into remove-shuffle-fallback-hack

dfe2d47

andygrove mentioned this pull request May 30, 2025

Incorrect results with JVM shuffle: Spark SQL - SPARK-32038: NormalizeFloatingNumbers should work on distinct aggregate #1824

Closed

Merge remote-tracking branch 'apache/main' into remove-shuffle-fallba…

a570f58

…ck-hack

andygrove mentioned this pull request Jun 3, 2025

[EPIC] Spark SQL test failures when Comet JVM shuffle is used #1254

Closed

andygrove and others added 17 commits June 3, 2025 14:51

Merge remote-tracking branch 'apache/main' into remove-shuffle-fallba…

628a623

…ck-hack

update test

dab5fbf

Fix shuffle writing rows containing null struct fields

8dc4e9b

Ignore miri for test_append_null_struct_field_to_struct_builder

0fcd7eb

Merge remote-tracking branch 'kontinuation/fix-appending-null-rows' i…

f69a4c6

…nto remove-shuffle-fallback-hack

Merge remote-tracking branch 'apache/main' into remove-shuffle-fallba…

b867d3e

…ck-hack

ignore test for 3.5.5

2cc00c2

update docs

0fda09b

3.4.3

c4219cd

3.5.4

eb69d50

4.0.0-preview1

6f90a2b

fall back to Spark for window ranges

25fcccf

Merge branch 'window-int-long' into remove-shuffle-fallback-hack

baad6df

Merge branch 'count-distinct-nan-in-aggregates' into remove-shuffle-f…

1f25095

…allback-hack

Merge remote-tracking branch 'apache/main' into remove-shuffle-fallba…

d7b09ed

…ck-hack

ignore DPP test with 3.5.5

cd03a1c

fix

a79b445

fmt

019bcd4

andygrove mentioned this pull request Jun 6, 2025

Release Comet 0.9.0 (June/July 2025) #1856

Closed

2 tasks

andygrove added 3 commits June 6, 2025 11:09

fix?

d054f6b

format

ccd710c

fix?

01c43aa

andygrove mentioned this pull request Jun 6, 2025

fix: Update broadcast exchange logic to support reused exchanges #1858

Closed

andygrove added 15 commits June 6, 2025 13:17

Merge remote-tracking branch 'apache/main' into remove-shuffle-fallba…

6d7f893

…ck-hack

upmerge and remove one change

28787ed

format

ab127fa

Update 3.5.5 diff

940a046

Update 3.4.3 diff

89ce244

Update 3.5.4 diff

e4d57ce

Update 3.4.3 diff

980d175

Update 4.0.0-preview1 diff

97a19b9

ignore wholestagecodegen tests

469b78b

ignore wholestagecodegen tests

99f6cb9

enable tests for 3.4.3

fe8b887

enable tests for 3.4.3

eaf2768

3.5.4

3c2d06b

4.0.0

30c962c

prep for review

2b416d3

andygrove marked this pull request as ready for review June 6, 2025 22:56

andygrove added 3 commits June 6, 2025 17:40

fix 3.4.3 diff

0968c29

fix 3.5.4 diff

3f6d03d

Merge remote-tracking branch 'apache/main' into remove-shuffle-fallba…

c105513

…ck-hack

andygrove closed this Jun 8, 2025

andygrove deleted the remove-shuffle-fallback-hack branch July 10, 2025 22:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Remove `COMET_SHUFFLE_FALLBACK_TO_COLUMNAR` hack#1736

fix: Remove `COMET_SHUFFLE_FALLBACK_TO_COLUMNAR` hack#1736
andygrove wants to merge 55 commits intoapache:mainfrom
andygrove:remove-shuffle-fallback-hack

andygrove commented May 13, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented May 13, 2025 •

edited

Loading

Uh oh!

andygrove commented May 30, 2025

Uh oh!

coderfender commented Jun 2, 2025

Uh oh!

coderfender commented Jun 2, 2025

Uh oh!

andygrove commented Jun 6, 2025

Uh oh!

andygrove commented Jun 6, 2025

Uh oh!

andygrove commented Jun 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

andygrove commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

codecov-commenter commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andygrove commented May 30, 2025

Uh oh!

coderfender commented Jun 2, 2025

Uh oh!

coderfender commented Jun 2, 2025

Uh oh!

andygrove commented Jun 6, 2025

Uh oh!

andygrove commented Jun 6, 2025

Uh oh!

andygrove commented Jun 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andygrove commented May 13, 2025 •

edited

Loading

codecov-commenter commented May 13, 2025 •

edited

Loading