Skip to content

fix: keep integers beyond 64 bits exact in SqliteSessionService state - #7586

Closed
Mahir-Puri wants to merge 1 commit into
google:mainfrom
Mahir-Puri:fix/sqlite-state-big-ints
Closed

Mahir-Puri wants to merge 1 commit into
google:mainfrom
Mahir-Puri:fix/sqlite-state-big-ints

Conversation

@Mahir-Puri

Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:

SqliteSessionService merges state deltas inside SQLite (_MERGE_STATE_SQL).
SQLite reads an integer outside the signed 64-bit range as a REAL, so a value
like 0xFEDCBA9876543210 came back as 1.8364758544493064e+19 (a float). This
also happened when a later turn wrote an unrelated key, because the merge
rebuilds every key in the row. The in-memory and database services keep the
value exact.

Solution:

  • In the merge SQL, a key whose type is integer but whose parsed value is a
    REAL is rebuilt from its original JSON text (using the -> operator) instead
    of the parsed value. The float handling from fix(sessions): keep SQLite session state numbers exact across updates #7426 is unchanged.
  • The session state update now binds the delta by number (?1 to ?5), since
    the delta is used more than once in the statement.

Note: the -> operator needs SQLite 3.38 or newer.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added a 64-bit fingerprint value to _EXACT_NUMBERS in
tests/unittests/sessions/test_session_service.py. It covers session, user:
and app: state, both for new writes and after an unrelated state delta.
Without the fix, 6 of the SQLite cases fail.

  • pytest tests/unittests/sessions: 604 passed.
  • pytest tests/unittests (macOS, Python 3.11): 20049 passed, 1 failed. The
    failure is test_reading_directory_raises in
    tests/unittests/integrations/vmaas/test_agent_engine_environment.py. It
    fails the same way on unmodified main, so it is unrelated to this change.

Manual End-to-End (E2E) Tests:

Ran the reproduction script from #7576 (fake model, SqliteSessionService)
with PYTHONPATH=src python repro.py:

expected                     18364758544493064720 (int)
after remember_fingerprint   state['fingerprint'] = 18364758544493064720 (int)
after count_visit            state['fingerprint'] = 18364758544493064720 (int)
after check_fingerprint      state['fingerprint'] = 18364758544493064720 (int)
check_fingerprint tool saw: {'matches': True, 'value': '18364758544493064720'}

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

SQLite reads an integer outside the signed 64-bit range as a REAL, so the
state merge turned values like 0xFEDCBA9876543210 into floats. Rebuild
those keys from the original JSON text (via ->) instead of the parsed
value, and bind the session delta by number since it is now used more
than once in the statement.

Fixes google#7576
@Somay-kousis

Copy link
Copy Markdown

@Mahir-Puri this is #7577 again. I opened #7576 and #7577 a day before this one: same example hash (0xFEDCBA9876543210 turning into 1.8364758544493064e+19), the description reworded from my issue, the same -> fullkey lookup and condition, the same ?1 binding change.

Using an AI agent is fine, I use one too. But it shouldn't take someone's open PR and send it back as a new one. Please close this in favor of #7577.

It also still has the bug BichengWang found in #7577: the -> lookup runs for every key, so every state merge is quadratic in the key count. #7577 fixed that in f094042.

@Mahir-Puri Mahir-Puri closed this Oct 17, 2026
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.

SqliteSessionService turns state integers outside 64 bits into floats

3 participants