Skip to content

Conversation

@beliefer
Copy link
Contributor

@beliefer beliefer commented Feb 22, 2025

What changes were proposed in this pull request?

This PR proposes to remove unnecessary inheritance from PlanTestBase, ExpressionEvalHelper and PlanTest.

Why are the changes needed?

  1. Some class extends both ExpressionEvalHelper and PlanTestBase, but ExpressionEvalHelper already extends PlanTestBase.
trait ExpressionEvalHelper extends ScalaCheckDrivenPropertyChecks with PlanTestBase {
  self: SparkFunSuite =>
  ...
}
  1. Class NullDownPropagationSuite, OptimizeCsvExprsSuite, PushFoldableIntoBranchesSuite doesn't need ExpressionEvalHelper at all.
  2. Some class extends both QueryTest and PlanTest, but QueryTest already extends PlanTest.
abstract class QueryTest extends PlanTest {
...
}

Does this PR introduce any user-facing change?

'No'.
Just update the inner code.

How was this patch tested?

GA.

Was this patch authored or co-authored using generative AI tooling?

'No'.

@github-actions github-actions bot added the SQL label Feb 22, 2025
@beliefer
Copy link
Contributor Author

ping @LuciferYang cc @cloud-fan

@beliefer beliefer closed this in 3027968 Feb 23, 2025
beliefer added a commit that referenced this pull request Feb 23, 2025
…ExpressionEvalHelper and PlanTest

### What changes were proposed in this pull request?
This PR proposes to remove unnecessary inheritance from `PlanTestBase`, `ExpressionEvalHelper` and `PlanTest`.

### Why are the changes needed?

1. Some class extends both `ExpressionEvalHelper` and `PlanTestBase`, but `ExpressionEvalHelper` already extends `PlanTestBase`.
```
trait ExpressionEvalHelper extends ScalaCheckDrivenPropertyChecks with PlanTestBase {
  self: SparkFunSuite =>
  ...
}
```
2. Class `NullDownPropagationSuite`, `OptimizeCsvExprsSuite`, `PushFoldableIntoBranchesSuite` doesn't need `ExpressionEvalHelper` at all.
3. Some class extends both `QueryTest` and `PlanTest`, but `QueryTest` already extends `PlanTest`.
```
abstract class QueryTest extends PlanTest {
...
}
```

### Does this PR introduce _any_ user-facing change?
'No'.
Just update the inner code.

### How was this patch tested?
GA.

### Was this patch authored or co-authored using generative AI tooling?
'No'.

Closes #50047 from beliefer/SPARK-51292.

Authored-by: beliefer <beliefer@163.com>
Signed-off-by: beliefer <beliefer@163.com>
(cherry picked from commit 3027968)
Signed-off-by: beliefer <beliefer@163.com>
@beliefer
Copy link
Contributor Author

Merged into branch-4.0/master
@LuciferYang Thank you!

zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 14, 2025
…ExpressionEvalHelper and PlanTest

### What changes were proposed in this pull request?
This PR proposes to remove unnecessary inheritance from `PlanTestBase`, `ExpressionEvalHelper` and `PlanTest`.

### Why are the changes needed?

1. Some class extends both `ExpressionEvalHelper` and `PlanTestBase`, but `ExpressionEvalHelper` already extends `PlanTestBase`.
```
trait ExpressionEvalHelper extends ScalaCheckDrivenPropertyChecks with PlanTestBase {
  self: SparkFunSuite =>
  ...
}
```
2. Class `NullDownPropagationSuite`, `OptimizeCsvExprsSuite`, `PushFoldableIntoBranchesSuite` doesn't need `ExpressionEvalHelper` at all.
3. Some class extends both `QueryTest` and `PlanTest`, but `QueryTest` already extends `PlanTest`.
```
abstract class QueryTest extends PlanTest {
...
}
```

### Does this PR introduce _any_ user-facing change?
'No'.
Just update the inner code.

### How was this patch tested?
GA.

### Was this patch authored or co-authored using generative AI tooling?
'No'.

Closes apache#50047 from beliefer/SPARK-51292.

Authored-by: beliefer <beliefer@163.com>
Signed-off-by: beliefer <beliefer@163.com>
(cherry picked from commit b107b11)
Signed-off-by: beliefer <beliefer@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants