apache / apache/gluten

[VL] DPP V1 DisableScan/DisableProject suites extend the V2 base class, so the V1 path is never covered

Open
#12,857 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Scala
Stars
1.6k
Forks
657
Avg merge
2d 14h
Merged PRs (30d)
80

Description

### What happens

Three DPP suite variants carry `V1` in their name but extend the V2 base class, so they re-run the V2 scenario instead of covering the V1 DataSource path. In `gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenDynamicPartitionPruningSuite.scala`:

```scala
class GlutenDynamicPartitionPruningV1SuiteAEOffDisableScan // :748
extends GlutenDynamicPartitionPruningV2SuiteAEOff { ... }
class GlutenDynamicPartitionPruningV1SuiteAEOffWSCGOnDisableProject // :774
extends GlutenDynamicPartitionPruningV2SuiteAEOff { ... }
class GlutenDynamicPartitionPruningV1SuiteAEOffWSCGOffDisableProject // :782
extends GlutenDynamicPartitionPruningV2SuiteAEOff { ... }
```

The correct base is right there in the same file: `GlutenDynamicPartitionPruningV1SuiteAEOff` at `:456` extends `GlutenDynamicPartitionPruningV1Suite`. Only the `Disable*` variants get it wrong.

### Why it matters

DPP with a V1 relation and either scan or project offload turned off is not covered by anything today, while the V2 equivalent runs twice. Two suites' worth of CI time buys nothing. The names also actively mislead anyone reading the settings files, since `VeloxTestSettings` enables them under their V1 names.

### History

Not introduced by any recent change. The `DisableScan` variant has been written this way in every version module for as long as it has existed; the two `DisableProject` variants came from `caf959e46` (2023) in `gluten-ut/spark33` and were copied verbatim into 3.4/3.5/4.0/4.1 by #12840. Copilot flagged it on that PR.

### Suggested fix

Point all three at `GlutenDynamicPartitionPruningV1SuiteAEOff`, then run them: the expected plan shapes were written against V2 behaviour and some may need adjusting once the V1 path is actually exercised. Change `DisableScan` and `DisableProject` in the same PR across all five modules. Fixing only the newer `DisableProject` pair would leave two conventions inside one file.

Contributor guide

Open the contributing guide

Research direction

Start in gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenDynamicPartitionPruningSuite.scala, comparing the three V1 DisableScan/DisableProject classes with GlutenDynamicPartitionPruningV1SuiteAEOff at line 456. Apply the same base-class correction across all five version modules, then run the three suites and update any expected plan shapes that fail under the V1 path. Done means V1 coverage exists for both disabled scan and project offload without duplicate V2 runs.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala
Domain
testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.