Skip to content

Pre-commit hook / GHA to detect change in SQL AST, regardless of method used #121

Description

@agritheory

Create a comprehensive tracking system for both frappe.db.sql calls and Frappe's Query Builder to de-risk changes in hard-to-test code like financial reports.

Background

Systematically migrate from raw SQL calls to Frappe's Query Builder to:

  • Maintain semantic equivalence during refactoring
  • Improve performance while refactoring SQL without dropping columns or conditions

As a bonus:

  • Improve type safety and IDE support
  • Ensure dialect compatibility (MariaDB → PostgreSQL)

Data Storage Strategy

Primary Storage: Pickle format (.sql_registry.pkl)

{
  "metadata": {
    "version": "1.0", 
    "last_scan": datetime,
    "repository": "org/repo_name",
    "total_calls": 47,
    "commit_hash": "abc123"
  },
  "calls": {
    "call_id_hash": SQLCall(
      call_id="abc123def",
      file_path="app/models.py",
      line_number=45, 
      function_context="def get_user_data():",
      sql_query="SELECT * FROM users WHERE active = 1",
      ast_object=<sqlglot.expressions.Select>,  # Full AST object
      ast_normalized="SELECT_users_WHERE_active_EQ_1",
      pypika_code="frappe.qb.from_('users').select('*').where(...)",
      migration_status="generated",
      confidence_score=0.85,
      notes=None,
      created_at=datetime,
      updated_at=datetime
    )
  }
}

Implementation

  1. SQL Pattern Detection (Extend existing find_sql_docstrings logic)

    # Detect both frappe.db.sql() and Query Builder patterns
    PATTERNS = {
      'frappe_db_sql': r'frappe\.db\.sql\s*\(\s*["\']([^"\']+)["\']',
      'query_builder': r'frappe\.qb\.from_\s*\(',
      'pypika_direct': r'Query\.from_\s*\('
    }
    
    # Reuse AST traversal from paste.txt:
    for node in ast.walk(tree):
      if isinstance(node, ast.Call):
        # Detect frappe.db.sql calls
      elif isinstance(node, ast.Constant):
        # Detect SQL strings in docstrings/variables
  2. AST Processing Pipeline (Build on format_sql_content pattern)

    def process_sql_call(sql_query: str, file_path: str, line_num: int):
      # 1. Parse with sqlglot (reuse replace_sql_patterns logic)
      sql_cleaned, replacements = replace_sql_patterns(sql_query)
      ast_object = sqlglot.parse(sql_cleaned, dialect="mysql")[0]
      
      # 2. Generate normalized signature for comparison
      ast_normalized = generate_ast_signature(ast_object)
      
      # 3. Convert to PyPika (extend existing format logic)
      pypika_code = ast_to_frappe_qb(ast_object, replacements)
  3. Pickle-based Registry

    @dataclass
    class SQLCall:
      call_id: str
      file_path: str  
      line_number: int
      function_context: str
      sql_query: str
      ast_object: sqlglot.Expression  # Full AST for semantic analysis
      ast_normalized: str
      pypika_code: str
      migration_status: str  # found|generated|reviewed|migrated
      confidence_score: float
      created_at: datetime
      updated_at: datetime
  4. Markdown Report Generation

    def generate_markdown_report(registry_data: Dict) -> str:
      # Generate GitHub-friendly progress report
      # Include tables, progress bars, code samples
      # Link to specific lines in GitHub

Data Storage Strategy

Primary Storage: Pickle format (.sql_registry.pkl)

{
  "metadata": {
    "version": "1.0",
    "last_scan": datetime,
    "repository": "org/repo_name",
    "total_calls": 47,
    "commit_hash": "abc123"
  },
  "calls": {
    "call_id_hash": SQLCall(
      call_id="abc123def",
      file_path="app/models.py", 
      line_number=45,
      function_context="def get_user_data():",
      sql_query="SELECT * FROM users WHERE active = 1",
      ast_object=<sqlglot.expressions.Select>,  # Full AST object
      ast_normalized="SELECT_users_WHERE_active_EQ_1",
      pypika_code="frappe.qb.from_('users').select('*').where(...)",
      migration_status="generated",
      confidence_score=0.85,
      notes=None,
      created_at=datetime,
      updated_at=datetime
    )
  }
}

Human Interface: Generated Markdown report (.sql_migration_report.md)

# SQL Migration Progress Report

**Repository**: user/project_name  
**Last Updated**: August 19, 2025  
**Total SQL Calls**: 47 | **Migrated**: 12 | **Remaining**: 35

## Progress by File
| File | Total | Migrated | Remaining | 
|------|--------|----------|-----------|
| `app/models.py` | 8 | 3 | 5 |
| `app/utils.py` | 12 | 8 | 4 | 

Integration Points

  1. Pre-commit Hook

    - id: register_sql
      name: Track SQL Migration Progress  
      entry: register_sql
      language: python
      files: '\.py$'
      require_serial: true
  2. GitHub Action

    • Run on PR creation/updates
    • Comment migration progress on PRs
    • Fail CI if new frappe.db.sql calls are introduced without Query Builder equivalents

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions