Skip to content

Commit fc15c9a

Browse files
hung12ctcopybara-github
authored andcommitted
fix: Update DynamicPickleType to support MySQL dialect
Merge #3282 The `process_bind_param` and `process_result_value` methods in the `DynamicPickleType` class have been modified to handle MySQL dialect in addition to Spanner. This change ensures that pickled values are correctly processed for both database types. **Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: #3283 **2. Or, if no issue exists, describe the change:** _If applicable, please follow the issue templates to provide as much detail as possible._ **Problem:** When using `DatabaseSessionService` with MySQL backend in google-adk v1.17.0, the application crashes with the following error: app.resources.runner:event_generator:260 - Error in event_generator: (builtins.TypeError) 'tuple' object cannot be interpreted as an integer <img width="1237" height="129" alt="image" src="https://github.com/user-attachments/assets/0a5fc223-600a-4a92-8443-4d37fb1267f6" /> Root cause: The `DynamicPickleType` class in `database_session_service.py` configures MySQL dialect to use `LONGBLOB` for storing pickled data (line 117-118), but the `process_bind_param` and `process_result_value` methods only handle pickle serialization/deserialization for Spanner dialect, not MySQL. This causes MySQL to attempt storing raw Python objects instead of pickled bytes, leading to serialization errors and potential data corruption. **Solution:** Added MySQL to the pickle serialization logic in both `process_bind_param` and `process_result_value` methods, treating it the same way as Spanner dialect. This ensures that: - Data is properly pickled to bytes before being stored in MySQL's LONGBLOB column - Data is properly unpickled when retrieved from the database - No breaking changes to existing functionality for other dialects (SQLite, PostgreSQL) ### Testing Plan _Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes._ **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. **Summary of `pytest` results:** <img width="929" height="306" alt="image" src="https://github.com/user-attachments/assets/3d548b96-ac49-4101-8405-a289a722293c" /> **Manual End-to-End (E2E) Tests:** _Please provide instructions on how to manually test your changes, including any necessary setup or configuration. Please provide logs or screenshots to help reviewers better understand the fix._ ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [x] Any dependent changes have been merged and published in downstream modules. ### Additional context _Add any other context or screenshots about the feature request here._ COPYBARA_INTEGRATE_REVIEW=#3282 from hung12ct:fix/mysql-pickle-serialization d9df37a PiperOrigin-RevId: 825834360
1 parent f9569bb commit fc15c9a

File tree

2 files changed

+183
-2
lines changed

2 files changed

+183
-2
lines changed

src/google/adk/sessions/database_session_service.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,14 @@ def load_dialect_impl(self, dialect):
125125
def process_bind_param(self, value, dialect):
126126
"""Ensures the pickled value is a bytes object before passing it to the database dialect."""
127127
if value is not None:
128-
if dialect.name == "spanner+spanner":
128+
if dialect.name in ("spanner+spanner", "mysql"):
129129
return pickle.dumps(value)
130130
return value
131131

