fix: Remote Online Store Type Inference Error with All-NULL Columns#6063
fix: Remote Online Store Type Inference Error with All-NULL Columns#6063yuan1j wants to merge 2 commits intofeast-dev:masterfrom
Conversation
|
小军已收到
|
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
|
||
| if value_type == ValueType.UNKNOWN: | ||
| raise TypeError("Couldn't infer value type from empty value") | ||
| return [ProtoValue() for _ in values] |
There was a problem hiding this comment.
🔴 Silent data loss for empty-collection values (e.g., []) now treated as null instead of erroring
Replacing the TypeError with return [ProtoValue() for _ in values] silently converts legitimate empty-collection values (like []) into null ProtoValues. The _non_empty_value function at sdk/python/feast/type_map.py:1351-1360 treats empty lists/sets as "empty" (returns False), so when the only values are empty collections, sample is None, value_type stays UNKNOWN, and the new code returns null ProtoValues.
This causes silent data corruption in at least two call paths:
-
_python_dict_to_map_proto(line 983): A struct like{"scores": []}will have its empty-list value silently converted to a null ProtoValue, losing the semantic distinction between "empty list" and "null". -
remote.py:205-213: When the remote online store returns a feature with status"PRESENT"and value[](a valid empty list), the function returnsProtoValue()(null). The caller then stores this as the feature value (feature_values_dict[feature_name] = message[0]), silently dropping the actual empty-list value.
A safer fix would be to only return empty ProtoValues when all values are actually None, not when they are empty collections.
Prompt for agents
In sdk/python/feast/type_map.py at line 1017-1018, the fix for all-NULL columns is too broad — it also silently converts empty-collection values (like []) to null ProtoValues. Instead of unconditionally returning empty ProtoValues when value_type is UNKNOWN, check whether ALL values are actually None. If so, return empty ProtoValues (the intended fix for all-NULL columns). If some values are non-None but just empty collections (like []), either raise the original TypeError or attempt more aggressive type inference. For example:
if value_type == ValueType.UNKNOWN:
if all(v is None for v in values):
return [ProtoValue() for _ in values]
raise TypeError("Couldn't infer value type from empty value")
Was this helpful? React with 👍 or 👎 to provide feedback.
yuan1j
left a comment
There was a problem hiding this comment.
Thank you for your helpful advice. This solution works for my case as well. Could you please review and approve the merge?
What this PR does / why we need it
📋 Problem Description
When reading features from a Feast Remote Online Store, a
TypeErroris raised if an entire feature column contains onlyNULL/NaNvalues. The error occurs in thepython_values_to_proto_values()function when it cannot infer the value type from empty data.🔍 Root Cause Analysis
feast/type_map.py-python_values_to_proto_values()functionNULL/NaNvaluesUNKNOWNvalue typeTypeError: Couldn't infer value type from empty value📊 Test Data Example
💡 Solution
Modify the type inference logic to handle
UNKNOWNvalue types gracefully:Key Changes
UNKNOWNCode Change Summary
✨ Expected Behavior
After this fix:
Which issue(s) this PR fixes
Fixes: Remote Online Store Type Inference Error with All-NULL Columns
Related Error:
Misc
🧪 Testing
Manual Test Steps
Configuration Used
📝 Additional Notes
feast/type_map.py,feast/infra/online_stores/remote.py👥 Reviewers
✅ Checklist