Copilot commented on code in PR #3743: URL: https://github.com/apache/iceberg-python/pull/3743#discussion_r3700093428
########## mkdocs/docs/api.md: ########## @@ -311,8 +311,8 @@ Next, write the data to the table. Both `append` and `overwrite` produce the sam <!-- prettier-ignore-start --> -!!! note inline end "Fast append" - PyIceberg defaults to the [fast append](https://iceberg.apache.org/spec/#snapshots) to minimize the amount of data written. This enables fast commit operations, reducing the possibility of conflicts. The downside of the fast append is that it creates more metadata than a merge commit. [Compaction is planned](https://github.com/apache/iceberg-python/issues/270) and will automatically rewrite all the metadata when a threshold is hit, to maintain performant reads. +!!! note inline end "Merge and fast append" + PyIceberg defaults to [merge append](https://iceberg.apache.org/spec/#snapshots), which automatically merges manifests when the configured threshold is reached to keep metadata compact and reads performant. Fast append remains available by setting the table property `commit.manifest-merge.enabled` to `False`. Fast append minimizes the metadata written during a commit, enabling faster commits and reducing the possibility of conflicts, but accumulates more manifest metadata over time. Review Comment: This note explains the new default and how to opt into fast append, but it doesn’t mention that changing the library default can affect existing tables that don’t have `commit.manifest-merge.enabled` set in table metadata. Adding a short upgrade/pinning note would make the behavior change clearer to users. ########## mkdocs/docs/configuration.md: ########## @@ -99,12 +99,12 @@ Iceberg tables support table properties to configure table behavior. | ------------------------------------ | ------------------- | ------------- | ----------------------------------------------------------- | | `commit.manifest.target-size-bytes` | Size in bytes | 8388608 (8MB) | Target size when merging manifest files | | `commit.manifest.min-count-to-merge` | Number of manifests | 100 | Minimum number of manifests to accumulate before merging | -| `commit.manifest-merge.enabled` | Boolean | False | Controls whether to automatically merge manifests on writes | +| `commit.manifest-merge.enabled` | Boolean | True | Controls whether to automatically merge manifests on writes | <!-- prettier-ignore-start --> -!!! note "Fast append" - Unlike Java implementation, PyIceberg default to the [fast append](api.md#write-to-a-table) and thus `commit.manifest-merge.enabled` is set to `False` by default. +!!! note "Append modes" + PyIceberg defaults to [merge append](api.md#write-to-a-table) to keep manifest metadata compact. Set `commit.manifest-merge.enabled` to `False` on a table to use fast append, which writes less metadata during each commit but accumulates more manifests over time. Review Comment: The default for `commit.manifest-merge.enabled` is being flipped, which means upgrading PyIceberg can change append behavior for any table that does not have this property explicitly set. Consider calling this out here so users know they can pin fast/merge append behavior across upgrades by setting the property explicitly. -- 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]
