|
| 1 | +# QueryCompiler Fix Summary |
| 2 | + |
| 3 | +## Problem |
| 4 | +The test `CompiledQuery_1000RepeatedSelects_CompletesUnder8ms` was failing with: |
| 5 | +``` |
| 6 | +1000 compiled queries with cached plans should complete in <2000ms; took 3591ms |
| 7 | +``` |
| 8 | + |
| 9 | +## Root Cause Analysis |
| 10 | + |
| 11 | +### Investigation Steps |
| 12 | +1. Used debugger to confirm `stmt.CompiledPlan` was `null` |
| 13 | +2. Added diagnostic logging to `QueryCompiler.Compile()` |
| 14 | +3. Discovered the compilation was failing with: |
| 15 | + ``` |
| 16 | + InvalidOperationException: The binary operator GreaterThan is not defined for the types 'System.Object' and 'System.Object'. |
| 17 | + ``` |
| 18 | + |
| 19 | +### Root Cause |
| 20 | +When compiling WHERE clauses like `value > 500`, the QueryCompiler was attempting to create `Expression.GreaterThan(left, right)` where: |
| 21 | +- `left` = Dictionary value (type: `object`) |
| 22 | +- `right` = Literal constant (type: `int`) |
| 23 | + |
| 24 | +The original code tried to convert both to the same type, but when both ended up as `object` type, the `Expression.GreaterThan()` method failed because LINQ expression trees cannot create comparison operators for `object` types. |
| 25 | + |
| 26 | +## Solution |
| 27 | + |
| 28 | +### Changes Made to `src\SharpCoreDB\Services\QueryCompiler.cs` |
| 29 | + |
| 30 | +1. **Use IComparable for object-typed comparisons** (new method `CompareUsingIComparable`): |
| 31 | + - When either operand is `object` type, use `IComparable.CompareTo()` instead of direct comparison operators |
| 32 | + - Example: `left.CompareTo(right) > 0` instead of `left > right` |
| 33 | + |
| 34 | +2. **Handle numeric type mismatches** (new method `GetCommonNumericType`): |
| 35 | + - Finds the widest common numeric type for two operands |
| 36 | + - Precedence: decimal > double > float > long > int > short > byte |
| 37 | + - Prevents cast exceptions when comparing Int32 with Decimal, etc. |
| 38 | + |
| 39 | +3. **Refactored `ConvertBinaryExpression`**: |
| 40 | + - Prioritize `IComparable` for object-typed values |
| 41 | + - Use `GetCommonNumericType()` for strongly-typed numeric comparisons |
| 42 | + - Fall back to `IComparable` if no common type exists |
| 43 | + |
| 44 | +### Code Structure |
| 45 | +```csharp |
| 46 | +if (left.Type == typeof(object) || right.Type == typeof(object)) |
| 47 | +{ |
| 48 | + return CompareUsingIComparable(left, right, op); |
| 49 | +} |
| 50 | + |
| 51 | +if (left.Type != right.Type) |
| 52 | +{ |
| 53 | + var commonType = GetCommonNumericType(left.Type, right.Type); |
| 54 | + if (commonType != null) |
| 55 | + { |
| 56 | + // Convert to common type |
| 57 | + } |
| 58 | + else |
| 59 | + { |
| 60 | + return CompareUsingIComparable(left, right, op); |
| 61 | + } |
| 62 | +} |
| 63 | +``` |
| 64 | + |
| 65 | +## Results |
| 66 | + |
| 67 | +### Performance Improvement |
| 68 | +- **Before**: 3,591ms+ for 1000 queries (FAILED) |
| 69 | +- **After**: 137-1466ms for 1000 queries (PASSED) |
| 70 | +- **Improvement**: ~2.5-26x faster (queries now use compiled expression trees) |
| 71 | + |
| 72 | +### Test Results |
| 73 | +- ✅ All 9 CompiledQueryTests passing (1 skipped by design) |
| 74 | +- ✅ Compilation now succeeds for queries with WHERE clauses |
| 75 | +- ✅ Handles complex WHERE clauses with multiple conditions |
| 76 | +- ✅ Properly converts between numeric types (Int32, Decimal, etc.) |
| 77 | + |
| 78 | +## Files Modified |
| 79 | +1. `src\SharpCoreDB\Services\QueryCompiler.cs` |
| 80 | + - Fixed `ConvertBinaryExpression()` method |
| 81 | + - Added `CompareUsingIComparable()` method |
| 82 | + - Added `GetCommonNumericType()` method |
| 83 | + |
| 84 | +2. `tests\SharpCoreDB.Tests\CompiledQueryTests.cs` |
| 85 | + - Removed workaround skip logic |
| 86 | + - Test now runs and passes successfully |
| 87 | + |
| 88 | +## Technical Details |
| 89 | + |
| 90 | +### Why IComparable? |
| 91 | +- `IComparable.CompareTo()` is implemented by all comparable types (Int32, Decimal, String, etc.) |
| 92 | +- Returns: -1 (less), 0 (equal), or 1 (greater) |
| 93 | +- Works with runtime-typed values without requiring compile-time type information |
| 94 | +- Handles boxing/unboxing automatically |
| 95 | + |
| 96 | +### Expression Tree Pattern |
| 97 | +```csharp |
| 98 | +// Convert to IComparable |
| 99 | +var left = Expression.Convert(objectValue, typeof(IComparable)); |
| 100 | +var right = objectValue2; // Keep as object for CompareTo parameter |
| 101 | +
|
| 102 | +// Call CompareTo |
| 103 | +var compareCall = Expression.Call(left, compareToMethod, right); |
| 104 | + |
| 105 | +// Compare result with 0 |
| 106 | +var zero = Expression.Constant(0); |
| 107 | +var result = Expression.GreaterThan(compareCall, zero); // compareTo > 0 |
| 108 | +``` |
| 109 | + |
| 110 | +## Future Considerations |
| 111 | +- Consider caching compiled WHERE filters to avoid recompilation |
| 112 | +- Could optimize by detecting numeric types at parse time |
| 113 | +- May want to add support for custom IComparer implementations |
| 114 | +- Consider adding telemetry to track compilation success rate |
| 115 | + |
| 116 | +## Related Issues |
| 117 | +- Fixes the "141/200 mystery" where compiled queries were falling back to SqlParser |
| 118 | +- Improves performance of repeated SELECT queries by 2.5-26x |
| 119 | +- Enables true compiled query execution using expression trees |
0 commit comments