Skip to content

IMPLEMENTATION_SUMMARY.md

Latest commit

 

History

History
111 lines (81 loc) · 4.08 KB

IMPLEMENTATION_SUMMARY.md

File metadata and controls

111 lines (81 loc) · 4.08 KB

FOR Clause Parser Support - Implementation Summary

What Was Done

This PR investigates and documents the challenges of adding native FOR clause support to Opteryx's SQL parser.

Deliverables

  1. Comprehensive Documentation (docs/FOR_CLAUSE_PARSING.md):

    • Explains Opteryx's temporal FOR clause syntax
    • Documents current Python-based implementation
    • Analyzes why native parser support is challenging
    • Outlines 4 potential approaches with trade-offs

  2. Proof-of-Concept Rust Module (src/temporal_parser.rs):

    • Skeleton implementation showing how temporal extraction could work in Rust
    • Exposed to Python via extract_temporal_filters function
    • Clearly documented as POC, not production-ready
    • Includes basic test structure

  3. Updated Rust Library (src/lib.rs):

    • Added extract_temporal_filters function to Python API
    • Maintained backward compatibility

Key Findings

After deep investigation of both sqlparser-rs (v0.59.0) architecture and Opteryx's current implementation:

Challenge: External Dependency Limitations

sqlparser-rs provides limited extension points:

  • parse_infix: For custom infix operators (e.g., @>> for ArrayContainsAll)
  • parse_prefix: For custom prefix operators
  • parse_statement: For custom statement types
  • No hook for extending table-level syntax (where FOR clauses appear)

Current Implementation is Well-Designed

The existing Python approach in sql_rewriter.py:

  • ✅ Handles complex cases (quoted strings, comments, nested queries)
  • ✅ Well-tested with comprehensive test suite
  • ✅ Proven in production
  • ✅ Supports special cases (b"" strings, r"" strings, EXTRACT/SUBSTRING/TRIM functions)

Options for Native Support

  1. Port to Rust (started in this PR): Move Python logic to Rust for performance
  2. Fork sqlparser-rs: Add native FOR support, but creates maintenance burden
  3. Use WITH Hints: Convert FOR X to WITH(__TEMPORAL__='X') - clever but awkward
  4. Keep Current: Python implementation is good enough

What This PR Does NOT Do

❌ Replace the existing Python implementation ❌ Change any query execution behavior ❌ Modify the AST structure ❌ Add new SQL syntax support

The Python implementation remains the authoritative version.

Recommendation

For the current issue: The investigation shows that adding native parser support is more complex than initially expected. The current Python implementation should be kept because:

  1. It works reliably
  2. It's well-tested
  3. The complexity of alternatives outweighs benefits
  4. Performance is not a bottleneck here

If parser support is still desired, the recommended approach is:

  1. Start with Option 3 (WITH hints) as a low-risk experiment
  2. If successful, consider Option 2 (fork sqlparser-rs) for clean integration

Files Changed

  • src/lib.rs: Added extract_temporal_filters function (POC)
  • src/temporal_parser.rs: New module with documented POC implementation
  • docs/FOR_CLAUSE_PARSING.md: Comprehensive documentation

Testing

# Rust tests pass
cargo test --release temporal_parser

# Python tests unchanged (existing implementation still used)
python -m pytest tests/unit/planner/test_temporal_extraction.py

Next Steps (If Pursuing This Further)

  1. Review docs/FOR_CLAUSE_PARSING.md and choose an approach
  2. If choosing Rust port (Option 1):
    • Complete the split_sql_parts function
    • Port the state machine logic accurately
    • Add comprehensive tests matching Python test suite
    • Benchmark vs Python
    • Gradual migration
  3. If choosing fork (Option 2):
    • Fork sqlparser-rs
    • Add TableFactor::Table fields for temporal info
    • Modify parser to recognize FOR clauses
    • Test with Opteryx
  4. If choosing hints (Option 3):
    • Modify sql_rewriter to convert FOR to WITH hints
    • Add post-parsing extraction of hints
    • Test thoroughly

Conclusion

This PR provides a thorough analysis and documentation of the problem space. The current Python implementation is good and should be kept. Native parser support is feasible but requires significant effort with unclear benefits.