Fokko commented on code in PR #1810:
URL: https://github.com/apache/iceberg-python/pull/1810#discussion_r2003124123
##########
pyiceberg/table/__init__.py:
##########
@@ -1198,10 +1198,11 @@ def upsert(
update_row_cnt = len(rows_to_update)
- # build the match predicate filter
- overwrite_mask_predicate =
upsert_util.create_match_filter(rows_to_update, join_cols)
+ if len(rows_to_update) > 0:
+ # build the match predicate filter
+ overwrite_mask_predicate =
upsert_util.create_match_filter(rows_to_update, join_cols)
- tx.overwrite(rows_to_update,
overwrite_filter=overwrite_mask_predicate)
+ tx.overwrite(rows_to_update,
overwrite_filter=overwrite_mask_predicate)
Review Comment:
Yes, that's a great suggestion. I was also looking into that, but there are
two caveats:
- Before computing all the manifests, we don't know if they are empty. The
delete manifests are lazily computed, so making sure that we short-circuit
earlier in the process, makes sure that we don't do all the work.
- There is also a behavioral aspect around it, for example, when you do
`tbl.append(df)` where `df` is empty, or `tbl.delete('x = 10')`, you may still
want to produce a snapshot to indicate that something happened. My opinion is
that it should result in a no-op, but Java creates a snapshot:
I believe @sungwy and I had a similar discussion.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
