From 8b6300cad2dbd1e72ee4542c02ce40583828823b Mon Sep 17 00:00:00 2001 From: David Mollitor Date: Fri, 11 Sep 2026 13:17:42 +0000 Subject: [PATCH] [MINOR][CORE][SQL] Inline argument checks and drop the network.util.JavaUtils dependency in ReadAheadInputStream and VectorizedDeltaBinaryPackedReader `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: JavaUtils.checkArgument(bufferSizeInBytes > 0, "bufferSizeInBytes should be greater than 0, but the value is " + bufferSizeInBytes); This has two problems: - The failure message is built eagerly on every call and then thrown away on the (overwhelmingly common) success path. - 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 (a stray `%` in the interpolated value would throw). Replace each call with an inline `if (!cond) { throw new IllegalArgumentException(...); }`, which builds the message only on failure and removes the `network.util.JavaUtils` import from both files. The exception type and message text are unchanged, and the inline form matches the `if (...) throw new ParquetDecodingException(...)` style already used in `VectorizedDeltaBinaryPackedReader`. Co-authored-by: Isaac --- .../org/apache/spark/io/ReadAheadInputStream.java | 7 ++++--- .../parquet/VectorizedDeltaBinaryPackedReader.java | 13 ++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/spark/io/ReadAheadInputStream.java b/core/src/main/java/org/apache/spark/io/ReadAheadInputStream.java index 0a1f51154a6d1..56461e6c3e5e6 100644 --- a/core/src/main/java/org/apache/spark/io/ReadAheadInputStream.java +++ b/core/src/main/java/org/apache/spark/io/ReadAheadInputStream.java @@ -33,7 +33,6 @@ import org.apache.spark.internal.SparkLoggerFactory; import org.apache.spark.internal.LogKeys; import org.apache.spark.internal.MDC; -import org.apache.spark.network.util.JavaUtils; import org.apache.spark.util.ThreadUtils; /** @@ -104,8 +103,10 @@ public class ReadAheadInputStream extends InputStream { */ public ReadAheadInputStream( InputStream inputStream, int bufferSizeInBytes) { - JavaUtils.checkArgument(bufferSizeInBytes > 0, - "bufferSizeInBytes should be greater than 0, but the value is " + bufferSizeInBytes); + if (bufferSizeInBytes <= 0) { + throw new IllegalArgumentException( + "bufferSizeInBytes should be greater than 0, but the value is " + bufferSizeInBytes); + } activeBuffer = ByteBuffer.allocate(bufferSizeInBytes); readAheadBuffer = ByteBuffer.allocate(bufferSizeInBytes); this.underlyingInputStream = inputStream; diff --git a/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedDeltaBinaryPackedReader.java b/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedDeltaBinaryPackedReader.java index c04fb49f1addf..1964e5256e8cf 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedDeltaBinaryPackedReader.java +++ b/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedDeltaBinaryPackedReader.java @@ -26,7 +26,6 @@ import org.apache.parquet.column.values.bitpacking.Packer; import org.apache.parquet.io.ParquetDecodingException; -import org.apache.spark.network.util.JavaUtils; import org.apache.spark.sql.catalyst.util.RebaseDateTime; import org.apache.spark.sql.execution.datasources.DataSourceUtils; import org.apache.spark.sql.execution.vectorized.WritableColumnVector; @@ -89,8 +88,10 @@ public class VectorizedDeltaBinaryPackedReader extends VectorizedReaderBase { @Override public void initFromPage(int valueCount, ByteBufferInputStream in) throws IOException { - JavaUtils.checkArgument(valueCount >= 1, - "Page must have at least one value, but it has " + valueCount); + if (valueCount < 1) { + throw new IllegalArgumentException( + "Page must have at least one value, but it has " + valueCount); + } this.in = in; // Read the header this.blockSizeInValues = BytesUtils.readUnsignedVarInt(in); @@ -105,8 +106,10 @@ public void initFromPage(int valueCount, ByteBufferInputStream in) throws IOExce + miniBlockNumInABlock + " (block size in values: " + blockSizeInValues + ")"); } double miniSize = (double) blockSizeInValues / miniBlockNumInABlock; - JavaUtils.checkArgument(miniSize % 8 == 0, - "miniBlockSize must be multiple of 8, but it's " + miniSize); + if (miniSize % 8 != 0) { + throw new IllegalArgumentException( + "miniBlockSize must be multiple of 8, but it's " + miniSize); + } this.miniBlockSizeInValues = (int) miniSize; // True value count. May be less than valueCount because of nulls this.totalValueCount = BytesUtils.readUnsignedVarInt(in);