# Phase 10: Unified Expression API Refactor

## Overview

This phase addresses the inconsistent expression handling throughout the ORM layer. Currently, the codebase uses raw strings for expressions in many places instead of `Invercargill.Expressions.Expression` objects.

This violates the original design intent where **all expressions should be `Invercargill.Expressions.Expression` objects**.

## Problem Statement

### Current Issues

1. **Dual API Surface**: The `where()` and `where_expr()` methods create confusion about which to use
2. **String-based Internal Storage**: `_where_clauses` stores raw strings instead of Expression objects
3. **ProjectionQuery Limitation**: `ProjectionQuery.where_expr()` throws an error, making Expression-based filtering impossible for projections
4. **Design Violation**: The Query class and Projection system have created their own ad-hoc expression language instead of using `Invercargill.Expressions`
5. **Widespread String Usage**: Expressions are stored and passed as strings throughout the projection system

### Affected Areas

| Area | Files | String Expression Usage |
|------|-------|------------------------|
| Query API | `query.vala`, `entity-query.vala`, `projection-query.vala` | `where()`, `or_where()`, `order_by()`, `order_by_desc()` |
| Projection Builder | `projection-builder.vala` | `join()`, `select()`, `select_nested()`, `select_many()`, `group_by()` |
| Selection Types | `selection-types.vala` | `ScalarSelection.expression`, `NestedProjectionSelection.entry_point_expression`, `CollectionProjectionSelection.entry_point_expression` |
| Projection Definition | `projection-definition.vala` | `JoinDefinition.join_condition`, `group_by_expressions` |
| SQL Builder | `projection-sql-builder.vala` | `build()`, `build_with_split()`, `translate_expression()` |
| Variable Translator | `variable-translator.vala` | `translate_expression()` |
| Aggregate Analyzer | `aggregate-analyzer.vala` | `analyze()`, `contains_aggregate()`, `split_expression()` |
| Friendly Name Resolver | `friendly-name-resolver.vala` | `resolve_to_expression()` returns string |

### Current Code Structure

```
Query<T>
├── where(string expression)         → stores string in _where_clauses
├── or_where(string expression)      → stores string in _where_clauses  
├── where_expr(Expression expression) → abstract, implemented differently
│
├── EntityQuery<T>
│   ├── where(string) → parses to Expression, stores in _filter
│   └── where_expr(Expression) → stores in _filter
│
└── ProjectionQuery<T>
    ├── where(string) → stores string, passes to ProjectionSqlBuilder
    └── where_expr(Expression) → THROWS ERROR!
```

## Proposed Solution

### New API Design

All `string expression` parameters will be converted to `Expression expression` parameters. The `expr()` convenience function from `Invercargill.Expressions` will be used to create expressions ergonomically.

```
Query<T>
├── where(Expression expression)      → stores Expression in _where_expressions
├── or_where(Expression expression)   → stores Expression in _where_expressions
├── order_by(Expression expression)   → stores Expression for ordering
├── order_by_desc(Expression expression) → stores Expression for ordering DESC
│
├── EntityQuery<T>
│   └── where(Expression) → combines with existing _filter using AND/OR
│
└── ProjectionQuery<T>
    └── where(Expression) → translates Expression through VariableTranslator
```

### The `expr()` Convenience Function

From `Invercargill.Expressions`:
```vala
public Expression expr(string expression, Element first, ...) throws ExpressionError {
    return ExpressionParser.parse_with_params(expression, Wrap.va_list<Element>(first, va_list()));
}
```

Usage examples:
```vala
// Simple expression
var results = session.query<User>()
    .where(expr("age > 25", new NativeElement<int?>(25)))
    .materialise();

// With parameters using $0, $1 syntax
var results = session.query<User>()
    .where(expr("age > $0 && name == $1", 
        new NativeElement<int?>(25),
        new NativeElement<string>("Alice")))
    .materialise();

// Reusable expressions
var active_filter = expr("is_active == true", new NativeElement<bool>(true));
var results = session.query<User>()
    .where(active_filter)
    .materialise();
```

## Implementation Plan

### Step 1: Update Query<T> Base Class

**File**: [`src/orm/query.vala`](src/orm/query.vala)

