IO: Absorb upsert PyArrow logic into io/pyarrow.py - #3813
Conversation
Move PyArrow-specific table operations (joins, group_by, duplicate detection, row comparison) from table/upsert_util.py into pyiceberg/io/pyarrow.py. The upsert_util module now delegates to helpers in io/pyarrow rather than importing pyarrow directly. This consolidates PyArrow logic behind the io/pyarrow module boundary, which is a precondition for the decomposition proposed in apache#3737. Fixes apache#3812 (PR A)
rambleraptor
left a comment
There was a problem hiding this comment.
This is just a simple code reorg, so it looks fine.
I think it's good to consolidate our PyArrow logic, especially since PyArrow is an optional dependency.
| Or, | ||
| ) | ||
| from pyiceberg.io.pyarrow import ( | ||
| _upsert_get_rows_to_update, |
There was a problem hiding this comment.
These shouldn't be private if we're importing them elsewhere.
There was a problem hiding this comment.
Hi @rambleraptor, thanks for catching that, made the corresponding changes.
rambleraptor
left a comment
There was a problem hiding this comment.
Just a code re-org, so this looks good. Thanks for doing this!
| if TYPE_CHECKING: | ||
| import pyarrow as pa |
There was a problem hiding this comment.
Since we still pass in PyArrow as arguments, should we dissolve this file completely into pyarrow.py? We could import those functions here and emit a deprecation warning. I can see folks importing these functions.
There was a problem hiding this comment.
That's elegant. I think that's a great way to consolidate everything without breaking any one.
Bonus, we get to remove the typechecking clause. I really don't like those.
Summary
Move PyArrow-specific table operations (joins, group_by, duplicate detection, row comparison) from
table/upsert_util.pyintopyiceberg/io/pyarrow.py. Theupsert_utilmodule now delegates to helpers inio/pyarrowrather than importingpyarrowdirectly.Changes
pyiceberg/io/pyarrow.py:_upsert_unique_keys,_upsert_has_duplicate_rows,_upsert_get_rows_to_updatepyiceberg/table/upsert_util.py: removed directimport pyarrow/from pyarrow import compute, now imports helpers frompyiceberg.io.pyarrowWhat stays the same
from pyiceberg.table.upsert_util import ...imports continue to workWhy
This consolidates PyArrow logic behind the
io/pyarrowmodule boundary.table/upsert_util.pywas the only file inpyiceberg/table/with a top-levelimport pyarrowand inline PyArrow compute operations (joins, aggregations, etc.). Moving these operations intoio/pyarrow.pymeans all runtime PyArrow usage routes through one module, which is a precondition for the decomposition proposed in #3737.See #3812 for the full audit and tracking.
Related