Upsert merge strategy for iceberg #2671
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for dlt-hub-docs canceled.
|
anuunchin
commented
May 23, 2025
sh-rp
requested changes
May 26, 2025
There was a problem hiding this comment.
Thanks for working on this! About your questions:
- You can use the suggested code and document the behavior. I would also update to
0.9.1, try to isolate the sigill in a very small code block (I get it too) and open an issue in pyicerg, they seem to have some kind of bug. - If you update to pyiceberg
0.9.1, you'll see the tests for append and replace also start failing, so they changed something there is unrelated to merging. I would also try to isolate this and see if you can figure out what provokes this error and wether this is a problem on our part or theirs. - Yes, please document (you already did) and fix the tests accordingly. What happens in delta? Do we need a code or docs update there?
| with table.update_schema() as update: | ||
| update.union_by_name(ensure_iceberg_compatible_arrow_schema(data.schema)) | ||
|
|
||
| if "parent" in schema: |
There was a problem hiding this comment.
I'm not sure about this child table loading strategy. I know you took it from delta, but it seems to me that the first unique column will be the _dlt_id which will always be new since it is generated in the normalized step and thus the merge condition is never met and we could just append. But maybe let's leave it like this for now.
There was a problem hiding this comment.
upsert is using deterministic row_key that is computed from primary_key if the root table
anuunchin
force-pushed
the
feat/2549-upsert-filesystem-iceberg
branch
2 times, most recently
from
8cd57d7 to
93cbfee
Compare
rudolfix
requested changes
Jun 2, 2025
| with table.update_schema() as update: | ||
| update.union_by_name(ensure_iceberg_compatible_arrow_schema(data.schema)) | ||
|
|
||
| if "parent" in schema: |
There was a problem hiding this comment.
upsert is using deterministic row_key that is computed from primary_key if the root table
anuunchin
force-pushed
the
feat/2549-upsert-filesystem-iceberg
branch
2 times, most recently
from
811ed93 to
7dfec68
Compare
rudolfix
previously approved these changes
Jun 4, 2025
anuunchin
force-pushed
the
feat/2549-upsert-filesystem-iceberg
branch
from
7dfec68 to
c17d230
Compare
anuunchin
force-pushed
the
feat/2549-upsert-filesystem-iceberg
branch
from
c17d230 to
b8f57be
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR enables the "upsert" merge write disposition for Iceberg. The functionality is similar to how delta works. Underneath, Iceberg's upsert table is used.
Related Issues
Additional Context (for failing tests)
The test
test_merge_on_keys_in_schema_nested_hintswould likely fail with maximum recursion error, as Iceberg's upsert has a limitation mentioned here. This is apparently resolved in pyiceberg 0.9.1, but I'm still getting a sigill error with 0.9.1 and maximum recursion error with 0.9.0, the reason for which I'm unsure about.The test
test_table_format_schema_evolutionis failing withData type struct<a: int64> is not supported in join non-key field jsonbecause iceberg's upsert uses arrow's hash join which doesn't handle nested types. The issue was addressed with this PR in pyiceberg 0.9.1, but with 0.9.1 I get a different error sayingUnexpected physical type FIXED_LEN_BYTE_ARRAY for decimal(6, 4), expected INT32. The latter issue was also addressed in this PR in pyiceberg, but it's not yet released. Therefore, the issue should go away with pyiceberg's next release. (I tested with the newest code and it works 👀 )The tests
test_pipeline_load_parquetandtest_table_format_child_tablesfail because iceberg's upsert strictly forbids duplicates in input data as well as target table - if the key columns are not unique inside the source it raises an error. Delta doesn't even seem to be working properly 👀 .