azwanzuharimi opened a new pull request, #4027: URL: https://github.com/apache/iceberg-python/pull/4027
Closes #3508 # Rationale for this change `Table.upsert` with two or more join columns builds one `Or(And(EqualTo, ...))` disjunct per key. PyArrow flattens that chain and recurses over it. From about 1,000 keys the C++ stack overflows and the process dies with SIGSEGV, SIGBUS or SIGILL. This happens in all three places that use the filter: the scan for matching rows, the insert filter and the overwrite filter. A balanced Or tree does not help, because PyArrow flattens it. This change keeps every filter flat: - `create_match_filter` returns one `In` per join column. For a composite key this can match more rows than the keys in the source, so it is a superset. - A new `upsert_util.exclude_keys` does the exact key match in Arrow with an anti join, in the same way `get_rows_to_update` does its inner join. The insert path uses it instead of the Arrow expression filter. - The overwrite filter is the flat filter on the updated keys. Before the overwrite, the transaction scans the rows that filter removes and appends back the rows whose key is not updated. Related: #3509 groups keys by prefix and stays exact, but still emits one disjunct per distinct prefix. #3420 bounds the scan filter but keeps the exact filter for the insert and overwrite paths. Results on macOS, pyarrow 25.0.1, 100,000 target rows, 20,000 upsert rows (10,000 updates and 10,000 inserts), correctness check reads the table back and compares every row. rc 138 is a stack overflow crash. | case | main | #3509 | #3420 | this PR | |---|---|---|---|---| | a: 2 columns, one with ~10 values | rc 138 | pass 0.3 s | rc 138 | pass 0.3 s | | b: 2 columns, all values unique | rc 138 | rc 138 | rc 138 | pass 0.5 s | | c: 3 columns, all values unique | rc 138 | rc 138 | rc 138 | pass 0.7 s | | d: null in a key column | TypeError | TypeError | TypeError | TypeError | | e: keys match nothing | pass | pass | pass | pass | | f: partitioned (day, bucket[16]) | rc 138 | rc 138 | rc 138 | pass 32 s | | control: 1 column | pass | pass | pass | pass | Case d rejects null keys on every branch. That is not changed here. Cost for a composite key: rows that share each key column value with an updated key, but are not updated themselves, are rewritten unchanged, and the overwrite reads the affected files one extra time. Single column keys are not changed. ## Are these changes tested? Yes. `tests/table/test_upsert.py` adds: - `test_create_match_filter_composite_key_is_flat` - `test_upsert_composite_key_keeps_rows_outside_source_keys`: every key column value of the source exists in the target, but only some tuples do. Guards against over deletion. - `test_upsert_composite_key_large_batch`: 5,000 key tuples. Crashes pytest with exit code 132 on main. All 4239 unit tests pass. ## Are there any user-facing changes? No API change. An upsert on a composite key can add one extra APPEND snapshot when the overwrite filter removes rows that are not updated. https://claude.ai/code/session_01DT6giqoRgXQtohFLVKkGLm -- 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]
