Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds support for PyArrow Parquet content-defined chunking (CDC) in the PyIceberg write path, exposing it via new table properties and guarding usage by the minimum supported PyArrow version.
Changes:
- Add new table properties for enabling/configuring Parquet CDC (enabled/min/max/norm-level).
- Wire CDC properties through
_get_parquet_writer_kwargs, including a shared_require_pyarrow_versionguard. - Add unit + integration-style tests and document the new properties.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/io/test_pyarrow.py | Adds tests for CDC kwargs generation, version gating, and end-to-end wiring into pq.ParquetWriter. |
| pyiceberg/table/init.py | Introduces new CDC-related TableProperties constants and defaults. |
| pyiceberg/io/pyarrow.py | Adds _require_pyarrow_version helper, reuses it for Azure FS guard, and forwards CDC options to PyArrow writer kwargs. |
| mkdocs/docs/configuration.md | Documents new CDC table properties and their defaults/requirements. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
2a93c6d to
d7f61aa
Compare
PyArrow's ParquetWriter has supported content-defined chunking natively since 21.0.0, producing stable page boundaries across appends for content-addressable storage. Wire this through as write.parquet.content-defined-chunking.* table properties, mirroring the property names and defaults already used by iceberg-rust. PyArrow validates the chunk sizes itself and raises a clear error, so pyiceberg doesn't duplicate those checks. Requesting CDC on an older PyArrow raises an ImportError from a shared _require_pyarrow_version helper, which also replaces the existing Azure filesystem guard.
The CDC tests raised ImportError on pyarrow 18-20, which pyproject still declares as supported.
d7f61aa to
390e404
Compare
rambleraptor
left a comment
rambleraptor
left a comment
There was a problem hiding this comment.
Whoops, I guess I did a review on this earlier and never sent it out. Apologies for that!
| return self | ||
|
|
||
|
|
||
| def _require_pyarrow_version(min_version: str, feature: str) -> None: |
There was a problem hiding this comment.
Idea (this is out-of-scope): What if we had a require_pyarrow_with_version method that checks if pyarrow exists and optionally checks the version?
We do pyarrow checks across the codebase.
There was a problem hiding this comment.
Sounds like a good idea, creating a follow-up PR for this.
| f"pyarrow version >= {MIN_PYARROW_VERSION_SUPPORTING_AZURE_FS} required for AzureFileSystem support, " | ||
| f"but found version {pyarrow.__version__}." | ||
| ) | ||
| _require_pyarrow_version("20.0.0", "AzureFileSystem support") |
Ohh, no worries :) |
Reopens #3608, which was closed by the stale bot. Rebased on current
main; GitHub refused to reopen the original PR after the branch was updated.Rationale for this change
PyArrow's
ParquetWriterhas natively supported content-defined chunking (CDC) since 21.0.0, producing stable page boundaries across appends (useful for content-addressable storage / dedup). PyIceberg's write path already funnels through a single kwargs builder (_get_parquet_writer_kwargs), so this wires CDC through aswrite.parquet.content-defined-chunking.*table properties, mirroring the property names and defaults iceberg-rust already uses for cross-engine consistency. Apyarrow>=21.0.0version guard raises a clearImportErrorif CDC is requested on an older PyArrow (extracted into a shared_require_pyarrow_versionhelper, reused by the existing Azure-filesystem version guard).Are these changes tested?
Yes: unit tests for
_get_parquet_writer_kwargs(disabled by default, enabled with defaults, enabled with custom values, unsupported PyArrow version) and an integration-style test that writes a table with CDC enabled end-to-end and reads it back.Are there any user-facing changes?
Yes: four new table properties (
write.parquet.content-defined-chunking.enabled,.min-chunk-size,.max-chunk-size,.norm-level), documented inconfiguration.md.