Changes:
1. Change `_where_clauses: Vector<string>` to `_where_expressions: Vector<Expression>`
2. Change `where(string expression)` to `where(Expression expression)`
3. Change `or_where(string expression)` to `or_where(Expression expression)`
4. Change `order_by(string expression)` to `order_by(Expression expression)`
5. Change `order_by_desc(string expression)` to `order_by_desc(Expression expression)`
6. Remove abstract `where_expr(Expression expression)` method
7. Update `get_combined_where()` to work with Expressions

```vala
// Before
protected Vector<string> _where_clauses;
public virtual Query<T> where(string expression) { ... }
public abstract Query<T> where_expr(Expression expression);

// After
protected Vector<Expression> _where_expressions;
public virtual Query<T> where(Expression expression) { ... }
// where_expr() removed
```

### Step 2: Update OrderByClause

**File**: [`src/orm/query.vala`](src/orm/query.vala)

Changes:
1. Change `expression: string` to `expression: Expression`

```vala
// Before
public class OrderByClause : Object {
    public string expression { get; construct; }
    public OrderByClause(string expression, bool descending) { ... }
}

// After
public class OrderByClause : Object {
    public Expression expression { get; construct; }
    public OrderByClause(Expression expression, bool descending) { ... }
}
```

### Step 3: Update EntityQuery<T>

**File**: [`src/orm/entity-query.vala`](src/orm/entity-query.vala)

Changes:
1. Remove `where(string expression)` override - base implementation now handles Expressions
2. Remove `where_expr(Expression expression)` override
3. Update `build_select_sql()` to handle Expression-based WHERE clauses
4. Combine multiple WHERE expressions using AND/OR

### Step 4: Update ProjectionQuery<T>

**File**: [`src/orm/projections/projection-query.vala`](src/orm/projections/projection-query.vala)

Changes:
1. Remove `where_expr()` that throws error
2. Update `materialise()` and `materialise_async()` to translate Expressions
3. Use `VariableTranslator` to convert Expressions for projection context

### Step 5: Update ProjectionBuilder

**File**: [`src/orm/projections/projection-builder.vala`](src/orm/projections/projection-builder.vala)

Changes:
1. `join<TEntity>(string variable_name, string join_condition)` → `join<TEntity>(string variable_name, Expression join_condition)`
2. `select<TValue>(string friendly_name, string expression, ...)` → `select<TValue>(string friendly_name, Expression expression, ...)`
3. `select_nested<TNested>(string friendly_name, string entry_point_expression, ...)` → `select_nested<TNested>(string friendly_name, Expression entry_point_expression, ...)`
4. `select_many<TItem>(string friendly_name, string entry_point_expression, ...)` → `select_many<TItem>(string friendly_name, Expression entry_point_expression, ...)`
5. `group_by(params string[] expressions)` → `group_by(params Expression[] expressions)`

### Step 6: Update Selection Types

**File**: [`src/orm/projections/selection-types.vala`](src/orm/projections/selection-types.vala)

Changes:
1. `ScalarSelection.expression: string` → `Expression`
2. `NestedProjectionSelection.entry_point_expression: string` → `Expression`
3. `CollectionProjectionSelection.entry_point_expression: string` → `Expression`

### Step 7: Update ProjectionDefinition and JoinDefinition

**File**: [`src/orm/projections/projection-definition.vala`](src/orm/projections/projection-definition.vala)

Changes:
1. `JoinDefinition.join_condition: string` → `Expression`
2. `ProjectionDefinition.group_by_expressions: Vector<string>` → `Vector<Expression>`
3. `add_group_by(string expression)` → `add_group_by(Expression expression)`

### Step 8: Update ProjectionSqlBuilder

**File**: [`src/orm/projections/projection-sql-builder.vala`](src/orm/projections/projection-sql-builder.vala)

Changes:
1. `build(string? where_expression, ...)` → `build(Expression? where_expression, ...)`
2. `build_with_split(string? where_expression, ...)` → `build_with_split(Expression? where_expression, ...)`
3. `translate_expression(string expression)` → `translate_expression(Expression expression)`
4. Update internal methods to work with Expression objects

### Step 9: Update VariableTranslator

**File**: [`src/orm/projections/variable-translator.vala`](src/orm/projections/variable-translator.vala)

Changes:
1. `translate_expression(string expression)` → `translate_expression(Expression expression)`
2. Update to use `ExpressionToSqlVisitor` pattern for translation
3. May need to accept `Expression` and output SQL directly