132132
def process_result_value(self, value, dialect):
133133
"""Ensures the raw bytes from the database are unpickled back into a Python object."""
134134
if value is not None:
135-
if dialect.name == "spanner+spanner":
135+
if dialect.name in ("spanner+spanner", "mysql"):
136136
return pickle.loads(value)
137137
return value
138138

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
from __future__ import annotations
16+
17+
import pickle
18+
from unittest import mock
19+
20+
from google.adk.sessions.database_session_service import DynamicPickleType
21+
import pytest
22+
from sqlalchemy import create_engine
23+
from sqlalchemy.dialects import mysql
24+
25+
26+
@pytest.fixture
27+
def pickle_type():
28+
"""Fixture for DynamicPickleType instance."""
29+
return DynamicPickleType()
30+
31+
32+
def test_load_dialect_impl_mysql(pickle_type):
33+
"""Test that MySQL dialect uses LONGBLOB."""
34+
# Mock the MySQL dialect
35+
mock_dialect = mock.Mock()
36+
mock_dialect.name = "mysql"
37+
38+
# Mock the return value of type_descriptor
39+
mock_longblob_type = mock.Mock()
40+
mock_dialect.type_descriptor.return_value = mock_longblob_type
41+
42+
impl = pickle_type.load_dialect_impl(mock_dialect)
43+
44+
# Verify type_descriptor was called once with mysql.LONGBLOB
45+
mock_dialect.type_descriptor.assert_called_once_with(mysql.LONGBLOB)
46+
# Verify the return value is what we expect
47+
assert impl == mock_longblob_type
48+
49+
50+
def test_load_dialect_impl_spanner(pickle_type):
51+
"""Test that Spanner dialect uses SpannerPickleType."""
52+
# Mock the spanner dialect
53+
mock_dialect = mock.Mock()
54+
mock_dialect.name = "spanner+spanner"
55+
56+
with mock.patch(
57+
"google.cloud.sqlalchemy_spanner.sqlalchemy_spanner.SpannerPickleType"
58+
) as mock_spanner_type:
59+
pickle_type.load_dialect_impl(mock_dialect)
60+
mock_dialect.type_descriptor.assert_called_once_with(mock_spanner_type)
61+
62+
63+
def test_load_dialect_impl_default(pickle_type):
64+
"""Test that other dialects use default PickleType."""
65+
engine = create_engine("sqlite:///:memory:")
66+
dialect = engine.dialect
67+
impl = pickle_type.load_dialect_impl(dialect)
68+
# Should return the default impl (PickleType)
69+
assert impl == pickle_type.impl
70+
71+
72+
@pytest.mark.parametrize(
73+
"dialect_name",
74+
[
75+
pytest.param("mysql", id="mysql"),
76+
pytest.param("spanner+spanner", id="spanner"),
77+
],
78+
)
79+
def test_process_bind_param_pickle_dialects(pickle_type, dialect_name):
80+
"""Test that MySQL and Spanner dialects pickle the value."""
81+
mock_dialect = mock.Mock()
82+
mock_dialect.name = dialect_name
83+
84+
test_data = {"key": "value", "nested": [1, 2, 3]}
85+
result = pickle_type.process_bind_param(test_data, mock_dialect)
86+
87+
# Should be pickled bytes
88+
assert isinstance(result, bytes)
89+
# Should be able to unpickle back to original
90+
assert pickle.loads(result) == test_data
91+
92+
93+
def test_process_bind_param_default(pickle_type):
94+
"""Test that other dialects return value as-is."""
95+
mock_dialect = mock.Mock()
96+
mock_dialect.name = "sqlite"
97+
98+
test_data = {"key": "value"}
99+
result = pickle_type.process_bind_param(test_data, mock_dialect)
100+
101+
# Should return value unchanged (SQLAlchemy's PickleType handles it)
102+
assert result == test_data
103+
104+
105+
def test_process_bind_param_none(pickle_type):
106+
"""Test that None values are handled correctly."""
107+
mock_dialect = mock.Mock()
108+
mock_dialect.name = "mysql"
109+
110+
result = pickle_type.process_bind_param(None, mock_dialect)
111+
assert result is None
112+
113+
114+
@pytest.mark.parametrize(
115+
"dialect_name",
116+
[
117+
pytest.param("mysql", id="mysql"),
118+
pytest.param("spanner+spanner", id="spanner"),
119+
],
120+
)
121+
def test_process_result_value_pickle_dialects(pickle_type, dialect_name):
122+
"""Test that MySQL and Spanner dialects unpickle the value."""
123+
mock_dialect = mock.Mock()
124+
mock_dialect.name = dialect_name
125+
126+
test_data = {"key": "value", "nested": [1, 2, 3]}
127+
pickled_data = pickle.dumps(test_data)
128+
129+
result = pickle_type.process_result_value(pickled_data, mock_dialect)
130+
131+
# Should be unpickled back to original
132+
assert result == test_data
133+
134+
135+
def test_process_result_value_default(pickle_type):
136+
"""Test that other dialects return value as-is."""
137+
mock_dialect = mock.Mock()
138+
mock_dialect.name = "sqlite"
139+
140+
test_data = {"key": "value"}
141+
result = pickle_type.process_result_value(test_data, mock_dialect)
142+
143+
# Should return value unchanged (SQLAlchemy's PickleType handles it)
144+
assert result == test_data
145+
146+
147+
def test_process_result_value_none(pickle_type):
148+
"""Test that None values are handled correctly."""
149+
mock_dialect = mock.Mock()
150+
mock_dialect.name = "mysql"
151+
152+
result = pickle_type.process_result_value(None, mock_dialect)
153+
assert result is None
154+
155+
156+
@pytest.mark.parametrize(
157+
"dialect_name",
158+
[
159+
pytest.param("mysql", id="mysql"),
160+
pytest.param("spanner+spanner", id="spanner"),
161+
],
162+
)
163+
def test_roundtrip_pickle_dialects(pickle_type, dialect_name):
164+
"""Test full roundtrip for MySQL and Spanner: bind -> result."""
165+
mock_dialect = mock.Mock()
166+
mock_dialect.name = dialect_name
167+
168+
original_data = {
169+
"string": "test",
170+
"number": 42,
171+
"list": [1, 2, 3],
172+
"nested": {"a": 1, "b": 2},
173+
}
174+
175+
# Simulate bind (Python -> DB)
176+
bound_value = pickle_type.process_bind_param(original_data, mock_dialect)
177+
assert isinstance(bound_value, bytes)
178+
179+
# Simulate result (DB -> Python)
180+
result_value = pickle_type.process_result_value(bound_value, mock_dialect)
181+
assert result_value == original_data

0 commit comments

Comments
 (0)