Skip to content

fix: preserve millisecond precision for UNIX_TIMESTAMP sort keys#369

Open
piket wants to merge 3 commits into
masterfrom
fix/EAPC-22316-sorted-fv-timestamp-precision
Open

fix: preserve millisecond precision for UNIX_TIMESTAMP sort keys#369
piket wants to merge 3 commits into
masterfrom
fix/EAPC-22316-sorted-fv-timestamp-precision

Conversation

@piket

@piket piket commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

SortedFeatureView rows whose UNIX_TIMESTAMP sort keys differed by less than one second were being assigned the same sort key score in Redis/Valkey (wrong ordering) and the same Cassandra clustering key (row overwrites).

Root cause: _python_datetime_to_int_timestamp() truncated all UNIX_TIMESTAMP values to integer seconds before storing in unix_timestamp_val. The * 1000 multiplications in zset_score() attempted ms precision but had no effect.

Fix:

  • Add _python_datetime_to_int_ms_timestamp() for millisecond conversion.
  • In _convert_arrow_fv_to_proto(), detect UNIX_TIMESTAMP sort key columns and use the ms function for those columns only; all other UNIX_TIMESTAMP columns remain at second precision (backward compatible).
  • Remove the * 1000 from zset_score() in Redis, Valkey, and Cassandra — unix_timestamp_val is now already ms for sort key columns.
  • Add a threshold discriminator (val > 1e11) in feast_value_type_to_python_type() so sort key ms values are correctly read back as datetimes without affecting the seconds interpretation for regular feature columns.

What this PR does / why we need it:

Which issue(s) this PR fixes:

Misc

piket and others added 2 commits June 30, 2026 12:12
…C-22316)

SortedFeatureView rows whose UNIX_TIMESTAMP sort keys differed by less
than one second were being assigned the same sort key score in Redis/Valkey
(wrong ordering) and the same Cassandra clustering key (row overwrites).

Root cause: _python_datetime_to_int_timestamp() truncated all UNIX_TIMESTAMP
values to integer seconds before storing in unix_timestamp_val. The * 1000
multiplications in zset_score() attempted ms precision but had no effect.

Fix:
- Add _python_datetime_to_int_ms_timestamp() for millisecond conversion.
- In _convert_arrow_fv_to_proto(), detect UNIX_TIMESTAMP sort key columns
  and use the ms function for those columns only; all other UNIX_TIMESTAMP
  columns remain at second precision (backward compatible).
- Remove the * 1000 from zset_score() in Redis, Valkey, and Cassandra —
  unix_timestamp_val is now already ms for sort key columns.
- Add a threshold discriminator (val > 1e11) in feast_value_type_to_python_type()
  so sort key ms values are correctly read back as datetimes without affecting
  the seconds interpretation for regular feature columns.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zabarn zabarn force-pushed the fix/EAPC-22316-sorted-fv-timestamp-precision branch 2 times, most recently from 721115e to 2e760ed Compare June 30, 2026 19:00
…s (EAPC-22316)

After the Python SDK was updated to store sort key unix_timestamp_val as
milliseconds, the Go serving layer needed matching changes to avoid treating
ms values as epoch-seconds (which would produce year ~57000 timestamps).

Production changes:
- typeconversion.go: add unixTsToTime() helper with a >1e11 threshold to
  distinguish ms from s; use it for all UnixTimestampVal conversions
- redis_read_range_utils.go: use UnixMilli() for ZRANGEBYSCORE bounds to
  match the ms ZADD scores written by the Python materializer

Test/infra changes:
- go_integration_test_utils.go: support FEAST_BIN env var for local venv
  feast; return UnixMilli() from ReadParquetDynamically for timestamp columns
- valkey_integration_test.go: fix ms/s conversions in getValueType and
  EventTimestamps assertions
- scylladb_integration_test.go: update equals filter to ms precision (1744769171919)
- http_integration_test.go: update equals filter to ms precision (1744769171919)
- redisonlinestore_test.go, redis_read_range_utils_test.go: update expected
  ZADD scores and range bounds to ms scale

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant