Repository navigation
Conversation
_build_partition_predicate used EqualTo for every non-null partition value, and expression literals reject NaN, so delete, overwrite and dynamic_partition_overwrite failed with "Cannot create expression literal from NaN" whenever they touched a NaN identity partition. Use IsNaN for a NaN value, next to the existing IsNull branch for None. Closes apache#4277 Generated-by: Grok Bot (AI coding assistant)
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
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.
Closes #4277
Rationale for this change
Transaction._build_partition_predicateemitsIsNullfor aNonepartition value andEqualTofor anything else. Expression literals reject NaN, so as soon as a delete or overwrite touches a NaN identity partition it fails withValueError: Cannot create expression literal from NaN.This affectsdelete()(both dropping and rewriting the file),overwrite(),upsertanddynamic_partition_overwrite. Both callers (_build_delete_files_partition_predicateanddynamic_partition_overwrite) go through this helper.This PR takes the narrow fix suggested in the issue: a float NaN partition value now becomes
IsNaN(Reference(field)), next to the existingIsNullbranch.Are these changes tested?
Yes.
test_delete_and_overwrite_nan_identity_partitionintests/catalog/test_catalog_behaviors.pyruns against each catalog fixture and covers the cases from the issue on adoubleidentity partition:delete("id = 3"), which rewrites the NaN filedelete("value is nan"), which drops the NaN filedynamic_partition_overwritewith NaN incoming dataoverwriteWithout the fix it fails with the NaN literal error.
Ran locally:
pytest tests/ -m unit --ignore=tests/integration: 4475 passed. The only failures are 4 Kerberos tests intest_hive.py, which also fail onmainhere because a Kerberos module is missing in my environment.prek(ruff, ruff-format, mypy, codespell) on the changed filesAre there any user-facing changes?
Yes, a bug fix: deletes and overwrites on tables with a NaN identity partition no longer raise.
This change was prepared with an AI coding assistant (Grok Bot) and tested locally as described above.