You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "koenvo (via GitHub)" <gi...@apache.org> on 2025/04/23 12:26:59 UTC
[PR] Feat/simplify upsert [iceberg-python]
koenvo opened a new pull request, #1947:
URL: https://github.com/apache/iceberg-python/pull/1947
(no comment)
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Feat/simplify upsert [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1947:
URL: https://github.com/apache/iceberg-python/pull/1947#discussion_r2056167122
##########
tests/table/test_upsert.py:
##########
@@ -478,8 +485,8 @@ def test_upsert_with_duplicate_rows_in_table(catalog: Catalog) -> None:
schema=arrow_schema,
)
- with pytest.raises(ValueError, match="Target table has duplicate rows, aborting upsert"):
- _ = tbl.upsert(df)
+ # with pytest.raises(ValueError, match="Target table has duplicate rows, aborting upsert"):
+ # _ = tbl.upsert(df)
Review Comment:
We should be able to let this test pass pretty easily: https://github.com/apache/iceberg-python/blob/b85127e05b699f5e8f7acc1034b9f258fd209477/pyiceberg/table/upsert_util.py#L67
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Feat/simplify upsert [iceberg-python]
Posted by "koenvo (via GitHub)" <gi...@apache.org>.
koenvo closed pull request #1947: Feat/simplify upsert
URL: https://github.com/apache/iceberg-python/pull/1947
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Feat/simplify upsert [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1947:
URL: https://github.com/apache/iceberg-python/pull/1947#discussion_r2056158865
##########
tests/table/test_upsert.py:
##########
@@ -369,18 +369,25 @@ def test_upsert_with_identifier_fields(catalog: Catalog) -> None:
)
upd = tbl.upsert(df)
- expected_operations = [Operation.APPEND, Operation.OVERWRITE, Operation.APPEND, Operation.APPEND]
+ expected_operations = [Operation.APPEND, Operation.OVERWRITE, Operation.APPEND]
- assert upd.rows_updated == 1
+ assert upd.rows_updated == 2
assert upd.rows_inserted == 1
assert [snap.summary.operation for snap in tbl.snapshots() if snap.summary is not None] == expected_operations
- # This should be a no-op
+ # This will update all 3 rows
Review Comment:
I don't think it is very nice to update rows that should not be updated 🤔 If nothing changes, we should just skip over it.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Feat/simplify upsert [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1947:
URL: https://github.com/apache/iceberg-python/pull/1947#discussion_r2056158865
##########
tests/table/test_upsert.py:
##########
@@ -369,18 +369,25 @@ def test_upsert_with_identifier_fields(catalog: Catalog) -> None:
)
upd = tbl.upsert(df)
- expected_operations = [Operation.APPEND, Operation.OVERWRITE, Operation.APPEND, Operation.APPEND]
+ expected_operations = [Operation.APPEND, Operation.OVERWRITE, Operation.APPEND]
- assert upd.rows_updated == 1
+ assert upd.rows_updated == 2
assert upd.rows_inserted == 1
assert [snap.summary.operation for snap in tbl.snapshots() if snap.summary is not None] == expected_operations
- # This should be a no-op
+ # This will update all 3 rows
Review Comment:
I don't think it is very nice to update rows that should not be updated 🤔
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org