### Step 10: Update AggregateAnalyzer

**File**: [`src/orm/projections/aggregate-analyzer.vala`](src/orm/projections/aggregate-analyzer.vala)

Changes:
1. `analyze(string expression)` → `analyze(Expression expression)`
2. `contains_aggregate(string expression)` → `contains_aggregate(Expression expression)`
3. `split_expression(string expression)` → `split_expression(Expression expression)`
4. Update internal scanning to work with Expression trees instead of strings

### Step 11: Update FriendlyNameResolver

**File**: [`src/orm/projections/friendly-name-resolver.vala`](src/orm/projections/friendly-name-resolver.vala)

Changes:
1. `resolve_to_expression(string friendly_name)` → Returns `Expression?` instead of `string?`
2. Update internal caching to store Expression objects

### Step 12: Update Tests

**Files**: 
- [`src/tests/orm-test.vala`](src/tests/orm-test.vala)
- [`src/tests/projection-test.vala`](src/tests/projection-test.vala)

Changes:
1. Import `Invercargill.Expressions` namespace
2. Convert all `where("string")` calls to `where(expr("string"))`
3. Convert all `order_by("column")` calls to use Expression-based approach
4. Convert all projection builder calls to use `expr()`
5. Add tests for parameterized expressions using `$0`, `$1` syntax

Example test conversions:

```vala
// Before
var results = session.query<TestUser>()
    .where("age > 28")
    .materialise();

// After
var results = session.query<TestUser>()
    .where(expr("age > $0", new NativeElement<int?>(28)))
    .materialise();
```

```vala
// Before - projection builder
ctx.registry.register_projection<UserOrderStats>(new ProjectionBuilder<UserOrderStats>(ctx.registry)
    .source<ProjTestUser>("u")
    .join<ProjTestOrder>("o", "u.id == o.user_id")
    .group_by("u.id")
    .select<int64?>("user_id", "u.id", (x, v) => x.user_id = v)
    .build()
);

// After - projection builder
ctx.registry.register_projection<UserOrderStats>(new ProjectionBuilder<UserOrderStats>(ctx.registry)
    .source<ProjTestUser>("u")
    .join<ProjTestOrder>("o", expr("u.id == o.user_id"))
    .group_by(expr("u.id"))
    .select<int64?>("user_id", expr("u.id"), (x, v) => x.user_id = v)
    .build()
);
```

### Step 13: Update Demo Application

**File**: [`examples/demo.vala`](examples/demo.vala)

Changes:
1. Add `using Invercargill.Expressions;`
2. Convert all query expressions to use `expr()`

```vala
// Before
var users_over_25 = session.query<User>()
    .where("age > 25")
    .materialise();

// After
var users_over_25 = session.query<User>()
    .where(expr("age > $0", new NativeElement<int?>(25)))
    .materialise();
```

## Detailed File Changes

### Files to Modify

| File | Changes |
|------|---------|
| `src/orm/query.vala` | Core API - where(), or_where(), order_by(), OrderByClause |
| `src/orm/entity-query.vala` | Remove overrides, update SQL building |
| `src/orm/projections/projection-query.vala` | Enable Expression support |
| `src/orm/projections/projection-builder.vala` | join(), select(), group_by() signatures |
| `src/orm/projections/selection-types.vala` | expression property types |
| `src/orm/projections/projection-definition.vala` | JoinDefinition, group_by_expressions |
| `src/orm/projections/projection-sql-builder.vala` | build() signatures, translate_expression() |
| `src/orm/projections/variable-translator.vala` | translate_expression() signature |
| `src/orm/projections/aggregate-analyzer.vala` | analyze(), contains_aggregate(), split_expression() |
| `src/orm/projections/friendly-name-resolver.vala` | resolve_to_expression() return type |
| `src/tests/orm-test.vala` | Convert all tests |
| `src/tests/projection-test.vala` | Convert all tests |
| `examples/demo.vala` | Update demo |

### Files to Review (may need changes)

| File | Reason |
|------|--------|
| `src/expressions/expression-to-sql-visitor.vala` | Ensure it works with projection contexts |

## Migration Guide for Users

