Skip to content

Conversation

cofin
Copy link
Member

@cofin cofin commented Oct 10, 2025

Fixes #118

Summary

Removes unnecessary clear_cache() calls that were causing SQL files to be loaded and parsed 3 times during a single migration operation.

Root Cause

SQLFileLoader.clear_cache() was being called before every migration operation, destroying CoreSQLFileLoader's internal cache (_files, _queries, _query_to_file). This forced files to be re-parsed multiple times during a single migration run.

Evidence from user logs:

INFO - sqlspec.loader - loaders - Loading SQL files
INFO - sqlspec.loader - loaders - Loaded 1 SQL files with 2 new queries in 0.883ms
INFO - sqlspec.loader - thread - Loading SQL files
INFO - sqlspec.loader - thread - Loaded 1 SQL files with 2 new queries in 2.596ms
INFO - sqlspec.loader - loaders - Loading SQL files
INFO - sqlspec.loader - loaders - Loaded 1 SQL files with 2 new queries in 0.216ms

Changes

  • Removed clear_cache() from SQLFileLoader.get_up_sql()
  • Removed clear_cache() from SQLFileLoader.get_down_sql()
  • Removed clear_cache() from SQLFileLoader.validate_migration_file()
  • Removed clear_cache() from SyncMigrationRunner.load_migration()
  • Removed clear_cache() from AsyncMigrationRunner.load_migration()

Total: 5 lines deleted

Why This Works

CoreSQLFileLoader already has proper caching built-in (see sqlspec/loader.py:428-429):

if path_str in self._files:
    return  # Early exit if cached

The cache clearing was unnecessary and counterproductive.

Impact

  • Files now loaded once per migration operation (down from 3x)
  • ~66% reduction in file I/O and parsing overhead
  • User will see single "Loading SQL files" log entry instead of 3

Testing

  • Added test_sql_loader_caches_files to verify cache persistence across operations
  • All 126 tests passing (90 unit + 36 integration)
  • No functional changes or breaking changes
  • Verified with both sync and async migration runners

Breaking Changes

None - internal optimization only.

Fixes #118

## Summary
Removes unnecessary `clear_cache()` calls that were causing SQL files
to be loaded and parsed 3 times during a single migration operation.

## Root Cause
SQLFileLoader.clear_cache() was being called before every migration
operation, destroying CoreSQLFileLoader's internal cache (_files,
_queries, _query_to_file). This forced files to be re-parsed multiple
times.

## Changes
- Removed clear_cache() from SQLFileLoader.get_up_sql()
- Removed clear_cache() from SQLFileLoader.get_down_sql()
- Removed clear_cache() from SQLFileLoader.validate_migration_file()
- Removed clear_cache() from SyncMigrationRunner.load_migration()
- Removed clear_cache() from AsyncMigrationRunner.load_migration()

## Impact
- Files now loaded once per migration operation (down from 3x)
- All 126 tests passing

## Testing
- Added test_sql_loader_caches_files to verify cache persistence
- Verified all existing migration tests pass
- No functional changes or breaking changes
@cofin cofin merged commit 0a7d716 into main Oct 10, 2025
21 of 28 checks passed
@cofin cofin deleted the sql-loader-duplicates branch October 10, 2025 20:27
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.

Bug: duplicate loading ?

1 participant