Add support for write.data.path
#1611
Conversation
There was a problem hiding this comment.
Thanks for picking this up @Fokko! Looks great, just small comments 😄
mkdocs/docs/configuration.md
Outdated
| | `write.object-storage.enabled` | Boolean | True | Enables the [`ObjectStoreLocationProvider`](configuration.md#object-store-location-provider) that adds a hash component to file paths. Note: the default value of `True` differs from Iceberg's Java implementation | | ||
| | `write.object-storage.partitioned-paths` | Boolean | True | Controls whether [partition values are included in file paths](configuration.md#partition-exclusion) when object storage is enabled | | ||
| | `write.py-location-provider.impl` | String of form `module.ClassName` | null | Optional, [custom `LocationProvider`](configuration.md#loading-a-custom-location-provider) implementation | | ||
| | `write.data.path` | String pointing to location | ∅ | Sets the location where to write the data. If not set, it will use the table location postfixed with `data/`. | |
There was a problem hiding this comment.
Not sure about the best wording here - maybe something like:
| | `write.data.path` | String pointing to location | ∅ | Sets the location where to write the data. If not set, it will use the table location postfixed with `data/`. | | |
| | `write.data.path` | String referencing a location | ∅ | Sets the location under which data is written. If not set, the table location (`table.metadata.location`) postfixed with `data/` is used. | |
There was a problem hiding this comment.
Also, what about 'table location + /data' as the default instead of ∅ like in the Java docs?
There was a problem hiding this comment.
Also bringing attention to #1537 (comment) - it would be great to make the location providers docs consistent with write.data.path now.
iceberg-python/mkdocs/docs/configuration.md
Line 213 in 4a055d7
iceberg-python/mkdocs/docs/configuration.md
Line 242 in 4a055d7
I think these are the only places that require changing - they'll be write.data.path (and maybe for clarity we should reiterate there that this defaults to {location}/data so examples are clearer). Could we do that in this PR maybe? (Happy to follow-up with this otherwise)
There was a problem hiding this comment.
Thanks for the input here, I've changed some wording en reworked some of the sections. Let me know what you think 👍
There was a problem hiding this comment.
Looks great - thank you!
| Paths still contain partitions just before the file name, in Hive-style, and a `data` directory beneath the table's location, | ||
| in a similar manner to the [`SimpleLocationProvider`](configuration.md#simple-location-provider). For example, a table | ||
| partitioned over a string column `category` might have a data file with location: (note the additional binary directories) | ||
| Paths still are also prefixed by `{location}/data/`, where `location` comes from the [table metadata](https://iceberg.apache.org/spec/#table-metadata-fields), in a similar manner to the [`SimpleLocationProvider`](configuration.md#simple-location-provider). This can be overridden by setting [`write.data.path` table configuration](#write-options). |
There was a problem hiding this comment.
| Paths still are also prefixed by `{location}/data/`, where `location` comes from the [table metadata](https://iceberg.apache.org/spec/#table-metadata-fields), in a similar manner to the [`SimpleLocationProvider`](configuration.md#simple-location-provider). This can be overridden by setting [`write.data.path` table configuration](#write-options). | |
| Paths are still prefixed by `{location}/data/`, where `location` comes from the [table metadata](https://iceberg.apache.org/spec/#table-metadata-fields), in a similar manner to the [`SimpleLocationProvider`](configuration.md#simple-location-provider). This can be overridden by setting [`write.data.path` table configuration](#write-options). |
Relates to #1492