Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public class ParquetProperties {
public static final boolean DEFAULT_SIZE_STATISTICS_ENABLED = true;

public static final boolean DEFAULT_PAGE_WRITE_CHECKSUM_ENABLED = true;
public static final double DEFAULT_PAGE_COMPRESS_THRESHOLD = 0.98;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this magic number? Is it better to use a smaller number like 0.9 or 0.85?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the suggestion linked here: #3344 (comment)


/**
* @deprecated This shared instance can cause thread safety issues when used by multiple builders concurrently.
Expand Down Expand Up @@ -121,6 +122,7 @@ public static WriterVersion fromString(String name) {
private final int statisticsTruncateLength;
private final boolean statisticsEnabled;
private final boolean sizeStatisticsEnabled;
private final double pageCompressThreshold;

// The expected NDV (number of distinct values) for each columns
private final ColumnProperty<Long> bloomFilterNDVs;
Expand Down Expand Up @@ -157,6 +159,8 @@ private ParquetProperties(Builder builder) {
this.statisticsTruncateLength = builder.statisticsTruncateLength;
this.statisticsEnabled = builder.statisticsEnabled;
this.sizeStatisticsEnabled = builder.sizeStatisticsEnabled;
this.pageCompressThreshold = builder.pageCompressThreshold;

this.bloomFilterNDVs = builder.bloomFilterNDVs.build();
this.bloomFilterFPPs = builder.bloomFilterFPPs.build();
this.bloomFilterEnabled = builder.bloomFilterEnabled.build();
Expand Down Expand Up @@ -327,6 +331,10 @@ public boolean getPageWriteChecksumEnabled() {
return pageWriteChecksumEnabled;
}

public double pageCompressThreshold() {
return pageCompressThreshold;
}

public OptionalLong getBloomFilterNDV(ColumnDescriptor column) {
Long ndv = bloomFilterNDVs.getValue(column);
return ndv == null ? OptionalLong.empty() : OptionalLong.of(ndv);
Expand Down Expand Up @@ -415,7 +423,8 @@ public String toString() {
+ "Page row count limit to " + getPageRowCountLimit() + '\n'
+ "Writing page checksums is: " + (getPageWriteChecksumEnabled() ? "on" : "off") + '\n'
+ "Statistics enabled: " + statisticsEnabled + '\n'
+ "Size statistics enabled: " + sizeStatisticsEnabled;
+ "Size statistics enabled: " + sizeStatisticsEnabled + '\n'
+ "Page compress threshold: " + pageCompressThreshold;
String perColumn = "";
if (!columnCodecs.toString().equals(Objects.toString(columnCodecs.getDefaultValue()))) {
perColumn = "Per-column codecs: " + columnCodecs;
Expand Down Expand Up @@ -445,6 +454,7 @@ public static class Builder {
private int statisticsTruncateLength = DEFAULT_STATISTICS_TRUNCATE_LENGTH;
private boolean statisticsEnabled = DEFAULT_STATISTICS_ENABLED;
private boolean sizeStatisticsEnabled = DEFAULT_SIZE_STATISTICS_ENABLED;
private double pageCompressThreshold = DEFAULT_PAGE_COMPRESS_THRESHOLD;
private final ColumnProperty.Builder<Long> bloomFilterNDVs;
private final ColumnProperty.Builder<Double> bloomFilterFPPs;
private int maxBloomFilterBytes = DEFAULT_MAX_BLOOM_FILTER_BYTES;
Expand Down Expand Up @@ -511,6 +521,7 @@ private Builder(ParquetProperties toCopy) {
this.sizeStatisticsEnabled = toCopy.sizeStatisticsEnabled;
this.columnCodecs = ColumnProperty.builder(toCopy.columnCodecs);
this.columnCompressionLevels = ColumnProperty.builder(toCopy.columnCompressionLevels);
this.pageCompressThreshold = toCopy.pageCompressThreshold();
}

/**
Expand Down Expand Up @@ -833,6 +844,27 @@ 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 saves less than 2% of space, the data will not be compressed.
* A value of 0 disables data page compression and a value of 1 only skips compression when it
* increases the data size.
*
* @param threshold the compression ratio threshold, default is {@value #DEFAULT_PAGE_COMPRESS_THRESHOLD}
* @return this builder for method chaining
*/
public Builder withPageCompressThreshold(double threshold) {

@wgtmac wgtmac Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added validation

Preconditions.checkArgument(
Double.isFinite(threshold) && threshold >= 0.0 && threshold <= 1.0,
"Invalid page compression threshold: %s. It must be a finite value between 0.0 and 1.0.",
threshold);
this.pageCompressThreshold = threshold;
return this;
}

public ParquetProperties build() {
ParquetProperties properties = new ParquetProperties(this);
// we pass a constructed but uninitialized factory to ParquetProperties above as currently
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,36 @@ public void withCompressionCodec_nullCodec_throwsNullPointerException() {
NullPointerException.class, () -> ParquetProperties.builder().withCompressionCodec("col_a", null));
}

@Test
public void pageCompressThreshold_rejectsValuesOutsideRatioRange() {
assertThrows(IllegalArgumentException.class, () -> ParquetProperties.builder()
.withPageCompressThreshold(-0.01));
assertThrows(IllegalArgumentException.class, () -> ParquetProperties.builder()
.withPageCompressThreshold(1.01));
assertThrows(IllegalArgumentException.class, () -> ParquetProperties.builder()
.withPageCompressThreshold(Double.NaN));
assertThrows(IllegalArgumentException.class, () -> ParquetProperties.builder()
.withPageCompressThreshold(Double.POSITIVE_INFINITY));
}

@Test
public void pageCompressThreshold_acceptsRatioBoundaries() {
assertEquals(
0.0,
ParquetProperties.builder()
.withPageCompressThreshold(0.0)
.build()
.pageCompressThreshold(),
0.0);
assertEquals(
1.0,
ParquetProperties.builder()
.withPageCompressThreshold(1.0)
.build()
.pageCompressThreshold(),
0.0);
}

@Test
public void copyBuilder_preservesColumnCodecAndLevel() {
ParquetProperties original = ParquetProperties.builder()
Expand Down
Loading
Loading