Move writes to Transaction #571
Conversation
|
|
||
|
|
||
| @pytest.mark.integration | ||
| def test_write_within_transaction(spark: SparkSession, session_catalog: Catalog, arrow_table_with_null: pa.Table) -> None: |
There was a problem hiding this comment.
Shall we cover overwrite in the test as well?
There was a problem hiding this comment.
LGTM! Thanks @syun64 for taking this and @jqin61 for reviewing!
Continue the discussion #498 (comment) here:
I agree that removing them from Table class can simplify the implementation and emphasize the concept of "transaction". It appears that these methods in the Table class serve as handy tools for users to perform simple updates or data insertion. Considering these methods were released in 0.6.0, how about we initiate an issue to gather feedback from our users? This could facilitate a discussion on whether these methods should be deprecated.
|
The methods on the table were added as shorthands indeed. I'm open to removing those methods and letting everything go through a transaction. This way we have a single way to do things, and it also emphasizes that it is a transaction and done in an atomic way. |
|
Thanks for working on this @syun64 and @HonahX for the review 🙌 |
As a followup from @HonahX 's suggestion on #498
This refactoring is required both for supporting CreateTableTransaction, and supporting static overwrites through delete + append