-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Expected Behavior
I should be able to use an int64 value as the join key value for an entity, when the value type is value_type=ValueType.INT64.
Current Behavior
Only values -2147483648 <= number <= 2147483647 can be used as the value. This is equivalent to -2^31 <= number <= 2^31 - 1
Steps to reproduce
Specify a value greater than 2^31 - 1 as a the join key value when looking up values from the feature store.
>>> fs.get_online_features(features=['driver_hourly_stats:conv_rate'], entity_rows=[{'driver_id': 2147483648}]).to_df()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/achal/tecton/feast/sdk/python/feast/usage.py", line 202, in exception_logging_wrapper
result = func(*args, **kwargs)
File "/Users/achal/tecton/feast/sdk/python/feast/feature_store.py", line 897, in get_online_features
union_of_entity_keys,
File "/Users/achal/tecton/feast/sdk/python/feast/feature_store.py", line 924, in _populate_result_rows_from_feature_view
requested_features=requested_features,
File "/Users/achal/tecton/feast/sdk/python/feast/infra/passthrough_provider.py", line 81, in online_read
result = self.online_store.online_read(config, table, entity_keys)
File "/Users/achal/tecton/feast/sdk/python/feast/infra/online_stores/redis.py", line 190, in online_read
redis_key_bin = _redis_key(project, entity_key)
File "/Users/achal/tecton/feast/sdk/python/feast/infra/online_stores/helpers.py", line 40, in _redis_key
key: List[bytes] = [serialize_entity_key(entity_key), project.encode("utf-8")]
File "/Users/achal/tecton/feast/sdk/python/feast/infra/key_encoding_utils.py", line 41, in serialize_entity_key
val_bytes, value_type = _serialize_val(v.WhichOneof("val"), v)
File "/Users/achal/tecton/feast/sdk/python/feast/infra/key_encoding_utils.py", line 17, in _serialize_val
return struct.pack("<l", v.int64_val), ValueType.INT64
struct.error: 'l' format requires -2147483648 <= number <= 2147483647
Possible Solution
The trivial fix is to make this line use "q" as the format string, which refers to a signed long long value which is 8 bytes in size.
Changing this behavior is a backwards incompatible change, since it would change the bytes encoded for an entity key which takes on int64 values. This means that materialized records oh that value type would would be unreadable.
The "correct" way of fixing this would require us to version the encoding scheme in the feature store.