-
Notifications
You must be signed in to change notification settings - Fork 2k
Description
Issue: SQLite Foreign Key Constraints Not Enforced - Sessions Not Properly Deleted
Problem Description
When using DatabaseSessionService
with SQLite, foreign key constraints are not enforced by default. This causes a critical issue where calling delete_session()
only deletes the session record but leaves all related events in the database.
Expected Behavior
When delete_session()
is called, the session and all related events should be deleted due to the CASCADE foreign key constraint defined in the schema:
class StorageEvent(Base):
__table_args__ = (
ForeignKeyConstraint(
["app_name", "user_id", "session_id"],
["sessions.app_name", "sessions.user_id", "sessions.id"],
ondelete="CASCADE", # This should delete related events
),
)
Actual Behavior
delete_session()
deletes only the session record- All events remain in the database with orphaned foreign keys
- When recreating a session with the same ID, all old events are still present
- Chat history appears unchanged despite "clearing"
Root Cause
SQLite has foreign key constraints disabled by default for backwards compatibility. The DatabaseSessionService
does not enable them, so the CASCADE delete never executes.
Impact
- Critical: Chat history clearing functionality is broken
- Data Integrity: Orphaned records accumulate in the database
- User Experience: Users believe their data is deleted when it isn't
- Storage: Database grows with unremovable event records
Reproduction Steps
- Create a session and add some events
- Call
delete_session()
with the session details - Recreate a session with the same ID
- Observe that all old events are still present
Proposed Solution
Modify the DatabaseSessionService.__init__()
method to automatically enable foreign key constraints for SQLite databases:
def __init__(self, db_url: str, **kwargs: Any):
# Existing code...
try:
# For SQLite, ensure foreign keys are enabled
if db_url.startswith('sqlite'):
# Ensure connect_args has proper SQLite settings
connect_args = kwargs.get('connect_args', {})
connect_args.setdefault('check_same_thread', False)
kwargs['connect_args'] = connect_args
# Enable foreign key constraints for SQLite
@event.listens_for(Engine, "connect")
def set_sqlite_pragma(dbapi_connection, connection_record):
if 'sqlite' in str(dbapi_connection):
cursor = dbapi_connection.cursor()
cursor.execute("PRAGMA foreign_keys=ON")
cursor.close()
db_engine = create_engine(db_url, **kwargs)
# ... rest of existing code
except ArgumentError as e:
# ... existing exception handling
Alternative Implementation Options
-
Per-Engine Listener (less reliable due to timing):
# After engine creation if db_url.startswith('sqlite'): @event.listens_for(self.db_engine, "connect") def set_sqlite_pragma(dbapi_connection, connection_record): # Same pragma logic
-
Connection String Parameter (not supported in all SQLAlchemy versions):
# Users could manually add: "sqlite:///database.db?foreign_keys=ON"
Why This Wasn't Enabled by Default
- Backwards Compatibility: Some existing databases might have constraint violations
- Performance: Foreign key checking adds overhead to operations
- SQLite Specifics: Foreign key support was added later to SQLite
- Conservative Approach: Let users decide if they want constraint enforcement
Testing the Fix
After implementing the solution:
- Create a session with events
- Delete the session
- Check that related events are also deleted from the database
- Verify that recreating a session with the same ID starts with empty events
Alternative Workarounds
If enabling foreign keys globally is not desired:
- Manual Event Deletion: Explicitly delete events before deleting sessions
- New Session IDs: Use new UUIDs instead of recreating with the same ID
- Database Migration: Clean up orphaned events with a one-time script
Files Affected
google/adk/sessions/database_session_service.py
- Add SQLite foreign key enablement to__init__()
method