### Before (Phase 9 and earlier)
```vala
// String-based expressions
var users = session.query<User>()
    .where("age > 18")
    .order_by("name")
    .materialise();

// Projection queries
var stats = session.query<UserOrderStats>()
    .where("user_id > 100")
    .materialise();

// Projection definitions
registry.register_projection<UserStats>(new ProjectionBuilder<UserStats>(registry)
    .source<User>("u")
    .join<Order>("o", "u.id == o.user_id")
    .group_by("u.id")
    .select<int64?>("user_id", "u.id", (x, v) => x.user_id = v)
    .build()
);
```

### After (Phase 10)
```vala
using Invercargill.Expressions;

// Expression-based queries
var users = session.query<User>()
    .where(expr("age > $0", new NativeElement<int?>(18)))
    .order_by(expr("name"))
    .materialise();

// Projection queries work the same way
var stats = session.query<UserOrderStats>()
    .where(expr("user_id > $0", new NativeElement<int?>(100)))
    .materialise();

// Projection definitions with Expressions
registry.register_projection<UserStats>(new ProjectionBuilder<UserStats>(registry)
    .source<User>("u")
    .join<Order>("o", expr("u.id == o.user_id"))
    .group_by(expr("u.id"))
    .select<int64?>("user_id", expr("u.id"), (x, v) => x.user_id = v)
    .build()
);

// Simple expressions without parameters
var active = session.query<User>()
    .where(expr("is_active == true"))
    .materialise();
```

## Benefits

1. **Type Safety**: Expressions are parsed at construction time, catching errors early
2. **Consistency**: Single API across EntityQuery and ProjectionQuery
3. **Composability**: Expressions can be stored, combined, and reused
4. **Parameterized Queries**: Native support for parameterized expressions via `$0`, `$1` syntax
5. **Design Alignment**: Properly uses `Invercargill.Expressions` throughout

## Risks and Mitigations

### Risk 1: Breaking Change
- **Impact**: All existing code using string expressions will break
- **Mitigation**: This is a major version change; provide clear migration guide

### Risk 2: Verbosity
- **Impact**: `expr("age > $0", new NativeElement<int?>(18))` is more verbose than `"age > 18"`
- **Mitigation**: Document common patterns; simple expressions can omit parameters

### Risk 3: Projection Expression Translation
- **Impact**: ProjectionQuery needs to translate expressions differently than EntityQuery
- **Mitigation**: Leverage existing `VariableTranslator` infrastructure

## Testing Strategy

1. **Unit Tests**: Update all existing tests to use `expr()`
2. **Expression Parsing Tests**: Verify `expr()` handles all expression types
3. **Integration Tests**: Ensure EntityQuery and ProjectionQuery produce correct SQL
4. **Parameter Tests**: Test `$0`, `$1`, etc. parameter substitution
5. **Error Handling**: Verify proper errors for invalid expressions

## Success Criteria

- [ ] `where_expr()` method removed from all Query classes
- [ ] `where()` accepts `Expression` parameter in all Query classes
- [ ] `order_by()` and `order_by_desc()` accept `Expression` parameters
- [ ] `ProjectionBuilder` methods accept `Expression` parameters
- [ ] Selection types store `Expression` objects
- [ ] `ProjectionSqlBuilder` accepts `Expression` for WHERE
- [ ] All analyzers and translators work with `Expression` objects
- [ ] ProjectionQuery supports Expression-based filtering
- [ ] All tests pass with new Expression-based API
- [ ] Demo application updated and working
- [ ] No string-based expression storage anywhere in ORM/Projections

## Estimated Scope

| Category | Count |
|----------|-------|
| Core files to modify | 10 |
| Test files to update | 2 |
| Example files to update | 1 |
| Breaking changes | Yes (major API change) |

## Dependencies

- `Invercargill.Expressions.expr()` function
- `Invercargill.Expressions.ExpressionParser`
- `Invercargill.Expressions.Expression` type hierarchy
- `Invercargill.NativeElement<T>` for parameter values

## Implementation Order

1. **Core Query Classes** - Foundation for everything else
2. **Selection Types** - Data structures used by builders
3. **Projection Builder** - API surface for defining projections
4. **Projection Definition** - Storage of projection configuration
5. **SQL Builder** - Translation to SQL
6. **Analyzers and Translators** - Supporting infrastructure
7. **Tests** - Verify all changes work correctly
8. **Demo** - Update example application