Skip to content

Commit 466ca6a

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 9c9fd58 commit 466ca6a

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
@@ -2874,35 +2874,24 @@ def _get_parquet_writer_kwargs(table_properties: Properties) -> dict[str, Any]:
28742874
default=TableProperties.PARQUET_CDC_ENABLED_DEFAULT,
28752875
):
28762876
_require_pyarrow_version("21.0.0", "Parquet content-defined chunking")
2877-
min_chunk_size = property_as_int(
2878-
properties=table_properties,
2879-
property_name=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE,
2880-
default=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE_DEFAULT,
2881-
)
2882-
max_chunk_size = property_as_int(
2883-
properties=table_properties,
2884-
property_name=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE,
2885-
default=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE_DEFAULT,
2886-
)
2887-
norm_level = property_as_int(
2888-
properties=table_properties,
2889-
property_name=TableProperties.PARQUET_CDC_NORM_LEVEL,
2890-
default=TableProperties.PARQUET_CDC_NORM_LEVEL_DEFAULT,
2891-
)
2892-
if min_chunk_size is not None and min_chunk_size <= 0:
2893-
raise ValueError(f"{TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE} must be greater than 0, got {min_chunk_size}")
2894-
if max_chunk_size is not None and min_chunk_size is not None and max_chunk_size <= min_chunk_size:
2895-
raise ValueError(
2896-
f"{TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE} ({max_chunk_size}) must be greater than "
2897-
f"{TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE} ({min_chunk_size})"
2898-
)
2899-
if norm_level is not None and norm_level < 0:
2900-
raise ValueError(f"{TableProperties.PARQUET_CDC_NORM_LEVEL} must be greater than or equal to 0, got {norm_level}")
2901-
2877+
# PyArrow itself validates these values (e.g. max-chunk-size > min-chunk-size) and raises a
2878+
# clear OSError, so there's no need to duplicate that validation here.
29022879
parquet_writer_kwargs["use_content_defined_chunking"] = {
2903-
"min_chunk_size": min_chunk_size,
2904-
"max_chunk_size": max_chunk_size,
2905-
"norm_level": norm_level,
2880+
"min_chunk_size": property_as_int(
2881+
properties=table_properties,
2882+
property_name=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE,
2883+
default=TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE_DEFAULT,
2884+
),
2885+
"max_chunk_size": property_as_int(
2886+
properties=table_properties,
2887+
property_name=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE,
2888+
default=TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE_DEFAULT,
2889+
),
2890+
"norm_level": property_as_int(
2891+
properties=table_properties,
2892+
property_name=TableProperties.PARQUET_CDC_NORM_LEVEL,
2893+
default=TableProperties.PARQUET_CDC_NORM_LEVEL_DEFAULT,
2894+
),
29062895
}
29072896

29082897
return parquet_writer_kwargs

‎tests/io/test_pyarrow.py‎

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

53365336

5337-
@pytest.mark.parametrize(
5338-
"table_properties,match",
5339-
[
5340-
(
5341-
{
5342-
TableProperties.PARQUET_CDC_ENABLED: "true",
5343-
TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE: "0",
5344-
},
5345-
"min-chunk-size must be greater than 0",
5346-
),
5347-
(
5348-
{
5349-
TableProperties.PARQUET_CDC_ENABLED: "true",
5350-
TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE: "8192",
5351-
TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE: "4096",
5352-
},
5353-
"max-chunk-size .* must be greater than .*min-chunk-size",
5354-
),
5355-
(
5356-
{
5357-
TableProperties.PARQUET_CDC_ENABLED: "true",
5358-
TableProperties.PARQUET_CDC_NORM_LEVEL: "-1",
5359-
},
5360-
"norm-level must be greater than or equal to 0",
5361-
),
5362-
],
5363-
)
5364-
def test_get_parquet_writer_kwargs_cdc_invalid_properties(table_properties: dict[str, str], match: str) -> None:
5365-
with pytest.raises(ValueError, match=match):
5366-
_get_parquet_writer_kwargs(table_properties)
5337+
def test_get_parquet_writer_kwargs_cdc_invalid_chunk_sizes_raises_from_pyarrow() -> None:
5338+
"""PyArrow validates min/max chunk sizes itself; pyiceberg doesn't duplicate that check."""
5339+
kwargs = _get_parquet_writer_kwargs(
5340+
{
5341+
TableProperties.PARQUET_CDC_ENABLED: "true",
5342+
TableProperties.PARQUET_CDC_MIN_CHUNK_SIZE: "8192",
5343+
TableProperties.PARQUET_CDC_MAX_CHUNK_SIZE: "4096",
5344+
}
5345+
)
5346+
table = pa.table({"id": pa.array([1, 2, 3], type=pa.int32())})
5347+
with pytest.raises(OSError, match="max_chunk_size must be greater than min_chunk_size"):
5348+
with pq.ParquetWriter(pa.BufferOutputStream(), table.schema, **kwargs) as writer:
5349+
writer.write_table(table)
53675350

53685351

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

0 commit comments

Comments
 (0)