deepakpanda93 opened a new pull request, #19471:
URL: https://github.com/apache/hudi/pull/19471
### Describe the issue this Pull Request addresses
Closes #17007
`TestPartitionBucketIndexSupport.exprFilePathAnswerCheck` builds a throwaway
logical plan so it can run the optimizer over a resolved expression. It used
Spark's `DummyExpressionHolder`, which hardcodes `output` to `Nil`, so the
attributes referenced by the expression appear nowhere in the plan's output.
The validation rule added by SPARK-44219 rejects exactly that on Spark 4:
```
org.apache.spark.SparkException: [PLAN_VALIDATION_FAILED_RULE_EXECUTOR] The
input plan of
org.apache.spark.sql.internal.BaseSessionStateBuilder$$anon$2 is invalid:
Aliases A#0L are dangling in the references for plan:
DummyExpressionHolder [(A#0L = cast(3 as bigint))]
```
Rather than being fixed, the whole method body was wrapped in `if
(!HoodieSparkUtils.gteqSpark4_0) { // TODO (HUDI-9403) }`. On Spark 4 the
method returned before asserting anything, so the eleven
`exprFilePathAnswerCheck` calls across `testSingleHashFieldsExpression` and
`testMultipleHashFieldsExpress` passed vacuously while the test reported green.
### Summary and Changelog
`TestBucketIndexSupport`, the parent class, already hit this and forked the
holder for exactly this reason:
```scala
// SPARK-44219 added extra rule to validate expressions against its
children's references to check if there are dangling references
// This is forked from Spark's [[DummyExpressionHolder]], which always set
the output to Nil and would fail this test
case class HoodieDummyExpressionHolder(exprs: Seq[Expression], output:
Seq[Attribute]) extends LeafNode
```
`TestPartitionBucketIndexSupport` extends `TestBucketIndexSupport`, so that
holder is already in scope and no new class is needed.
- Use `HoodieDummyExpressionHolder(Seq(resolveExpr),
resolveExpr.references.toSeq)` so the referenced attributes are part of the
plan's output and the plan validates.
- Drop the `!gteqSpark4_0` guard so the assertions run on every Spark
version.
- Remove the two imports that become unused (`DummyExpressionHolder`,
`HoodieSparkUtils`).
Test-only change; no production code is touched. `git diff -w` is six lines,
the rest of the diff is re-indentation from removing the guard.
### Impact
Restores real assertions for `PartitionBucketIndexSupport` on Spark 4, where
they were being skipped. No behaviour change on Spark 3.x, where the guard was
already inactive.
### Risk Level
low
### Documentation Update
none
### Verification
| Configuration | Result |
| --- | --- |
| Spark 3.5 / Scala 2.12 | 4/4 pass (no regression) |
| Spark 4.0 / Scala 2.13, `SPARK_TESTING=1` | 4/4 pass |
| Spark 4.0 / Scala 2.13, `SPARK_TESTING=1`, with Spark's
`DummyExpressionHolder` restored and the guard removed | fails with
`PLAN_VALIDATION_FAILED_RULE_EXECUTOR`, matching the report |
The third row is the control: it confirms the assertions genuinely execute
on Spark 4 now, rather than the test passing for the same reason it passed
before.
Worth noting for anyone reproducing this: Spark gates plan change validation
behind `Utils.isTesting`, so running this test on its own passes even with the
broken holder. A full module run sets `spark.testing` through
`HoodieSparkSqlTestBase`'s static initializer, and with
`forkCount=1`/`reuseForks=true` that property is live process-wide for every
subsequent test class. That is why the failure shows up in a suite run but not
in isolation.
Commands used:
```bash
# Spark 3.5 / Scala 2.12
mvn -o -pl hudi-spark-datasource/hudi-spark test
-Dtest=TestPartitionBucketIndexSupport
# Spark 4.0 / Scala 2.13
mvn clean install -Dscala-2.13 -Dspark4.0 -DskipTests=true -pl
hudi-spark-datasource/hudi-spark -am
SPARK_TESTING=1 mvn test-compile surefire:test -Dscala-2.13 -Dspark4.0 \
-pl hudi-spark-datasource/hudi-spark -Dtest=TestPartitionBucketIndexSupport
```
`scalastyle:check`, `checkstyle:check` and `apache-rat:check` pass on the
module.
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
