Repository navigation
fix: Read None elements of UUID and Decimal lists as None - #7039
Open
spicysharkco wants to merge 1 commit into
Open
spicysharkco wants to merge 1 commit into
spicysharkco wants to merge 1 commit into
Conversation
A None element in a UUID_LIST, TIME_UUID_LIST or DECIMAL_LIST is written as "" (see _LIST_NONE_DEFAULTS), but feast_value_type_to_python_type passed every element to UUID() or Decimal(). The write succeeded and reading the row back raised ValueError or InvalidOperation. Read "" back as None, as the unix timestamp list reader already does for its null sentinel. Signed-off-by: spicysharkco <321699413+spicysharkco@users.noreply.github.com>
This branch has not been deployed
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.
What this PR does / why we need it:
A
Noneinside aUUID_LIST,TIME_UUID_LISTorDECIMAL_LISTis written to the proto as""(_LIST_NONE_DEFAULTSintype_map.py, because a repeated string field cannot holdNone).feast_value_type_to_python_typethen callsUUID("")orDecimal("")on it. So the write works and the read fails:With this change the same call returns
{'u': [[UUID('...'), None]], 'dec': [[Decimal('1.5'), None]]}. The reader now maps""toNonefor the three list types, the way theunix_timestamp_list_valbranch maps itsNULL_TIMESTAMP_INT_VALUEsentinel toNone. The scalar and set branches are not touched.Cause: the list readers and the
""default.Which issue(s) this PR fixes:
No issue, found while checking list conversions in
type_map.py.Checks
git commit -s)Testing Strategy
Added
test_uuid_list_with_none_roundtrip,test_time_uuid_list_with_none_roundtripandtest_decimal_list_with_none_roundtriptosdk/python/tests/unit/test_type_map.py. All three fail on master with the errors above and pass with the fix:pytest sdk/python/tests/unit/test_type_map.py -k with_none_roundtripgives 3 passed. The rest of the file passes apart fromTestSparkNativeTypeValidation(needspyspark).ruff checkandruff format --checkare clean on both files.