apache
Bump to PyArrow 17.0.0
Vote has been passed: https://lists.apache.org/thread/mnzdpwzhctx6yrjl16zn8hl7pcxxt575
Amazing. Given that this issue (https://github.com/apache/iceberg-python/issues/936) also requires 17.0.0 for the fix, maybe it's right for us to move forward with onboarding 17.0.0
@syun64 It has been released, and I've updated the lockfile
Awesome :) we live in exciting times 🎉
It looks like the 3.9 artifacts are missing:
- Devlist: https://lists.apache.org/thread/xpvsr3qvc86nbzp1sxxx9vp41wbo6d02
- Github tracking issue: https://github.com/apache/arrow/issues/43289
The missing wheels and source distribution for pyarrow 17.0.0 have been uploaded to PyPI. Sorry for the inconvenience.
@syun64 @HonahX @kevinjqliu This provides a nice cleanup of the types (and probably also a speed-up), the downside is that we have to raise the lower bound to PyArrow 17. PTAL
btw, are dependabot PRs automatically merged? It seems it updated pyarrow (https://github.com/apache/iceberg-python/commit/4282d2f342167255bb65034a12131ce8fe6a9c04)
btw, are dependabot PRs automatically merged? It seems it updated pyarrow (https://github.com/apache/iceberg-python/commit/4282d2f342167255bb65034a12131ce8fe6a9c04)
No, we still have to merge them by hand https://github.com/apache/iceberg-python/pull/937. The good thing is that the current codebase runs with 17 :)
@syun64 @HonahX @kevinjqliu This provides a nice cleanup of the types (and probably also a speed-up), the downside is that we have to raise the lower bound to PyArrow 17. PTAL
Great question @Fokko ... after thinking a lot about this the past week, here's my long answer organized by different topics of consideration
Benefits of 17.0.0
- Table Scan API
to_arrow()will now be able to infer the type correctly based on the encoding in a parquet file (instead of always reading as large types). If there's a discrepancy in the encodings across the files, this will result in some of the tables's data being casted to larger types. Forto_arrow_batches()this means that we will be reading the batches with types matching the encoding in the parquet file, and then always casting to large types ** - We will be able conform with the Iceberg Spec's physical type mapping for DecimalType: https://github.com/apache/iceberg-python/issues/936
User's ability to use PyIceberg in applications
- pyarrow 8.0.0 ~ pyarrow 17.0.0 all have the same single subdependency on numpy of the same version, which means it shouldn't be all that difficult for users to switch to a higher version of pyarrow in existing applications
- However, switching versions of a core dependency comes with the risk of introducing changes to the data underneath. Although the arrow representation under the hood won't change, I wonder if there'd still be subtle differences in the API args that would need to be considered carefully in a version update. I'd imagine that it would require a lot of effort for owners of existing Production applications to update the PyArrow version and QC their output artifacts as a pre-requisite for adding a PyIceberg dependency to their application (if they want to use pyarrow for scans and writes)
- This would also mean that there is only one version of PyArrow available for users to use with PyIceberg - there's some element of risk in having just one version of a package available to use. For example, what if there's a really bad issue with 17.0.0 that affects a specific use case?
** -> I'm of the impression that while this change seems to make sense from the perspective of preserving type or encoding correctness, it will actually result in a performance regression due to the fact that we will be reading most batches as small types, but having to cast them to large types (infrequently for pa.Table, but always for pa.RecordBatchReader). Another option is to always choose to cast to a small type instead in to_arrow_batch_reader
Based on these points, I'm leaning towards not aggressively increasing the lower bound to 17.0.0, at least for this minor release, but I'm very excited to hear what others think as well!
@syun64 already pointed to the cost/benefits of upgrading.
I lean more towards correctness than performance. What is the correctness issue if we do not upgrade? As I understand from the above, if the parquet file is of type string, we read it as large_string but write it as string again.
As for updating the minimum dependency to pyarrow 17.0.0, I would prefer to wait for the new arrow version to be baked for a time before we require all new versions of Pyiceberg to use it.
I also think the 0.7.0 release's feature set is getting massive. We can add this upgrade as a fast-follow release.
