david-mollitor-db opened a new pull request, #58739:
URL: https://github.com/apache/spark/pull/58739
### What changes were proposed in this pull request?
`ReadAheadInputStream` (core) and `VectorizedDeltaBinaryPackedReader`
(sql/core) validated their
arguments through
`org.apache.spark.network.util.JavaUtils.checkArgument(check, msg, args...)`,
passing an already-concatenated message string, e.g.:
```java
JavaUtils.checkArgument(bufferSizeInBytes > 0,
"bufferSizeInBytes should be greater than 0, but the value is " +
bufferSizeInBytes);
```
This PR replaces each of the three call sites with an inline
`if (!cond) { throw new IllegalArgumentException(...); }` and removes the
now-unused
`org.apache.spark.network.util.JavaUtils` import from both files. The
exception type and message
text are unchanged.
### Why are the changes needed?
The `checkArgument(boolean, String, Object...)` overload is designed for
deferred `%s` templating
(`String.format(msg, args)` is evaluated only on failure). Passing an
already-concatenated string
misuses it in two ways:
- The failure message is built eagerly on every call and then discarded on
the (overwhelmingly
common) success path — `initFromPage` runs once per Parquet page.
- It pulls in a cross-package dependency on the shuffle/network module's
`JavaUtils` purely for a
one-line precondition, and passes an already-interpolated string as a
`String.format` template,
so a stray `%` in the interpolated value would throw a
`FormatFlagsConversionMismatchException`
(or similar) instead of the intended `IllegalArgumentException`.
The inline form builds the message only on failure, drops the cross-package
dependency, and in
`VectorizedDeltaBinaryPackedReader` matches the `if (...) throw new
ParquetDecodingException(...)`
style already used throughout that file.
### Does this PR introduce _any_ user-facing change?
No. The same `IllegalArgumentException` with the same message is thrown on
the same conditions.
### How was this patch tested?
Existing suites pass: `ReadAheadInputStreamSuite` (8 tests) and
`ParquetDeltaEncodingInteger` +
`ParquetDeltaEncodingLong` (32 tests). Checkstyle is clean on both `core`
and `sql/core`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8
This pull request and its description were written by Isaac.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
