rambleraptor commented on code in PR #4345:
URL: https://github.com/apache/iceberg-python/pull/4345#discussion_r4541601334
##########
tests/table/test_commit_retry.py:
##########
@@ -207,6 +207,47 @@ def
test_concurrent_append_delete_raises_validation_exception(catalog: Catalog)
tbl2.delete("x == 1")
+def
test_concurrent_append_case_insensitive_delete_retries_successfully(catalog:
Catalog) -> None:
Review Comment:
I think we should have some additional tests, since we're not testing all of
the new code paths. Some ideas:
- a snapshot-isolation variant (write.delete.isolation-level=snapshot, a
concurrent delete, then the case-insensitive delete)
- Some kind of test where we set case_sensitive = False
- Some kind of mixed-case-schema version of the existing
_added_delete_files / _validate_no_new_delete_files unit tests in
tests/table/test_validate.py? PyIceberg can't write delete files itself, so
those unit tests are the only way to reach this path, but this isn't the ideal
way to write this test.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
