rangareddy commented on issue #17268:
URL: https://github.com/apache/hudi/issues/17268#issuecomment-5508894667
Followed up on this against `master` at `a2788eef922e`. The TODO in the
description is **not** stale, and I can now say exactly why it is still
unreachable and what is needed to reach it.
### The TODO path is confirmed dead, by measurement
I put a print statement inside the bootstrap branch of
`shouldValidatePartialRead` and ran the existing bootstrap test:
```
mvn test -pl hudi-spark-datasource/hudi-spark \
-Dtest='TestHoodieFileGroupReaderOnSpark#testReadFileGroupInBootstrapMergeOnReadTable'
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
branch-reached hits: 0
```
The test passes and the branch is entered zero times, so the comment is
accurate rather than left-over.
### Why the bootstrap test does not reach it
https://github.com/apache/hudi/blob/a2788eef922e2375432ead3fe50af7fe5f55faf7/hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java#L1122-L1133
The log-file check comes first and returns `true`. A **Merge-on-Read** file
slice always has log files, so for
`testReadFileGroupInBootstrapMergeOnReadTable` (`:771`, driven by the
`file-group-reader/bootstrap_data.zip` fixture) the method short-circuits on
the first `if` and the bootstrap branch below it is never evaluated. The
bootstrap read test that landed under HUDI-8632 is therefore necessary but not
sufficient for the second half of this ticket.
### What would actually exercise it
A **bootstrap Copy-on-Write** read test: a file slice with a bootstrap base
file present and no log files, which is the only shape that falls through the
first check into the bootstrap branch. That likely means a COW bootstrap
fixture alongside the existing MOR `bootstrap_data.zip`, since the current one
is MOR.
So the remaining scope on this ticket is one specific missing test rather
than a general "add bootstrap coverage", which should make it considerably
cheaper to finish than it looks. Worth noting for whoever picks it up that the
assertion guarded by that branch (`assertThrows(IllegalArgumentException.class,
...)` at `:1025`) has consequently never run for a bootstrap slice, so it is
unverified rather than known-good.
Read and measured on Spark 3.5 / Scala 2.12; the instrumentation was
reverted afterwards.
--
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]
