Skip to content

Commit ea77edc

Browse files
committed
Remove redundant CDC chunk-size/norm-level validation
PyArrow already validates these values itself (e.g. max_chunk_size must be greater than min_chunk_size) and raises a clear OSError, and iceberg-rust's equivalent ParquetWriterBuilder::from_table_properties doesn't duplicate this validation either -- defer to PyArrow instead of maintaining a second, slightly different copy of the same checks.
1 parent 0d66185 commit ea77edc

2 files changed

Lines changed: 30 additions & 58 deletions

File tree

pyiceberg/io/pyarrow.py

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2968,35 +2968,24 @@ def _get_parquet_writer_kwargs(table_properties: Properties) -> dict[str, Any]:
29682968
default=TableProperties.PARQUET_CDC_ENABLED_DEFAULT,
29692969
):
29702970
_require_pyarrow_version("21.0.0", "Parquet content-defined chunking")
2971-
min_chunk_size = property_as_int(
2972-
properties=table_properties,
2973-
property_name=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE,
2974-
default=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE_DEFAULT,
2975-
)
2976-
max_chunk_size = property_as_int(
2977-
properties=table_properties,
2978-
property_name=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE,
2979-
default=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE_DEFAULT,
2980-
)
2981-
norm_level = property_as_int(
2982-
properties=table_properties,
2983-
property_name=TableProperties.PARQUET_CDC_NORM_LEVEL,
2984-
default=TableProperties.PARQUET_CDC_NORM_LEVEL_DEFAULT,
2985-
)
2986-
if min_chunk_size is not None and min_chunk_size <= 0:
2987-
raise ValueError(f"{TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE} must be greater than 0, got {min_chunk_size}")
2988-
if max_chunk_size is not None and min_chunk_size is not None and max_chunk_size <= min_chunk_size:
2989-
raise ValueError(
2990-
f"{TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE} ({max_chunk_size}) must be greater than "
2991-
f"{TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE} ({min_chunk_size})"
2992-
)
2993-
if norm_level is not None and norm_level < 0:
2994-
raise ValueError(f"{TableProperties.PARQUET_CDC_NORM_LEVEL} must be greater than or equal to 0, got {norm_level}")
2995-
2971+
# PyArrow itself validates these values (e.g. max-chunk-size > min-chunk-size) and raises a
2972+
# clear OSError, so there's no need to duplicate that validation here.
29962973
parquet_writer_kwargs["use_content_defined_chunking"] = {
2997-
"min_chunk_size": min_chunk_size,
2998-
"max_chunk_size": max_chunk_size,
2999-
"norm_level": norm_level,
2974+
"min_chunk_size": property_as_int(
2975+
properties=table_properties,
2976+
property_name=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE,
2977+
default=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE_DEFAULT,
2978+
),
2979+
"max_chunk_size": property_as_int(
2980+
properties=table_properties,
2981+
property_name=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE,
2982+
default=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE_DEFAULT,
2983+
),
2984+
"norm_level": property_as_int(
2985+
properties=table_properties,
2986+
property_name=TableProperties.PARQUET_CDC_NORM_LEVEL,
2987+
default=TableProperties.PARQUET_CDC_NORM_LEVEL_DEFAULT,
2988+
),
30002989
}
30012990

30022991
return parquet_writer_kwargs

tests/io/test_pyarrow.py

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5499,36 +5499,19 @@ def test_get_parquet_writer_kwargs_cdc_enabled_unsupported_pyarrow_version(monke
54995499
_get_parquet_writer_kwargs({TableProperties.PARQUET_CDC_ENABLED: "true"})
55005500

55015501

5502-
@pytest.mark.parametrize(
5503-
"table_properties,match",
5504-
[
5505-
(
5506-
{
5507-
TableProperties.PARQUET_CDC_ENABLED: "true",
5508-
TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE: "0",
5509-
},
5510-
"min-chunk-size must be greater than 0",
5511-
),
5512-
(
5513-
{
5514-
TableProperties.PARQUET_CDC_ENABLED: "true",
5515-
TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE: "8192",
5516-
TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE: "4096",
5517-
},
5518-
"max-chunk-size .* must be greater than .*min-chunk-size",
5519-
),
5520-
(
5521-
{
5522-
TableProperties.PARQUET_CDC_ENABLED: "true",
5523-
TableProperties.PARQUET_CDC_NORM_LEVEL: "-1",
5524-
},
5525-
"norm-level must be greater than or equal to 0",
5526-
),
5527-
],
5528-
)
5529-
def test_get_parquet_writer_kwargs_cdc_invalid_properties(table_properties: dict[str, str], match: str) -> None:
5530-
with pytest.raises(ValueError, match=match):
5531-
_get_parquet_writer_kwargs(table_properties)
5502+
def test_get_parquet_writer_kwargs_cdc_invalid_chunk_sizes_raises_from_pyarrow() -> None:
5503+
"""PyArrow validates min/max chunk sizes itself; pyiceberg doesn't duplicate that check."""
5504+
kwargs = _get_parquet_writer_kwargs(
5505+
{
5506+
TableProperties.PARQUET_CDC_ENABLED: "true",
5507+
TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE: "8192",
5508+
TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE: "4096",
5509+
}
5510+
)
5511+
table = pa.table({"id": pa.array([1, 2, 3], type=pa.int32())})
5512+
with pytest.raises(OSError, match="max_chunk_size must be greater than min_chunk_size"):
5513+
with pq.ParquetWriter(pa.BufferOutputStream(), table.schema, **kwargs) as writer:
5514+
writer.write_table(table)
55325515

55335516

55345517
def test_write_file_with_content_defined_chunking_enabled(tmp_path: Path) -> None:

0 commit comments

Comments
 (0)