wgtmac commented on code in PR #3368:
URL: https://github.com/apache/parquet-java/pull/3368#discussion_r3584228280
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ColumnChunkPageWriteStore.java:
##########
@@ -313,9 +316,14 @@ public void writePageV2(
boolean compressed = false;
BytesInput compressedData = BytesInput.empty();
if (data.size() > 0) {
- // TODO: decide if we compress
compressedData = compressor.compress(data);
compressed = true;
+ double compressionRatio = (double) compressedData.size() / data.size();
+ if (compressor.getCodecName() != CompressionCodecName.UNCOMPRESSED
+ && compressionRatio > pageCompressThreshold) {
Review Comment:
This fallback can break for one-shot `BytesInput` implementations.
`compressor.compress(data)` may consume `data` (for example stream-backed
input), and then the fallback assigns the already-consumed input back to
`compressedData` for CRC/writing. Please materialize/copy the original page
bytes before compression, or otherwise make the fallback use a replayable copy.
(Reviewed by Codex)
##########
parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java:
##########
@@ -833,6 +844,21 @@ public Builder withCompressionLevel(String columnPath,
Integer level) {
return this;
}
+ /**
+ * Sets the compression threshold for data pages, only effect for V2 pages.
+ *
+ * <p>When the compression ratio (compressed size / uncompressed size)
exceeds this threshold,
+ * the uncompressed data will be used instead. For example, with a
threshold of 0.98, if
+ * compression only saves 2% of space, the data will not be compressed.
+ *
+ * @param threshold the compression ratio threshold, default is {@value
#DEFAULT_PAGE_COMPRESS_THRESHOLD}
+ * @return this builder for method chaining
+ */
+ public Builder withPageCompressThreshold(double threshold) {
Review Comment:
Please validate the threshold or document the allowed range here. As
written, values like `NaN`, negative numbers, or `100` are accepted even though
the API describes this as a compressed/uncompressed ratio threshold. I would
either enforce a finite range (for example `0.0 <= threshold <= 1.0`) or
explicitly document the special meanings of out-of-range values. (Reviewed by
Codex)
--
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]
