fix: convert nano timestamps to nanoseconds in partition representation#3695
Open
mittalpk wants to merge 1 commit into
Open
fix: convert nano timestamps to nanoseconds in partition representation#3695mittalpk wants to merge 1 commit into
mittalpk wants to merge 1 commit into
Conversation
_to_partition_representation() had no handler for TimestampNanoType/ TimestamptzNanoType, so a datetime partition value passed through unconverted instead of being converted to nanoseconds since epoch (the same way TimestampType/TimestamptzType already convert to micros). The resulting datetime object would reach DataFile.partition where an int is expected, since TimestampNanoWriter calls write_int. Fixes apache#3652
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.
Summary
_to_partition_representation()inpyiceberg/partitioning.pyhas no registered handler forTimestampNanoType/TimestamptzNanoType. Adatetimepartition value for a nano-precision timestamp column passes through unconverted instead of being converted to nanoseconds since epoch, the same wayTimestampType/TimestamptzTypealready convert to microseconds.That unconverted
datetimeobject would reachDataFile.partition, whereTimestampNanoWritercallswrite_int— an int is expected, not adatetime.Fixes #3652
Changes
TimestampNanoType/TimestamptzNanoTypein_to_partition_representation, using the existingdatetime_to_nanos()frompyiceberg/utils/datetime.py(mirrors the existing micros handler exactly).test_to_partition_representation_timestamps(8 parametrized cases coveringNone, int passthrough, anddatetimeconversion for both the existing micros types and the new nano types) andtest_to_partition_representation_unrecognized_type_raises(confirms unrecognized input types raiseValueErrorfor both micros and nanos types — previously nano types silently fell through to the genericPrimitiveTypehandler, which returns the value unchanged rather than raising).Test plan
git stashthe fix inpartitioning.py, rerun — 3 of 9 new tests fail as expected) and pass post-fix.uv run pytest tests/test_transforms.py: 274 passed (up from 265 pre-fix, the +9 are the new tests); the 17 pre-existing failures (missingpyiceberg-coreextension in this environment) are unchanged before/after — confirmed identical failure set on unmodifiedmain.uv run pytest tests/table/test_partitioning.py: 19 passed / 11 pre-existing failures (same extension-availability cause), unchanged before/after.uv run ruff check/ruff format --check: clean on both changed files.uv run mypy pyiceberg/partitioning.py: no errors attributable to this file (pre-existing unrelated errors in other files are due to optional dependency stubs not installed in this environment).