Skip to content

fix(sessions): use LONGTEXT for MySQL message storage - #4910

Closed
rioyu123 wants to merge 1 commit into
openai:mainfrom
rioyu123:codex/mysql-session-longtext
Closed

rioyu123 wants to merge 1 commit into
openai:mainfrom
rioyu123:codex/mysql-session-longtext

Conversation

@rioyu123

@rioyu123 rioyu123 commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This pull request changes the message_data column to LONGTEXT for newly created MySQL and MariaDB message tables. Other dialects retain TEXT; serialization, session identifiers, transactions, and public APIs are unchanged. Existing tables are not migrated.

MySQL-family TEXT storage is too small for some serialized session items. In a live example, Chinese/emoji content serialized to 1,200,028 bytes: strict mode rejected the TEXT insert with error 1406, while non-strict mode logged truncation and stored 65,535 bytes of invalid JSON that the SDK could not return as the original item. A LONGTEXT column preserved the full item through SDK add/get/pop in both modes.

Dependency status: #4741 was closed without merging because MySQL/MariaDB schema-creation work is not a current maintainer priority. This draft is being closed as well. It contains none of #4741's changes and does not independently enable automatic table creation. Existing caller-managed schemas remain unchanged.

Any future continuation would need an accepted schema-creation baseline, fresh MySQL/MariaDB automatic-table-creation and small/large-item tests, and the full verification and review steps. The results below cover the draft as tested, not that combined path.

Test plan

  • tests/extensions/memory/test_sqlalchemy_session.py: 47 passed, including four new column-compilation cases for MySQL, MariaDB, PostgreSQL, and SQLite.
  • Ruff check and format check passed for both changed files; git diff --check passed.
  • Real MySQL 8.0.46 and MariaDB 11.8.9, using caller-managed tables and SQLAlchemySession(create_tables=False): small-item controls passed; strict TEXT raised error 1406; non-strict TEXT produced truncated, invalid JSON; LONGTEXT preserved the complete 1,200,028-byte serialized value and exact SDK get/pop results in both modes.
  • Offline complete-table compilation: current main plus this change still fails on unbounded session_id; fix(sessions): support MySQL and MariaDB schema creation #4741's schema plus the proposed column type compiles for both MySQL and MariaDB. This was a disposable metadata composition check, not a combined-source live automatic-creation test.
  • The full repository verification stack and final schema-change review were not completed for this draft. No full-stack pass is claimed.

Issue number

N/A. Related schema-creation work: #4741.

Checks

  • I've added new tests, if relevant
  • I've run .agents/skills/code-change-verification/scripts/run.sh
  • I've confirmed all verification steps pass
  • If using Codex, I've run /review before submitting this PR

@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open for 10 days with no activity.

@github-actions github-actions Bot added the stale label Sep 19, 2026
@rioyu123

Copy link
Copy Markdown
Contributor Author

Still planned, and intentionally kept as a draft while #4741 (or an equivalent MySQL/MariaDB schema-creation fix) is reviewed. That dependency has not merged yet. Once the accepted fix reaches main, I will update this branch and verify automatic table creation plus small- and large-item round trips on both databases before requesting review.

Could a maintainer add skip-stale while that dependency is pending? I would like to keep the column-capacity fix separate from the schema-creation work.

@github-actions github-actions Bot removed the stale label Sep 20, 2026
@rioyu123

Copy link
Copy Markdown
Contributor Author

Closing this draft following the decision on #4741. The LONGTEXT change depends on an accepted MySQL/MariaDB schema-creation baseline, so it cannot move forward on its own. The patch and test results remain available here if this work is revisited. Thanks for the consideration.

@rioyu123 rioyu123 closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants