-
Notifications
You must be signed in to change notification settings - Fork 61
feat: Add compile-time type validation for column specifications and optimize performance #208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for the PR. Direction wise, I don't think we can reliability do the check in the macro. For example, above the macro, there could be |
|
Thank you for the feedback @carllerche. I was coming to a similar conclusion late last night as I was trying to iterate on this, but ran out of time and steam. I'll move this back to draft for now so it doesn't clutter the list of PRs, but should be able to revisit this in the next couple of days. |
3f6dfda to
96e4a9f
Compare
Implements runtime type validation during schema building to catch type mismatches that can't be detected at macro expansion time (e.g., type alias shadowing). Key features: - Schema-time validation in toasty-core/schema/verify.rs - Whitelist approach for maintainability (explicit compatible types) - Support for all integer types (i8-i64, u8-u64) with size validation - Support for String, bool, and UUID types - Clear error messages with suggestions - Full bool primitive support across all drivers (SQLite, PostgreSQL, MySQL) - Comprehensive test coverage (runtime validation + integration tests) Changes: - Add verify_column_type_compatibility() to schema verification - Remove macro-time validation (can't handle type aliases) - Add bool value conversion for SQLite (INTEGER 0/1) - Add integration tests for valid type combinations - Add runtime validation tests for invalid combinations Benefits: - Catches type mismatches at schema build time (not runtime) - Handles complex type alias scenarios that macros cannot - Provides clear error messages with type suggestions - Maintains type safety without runtime overhead
96e4a9f to
3502403
Compare
- Add support for Type::BigDecimal in validation logic - BigDecimal is compatible with TEXT storage type across all databases - Update error messages and suggestions for BigDecimal fields Fixes CI test failures in tests/bigdecimal.rs
- Add support for Type::Uuid with DbType::Blob and DbType::Binary - UUID types now support TEXT, VARCHAR, UUID, BLOB, BINARY storage - Update error messages to include BLOB and BINARY options - Handle custom type strings for blob and binary types Fixes failing tests: specify_uuid_as_bytes
Auto-format long line to satisfy CI formatting checks
- Add validation for Type::Timestamp, Type::Date, Type::Time, Type::DateTime - Support compatibility with TEXT storage (SQLite/DynamoDB) and native types (PostgreSQL/MySQL) - Update helper methods for error messages and suggestions - Fixes CI test failures for jiff date/time types showing as 'unknown'
Major improvements to type validation system: 1. **Add comprehensive test coverage** for schema validation logic - Add 7 new jiff date/time validation tests - Test valid and invalid combinations across all jiff types - Add case-insensitive type matching test - Fix existing type_alias_detection test assertions 2. **Fix case sensitivity bug** in custom type string matching - All custom type comparisons now use .to_lowercase() - Fixes issue where 'TEXT' failed but 'text' worked - Applies to all types: String, bool, integers, UUID, BigDecimal, jiff types 3. **Improve database abstraction** with better documentation and structure - Add comprehensive documentation to is_storage_compatible() method - Document database compatibility matrix in code comments - Extract jiff validation to separate helper method for maintainability - Acknowledge current architectural limitations and future improvements Code quality improvements: - Reduce code duplication in jiff type validation - Better error handling with consistent case-insensitive matching - Clearer separation of concerns with helper methods - Comprehensive inline documentation for future maintainers
Summary
Key Features
Changes Made
Core Implementation
Test Coverage
Problem Solved
Original issue (#185): Type validation at macro time fails with type aliases
Example that was problematic:
Macro-time validation can't detect this because it sees the identifier String and assumes it's std::String.
Schema-time validation (our solution) catches this at runtime during schema building when the actual stmt::Type is resolved.
Architecture Decision
Following maintainer feedback (@carllerche), we moved validation from macro expansion time to schema building time:
Benefits
Fixes #185