refactor(orm): consolidate expression string conversion to library method

Remove duplicate ExpressionStringVisitor implementations across multiple files
in favor of using the library's Expression.to_expression_string() method.

Update SplitExpression to store Expression objects directly instead of strings,
preserving type information and parameter values throughout the expression
processing pipeline. This enables proper handling of ParameterExpression nodes
which contain actual values passed via expr() calls.

Key changes:
- Remove ExpressionStringVisitor classes from expression-to-sql-visitor.vala
  and aggregate-analyzer.vala
- Change SplitExpression properties from string to Expression types
- Update VariableTranslationVisitor with visit tracking to prevent
  double-visiting when library's visitor pattern calls children
- Add ParameterExpression handling in visitors for proper value output
- Fix dictionary access patterns to use has() before get()

src/expressions/expression-to-sql-visitor.vala

@@ -246,10 +246,8 @@ namespace InvercargillSql.Expressions {
                     if (_variable_translator != null) {
                         _sql.append(_variable_translator.translate_expression(resolved));
                     } else {
-                        // Convert Expression to string using ExpressionStringVisitor
-                        var string_visitor = new ExpressionStringVisitor();
-                        resolved.accept(string_visitor);
-                        _sql.append(string_visitor.get_string());
+                        // Convert Expression to string using library method
+                        _sql.append(resolved.to_expression_string());
                     }
                     return;
                 }
@@ -535,10 +533,8 @@ namespace InvercargillSql.Expressions {
             if (_variable_translator != null) {
                 return _variable_translator.translate_expression(expression);
             }
-            // Convert Expression to string using ExpressionStringVisitor
-            var string_visitor = new ExpressionStringVisitor();
-            expression.accept(string_visitor);
-            return string_visitor.get_string();
+            // Convert Expression to string using library method
+            return expression.to_expression_string();
         }
         
         /**
@@ -569,9 +565,8 @@ namespace InvercargillSql.Expressions {
             if (_friendly_name_resolver != null) {
                 Expression? expr = _friendly_name_resolver.resolve_to_expression(friendly_name);
                 if (expr != null) {
-                    var string_visitor = new ExpressionStringVisitor();
-                    expr.accept(string_visitor);
-                    return string_visitor.get_string();
+                    // Convert Expression to string using library method
+                    return expr.to_expression_string();
                 }
             }
             return null;
@@ -610,169 +605,30 @@ namespace InvercargillSql.Expressions {
         
         /**
          * Converts an expression tree to a string representation.
-         * 
+         *
          * @param expr The expression to convert
          * @return The string representation
          */
         private string expression_to_string(Expression expr) {
-            var visitor = new ExpressionStringVisitor();
-            expr.accept(visitor);
-            return visitor.get_string();
-        }
-    }
-    
-    /**
-     * Internal visitor for converting expressions to strings.
-     * 
-     * Used by ExpressionToSqlVisitor.contains_aggregate() to get a string
-     * representation of an expression tree.
-     */
-    internal class ExpressionStringVisitor : Object, ExpressionVisitor {
-        
-        private StringBuilder _builder;
-        
-        public ExpressionStringVisitor() {
-            _builder = new StringBuilder();
-        }
-        
-        public string get_string() {
-            return _builder.str;
-        }
-        
-        public void visit_binary(BinaryExpression expr) {
-            _builder.append("(");
-            expr.left.accept(this);
-            _builder.append(get_operator_string(expr.op));
-            expr.right.accept(this);
-            _builder.append(")");
-        }
-        
-        private string get_operator_string(BinaryOperator op) {
-            switch (op) {
-                case BinaryOperator.EQUAL: return " == ";
-                case BinaryOperator.NOT_EQUAL: return " != ";
-                case BinaryOperator.GREATER_THAN: return " > ";
-                case BinaryOperator.GREATER_EQUAL: return " >= ";
-                case BinaryOperator.LESS_THAN: return " < ";
-                case BinaryOperator.LESS_EQUAL: return " <= ";
-                case BinaryOperator.AND: return " && ";
-                case BinaryOperator.OR: return " || ";
-                case BinaryOperator.ADD: return " + ";
-                case BinaryOperator.SUBTRACT: return " - ";
-                case BinaryOperator.MULTIPLY: return " * ";
-                case BinaryOperator.DIVIDE: return " / ";
-                case BinaryOperator.MODULO: return " % ";
-                default: return " ? ";
-            }
-        }
-        
-        public void visit_property(PropertyExpression expr) {
-            _builder.append(".");
-            _builder.append(expr.property_name);
+            return expr.to_expression_string();
         }
         
-        public void visit_literal(LiteralExpression expr) {
-            var value = expr.value;
-            if (value.is_null()) {
-                _builder.append("null");
-            } else if (value.assignable_to_type(typeof(string))) {
-                string? s = null;
-                if (value.try_get_as<string>(out s) && s != null) {
-                    _builder.append("\"");
-                    _builder.append(s.replace("\"", "\\\""));
-                    _builder.append("\"");
-                }
-            } else if (value.assignable_to_type(typeof(int64))) {
-                int64? i = null;
-                if (value.try_get_as<int64?>(out i) && i != null) {
-                    _builder.append(i.to_string());
-                }
-            } else if (value.assignable_to_type(typeof(double))) {
-                double? d = null;
-                if (value.try_get_as<double?>(out d) && d != null) {
-                    _builder.append(d.to_string());
-                }
-            } else if (value.assignable_to_type(typeof(bool))) {
-                bool? b = null;
-                if (value.try_get_as<bool?>(out b) && b != null) {
-                    _builder.append(b ? "true" : "false");
-                }
-            } else {
-                _builder.append(value.to_string());
-            }
-        }
-        
-        public void visit_unary(UnaryExpression expr) {
-            if (expr.operator == UnaryOperator.NOT) {
-                _builder.append("!");
-            } else if (expr.operator == UnaryOperator.NEGATE) {
-                _builder.append("-");
-            }
-        }
-        
-        public void visit_ternary(TernaryExpression expr) {
-            expr.condition.accept(this);
-            _builder.append(" ? ");
-            expr.true_expression.accept(this);
-            _builder.append(" : ");
-            expr.false_expression.accept(this);
-        }
-        
-        public void visit_lambda(LambdaExpression expr) {
-            _builder.append(expr.parameter_name);
-            _builder.append(" => ");
-            expr.body.accept(this);
-        }
-        
-        public void visit_bracketed(BracketedExpression expr) {
-            _builder.append("(");
-            expr.inner.accept(this);
-            _builder.append(")");
-        }
-        
-        public void visit_variable(VariableExpression expr) {
-            _builder.append(expr.variable_name);
-        }
-        
-        public void visit_function_call(FunctionCallExpression expr) {
-            expr.target.accept(this);
-            _builder.append(".");
-            _builder.append(expr.function_name);
-            _builder.append("(");
-            if (expr.arguments != null) {
-                bool first = true;
-                foreach (var arg in expr.arguments) {
-                    if (!first) _builder.append(", ");
-                    arg.accept(this);
-                    first = false;
-                }
-            }
-            _builder.append(")");
-        }
-        
-        public void visit_global_function_call(GlobalFunctionCallExpression expr) {
-            _builder.append(expr.function_name);
-            _builder.append("(");
-            if (expr.arguments != null) {
-                bool first = true;
-                foreach (var arg in expr.arguments) {
-                    if (!first) _builder.append(", ");
-                    arg.accept(this);
-                    first = false;
-                }
-            }
-            _builder.append(")");
-        }
-        
-        public void visit_lot_literal(LotLiteralExpression expr) {
-            _builder.append("[");
-            bool first = true;
-            foreach (var element in expr.elements) {
-                if (!first) _builder.append(", ");
-                element.accept(this);
-                first = false;
-            }
-            _builder.append("]");
+        /**
+         * Visits a parameter expression.
+         * 
+         * Parameter expressions represent values passed at parse time
+         * using positional placeholders like $0, $1, etc.
+         * 
+         * @param expr The parameter expression to visit
+         */
+        public void visit_parameter(ParameterExpression expr) {
+            // Output the parameter as a SQL parameter placeholder
+            _param_counter++;
+            string param_name = @"p$(_param_counter)";
+            _sql.append(":");
+            _sql.append(param_name);
+            _parameters.add(expr.value);
+            _parameter_names.add(param_name);
         }
     }
 }

src/orm/projections/aggregate-analyzer.vala

@@ -58,41 +58,48 @@ namespace InvercargillSql.Orm.Projections {
     
     /**
      * Result of splitting an expression into aggregate and non-aggregate parts.
-     * 
+     *
      * When a compound expression contains both aggregate and non-aggregate conditions,
      * this class holds the separated parts for use in WHERE and HAVING clauses.
-     * 
+     *
+     * The parts are stored as Expression objects to preserve all type information,
+     * including parameter values in ParameterExpression nodes.
+     *
      * Example:
      * {{{
      * // Input: "user_id > 100 && order_count >= 5"
      * // Where order_count is an aggregate like COUNT(o.id)
-     * split.non_aggregate_part == "user_id > 100"  // Goes to WHERE
-     * split.aggregate_part == "order_count >= 5"   // Goes to HAVING
+     * split.non_aggregate_expression // Expression for "user_id > 100" - goes to WHERE
+     * split.aggregate_expression    // Expression for "order_count >= 5" - goes to HAVING
      * }}}
      */
     public class SplitExpression : Object {
         /**
          * The non-aggregate part of the expression.
-         * 
+         *
          * This part should be applied to the WHERE clause.
          * May be null if the entire expression contains aggregates.
+         *
+         * Stored as Expression to preserve parameter values and type information.
          */
-        public string? non_aggregate_part { get; construct; }
+        public Expression? non_aggregate_expression { get; construct; }
         
         /**
          * The aggregate part of the expression.
-         * 
+         *
          * This part should be applied to the HAVING clause.
          * May be null if the expression contains no aggregates.
+         *
+         * Stored as Expression to preserve parameter values and type information.
          */
-        public string? aggregate_part { get; construct; }
+        public Expression? aggregate_expression { get; construct; }
         
         /**
          * Indicates if the expression combines aggregate and non-aggregate parts with OR.
-         * 
+         *
          * When true, a subquery wrapper is required because SQL cannot mix
          * WHERE and HAVING conditions with OR logic.
-         * 
+         *
          * Example requiring subquery:
          * {{{
          * // "user_id > 100 || COUNT(o.id) >= 5"
@@ -104,19 +111,19 @@ namespace InvercargillSql.Orm.Projections {
         
         /**
          * Creates a new SplitExpression.
-         * 
-         * @param non_aggregate_part The WHERE clause part (or null)
-         * @param aggregate_part The HAVING clause part (or null)
+         *
+         * @param non_aggregate_expression The WHERE clause Expression (or null)
+         * @param aggregate_expression The HAVING clause Expression (or null)
          * @param needs_subquery True if subquery wrapper is needed
          */
         public SplitExpression(
-            string? non_aggregate_part,
-            string? aggregate_part,
+            Expression? non_aggregate_expression,
+            Expression? aggregate_expression,
             bool needs_subquery
         ) {
             Object(
-                non_aggregate_part: non_aggregate_part,
-                aggregate_part: aggregate_part,
+                non_aggregate_expression: non_aggregate_expression,
+                aggregate_expression: aggregate_expression,
                 needs_subquery: needs_subquery
             );
         }
@@ -251,12 +258,15 @@ namespace InvercargillSql.Orm.Projections {
         
         /**
          * Splits a parsed expression tree into aggregate and non-aggregate parts.
-         * 
+         *
          * This method handles the expression tree structure properly, correctly
          * identifying AND vs OR combinations and nested expressions.
-         * 
+         *
+         * All parts are preserved as Expression objects to maintain type information
+         * and parameter values.
+         *
          * @param expr The parsed expression tree
-         * @return A SplitExpression with the separated parts
+         * @return A SplitExpression with the separated parts as Expression objects
          */
         private SplitExpression split_expression_tree(Expression expr) {
             // Check if it's a binary expression (potential AND/OR)
@@ -269,7 +279,7 @@ namespace InvercargillSql.Orm.Projections {
                     var right_split = split_expression_tree(binary.right);
                     
                     // Combine results
-                    return combine_and_splits(left_split, right_split);
+                    return combine_and_splits(left_split, right_split, binary);
                 }
                 
                 // Handle OR - check if mixed
@@ -281,10 +291,10 @@ namespace InvercargillSql.Orm.Projections {
                     if (left_has_agg == right_has_agg) {
                         if (left_has_agg) {
                             // Both aggregate - whole thing goes to HAVING
-                            return new SplitExpression(null, expr_to_string(expr), false);
+                            return new SplitExpression(null, expr, false);
                         } else {
                             // Both non-aggregate - whole thing goes to WHERE
-                            return new SplitExpression(expr_to_string(expr), null, false);
+                            return new SplitExpression(expr, null, false);
                         }
                     }
                     
@@ -295,46 +305,59 @@ namespace InvercargillSql.Orm.Projections {
             
             // For non-binary or other operators, check if it contains aggregates
             bool has_aggregate = expression_contains_aggregate(expr);
-            string expr_str = expr_to_string(expr);
             
             if (has_aggregate) {
-                return new SplitExpression(null, expr_str, false);
+                return new SplitExpression(null, expr, false);
             } else {
-                return new SplitExpression(expr_str, null, false);
+                return new SplitExpression(expr, null, false);
             }
         }
         
         /**
          * Combines two SplitExpressions from AND-connected expressions.
-         * 
+         *
+         * When both sides have expressions of the same type (aggregate or non-aggregate),
+         * they are combined into a new BinaryExpression with AND operator.
+         *
          * @param left The split from the left side of AND
          * @param right The split from the right side of AND
+         * @param original_binary The original BinaryExpression (used to preserve structure when needed)
          * @return A combined SplitExpression
          */
-        private SplitExpression combine_and_splits(SplitExpression left, SplitExpression right) {
+        private SplitExpression combine_and_splits(SplitExpression left, SplitExpression right, BinaryExpression original_binary) {
             // If either needs subquery, propagate that
             if (left.needs_subquery || right.needs_subquery) {
                 return new SplitExpression(null, null, true);
             }
             
             // Combine non-aggregate parts
-            string? non_agg = null;
-            if (left.non_aggregate_part != null && right.non_aggregate_part != null) {
-                non_agg = @"($(left.non_aggregate_part) AND $(right.non_aggregate_part))";
-            } else if (left.non_aggregate_part != null) {
-                non_agg = left.non_aggregate_part;
-            } else if (right.non_aggregate_part != null) {
-                non_agg = right.non_aggregate_part;
+            Expression? non_agg = null;
+            if (left.non_aggregate_expression != null && right.non_aggregate_expression != null) {
+                // Both sides have non-aggregate parts - combine with AND
+                non_agg = new BinaryExpression(
+                    left.non_aggregate_expression,
+                    right.non_aggregate_expression,
+                    BinaryOperator.AND
+                );
+            } else if (left.non_aggregate_expression != null) {
+                non_agg = left.non_aggregate_expression;
+            } else if (right.non_aggregate_expression != null) {
+                non_agg = right.non_aggregate_expression;
             }
             
             // Combine aggregate parts
-            string? agg = null;
-            if (left.aggregate_part != null && right.aggregate_part != null) {
-                agg = @"($(left.aggregate_part) AND $(right.aggregate_part))";
-            } else if (left.aggregate_part != null) {
-                agg = left.aggregate_part;
-            } else if (right.aggregate_part != null) {
-                agg = right.aggregate_part;
+            Expression? agg = null;
+            if (left.aggregate_expression != null && right.aggregate_expression != null) {
+                // Both sides have aggregate parts - combine with AND
+                agg = new BinaryExpression(
+                    left.aggregate_expression,
+                    right.aggregate_expression,
+                    BinaryOperator.AND
+                );
+            } else if (left.aggregate_expression != null) {
+                agg = left.aggregate_expression;
+            } else if (right.aggregate_expression != null) {
+                agg = right.aggregate_expression;
             }
             
             return new SplitExpression(non_agg, agg, false);
@@ -342,7 +365,7 @@ namespace InvercargillSql.Orm.Projections {
         
         /**
          * Checks if an expression tree contains aggregate functions.
-         * 
+         *
          * @param expr The expression tree to check
          * @return True if aggregates are found
          */
@@ -351,18 +374,6 @@ namespace InvercargillSql.Orm.Projections {
             expr.accept(visitor);
             return visitor.found_aggregate;
         }
-        
-        /**
-         * Converts an expression tree back to a string representation.
-         * 
-         * @param expr The expression tree to convert
-         * @return The string representation
-         */
-        private string expr_to_string(Expression expr) {
-            var visitor = new ExpressionStringVisitor();
-            expr.accept(visitor);
-            return visitor.get_string();
-        }
     }
     
     /**
@@ -595,181 +606,4 @@ namespace InvercargillSql.Orm.Projections {
             // Collection literals don't contain aggregates
         }
     }
-    
-    /**
-     * Expression visitor that converts an expression tree back to a string.
-     * 
-     * This is used to reconstruct expression strings after splitting.
-     */
-    internal class ExpressionStringVisitor : Object, ExpressionVisitor {
-        
-        private StringBuilder _builder;
-        private string? _pending_property_name = null;  // Property name waiting for target variable
-        
-        /**
-         * Creates a new ExpressionStringVisitor.
-         */
-        public ExpressionStringVisitor() {
-            _builder = new StringBuilder();
-            _pending_property_name = null;
-        }
-        
-        /**
-         * Gets the string representation of the visited expression.
-         * 
-         * @return The expression as a string
-         */
-        public string get_string() {
-            return _builder.str;
-        }
-        
-        public void visit_binary(BinaryExpression expr) {
-            _builder.append("(");
-            expr.left.accept(this);
-            _builder.append(get_operator_string(expr.op));
-            expr.right.accept(this);
-            _builder.append(")");
-        }
-        
-        private string get_operator_string(BinaryOperator op) {
-            switch (op) {
-                case BinaryOperator.EQUAL: return " == ";
-                case BinaryOperator.NOT_EQUAL: return " != ";
-                case BinaryOperator.GREATER_THAN: return " > ";
-                case BinaryOperator.GREATER_EQUAL: return " >= ";
-                case BinaryOperator.LESS_THAN: return " < ";
-                case BinaryOperator.LESS_EQUAL: return " <= ";
-                case BinaryOperator.AND: return " && ";
-                case BinaryOperator.OR: return " || ";
-                case BinaryOperator.ADD: return " + ";
-                case BinaryOperator.SUBTRACT: return " - ";
-                case BinaryOperator.MULTIPLY: return " * ";
-                case BinaryOperator.DIVIDE: return " / ";
-                case BinaryOperator.MODULO: return " % ";
-                default: return " ? ";
-            }
-        }
-        
-        public void visit_property(PropertyExpression expr) {
-            // The library's PropertyExpression.accept() calls:
-            // 1. visit_property() - our method here
-            // 2. target.accept() - which calls visit_variable()
-            // So visit_variable is called AFTER visit_property.
-            // We save the property name and output it when visit_variable is called.
-            _pending_property_name = expr.property_name;
-        }
-        
-        public void visit_literal(LiteralExpression expr) {
-            var value = expr.value;
-            if (value.assignable_to_type(typeof(string))) {
-                string? s = null;
-                if (value.try_get_as<string>(out s) && s != null) {
-                    _builder.append("\"");
-                    _builder.append(s);
-                    _builder.append("\"");
-                }
-            } else if (value.assignable_to_type(typeof(int64))) {
-                int64? i = null;
-                if (value.try_get_as<int64?>(out i) && i != null) {
-                    _builder.append(i.to_string());
-                }
-            } else if (value.assignable_to_type(typeof(double))) {
-                double? d = null;
-                if (value.try_get_as<double?>(out d) && d != null) {
-                    _builder.append(d.to_string());
-                }
-            } else if (value.assignable_to_type(typeof(bool))) {
-                bool? b = null;
-                if (value.try_get_as<bool>(out b) && b != null) {
-                    _builder.append(b ? "true" : "false");
-                }
-            } else if (value.is_null()) {
-                _builder.append("null");
-            } else {
-                _builder.append(value.to_string());
-            }
-        }
-        
-        public void visit_unary(UnaryExpression expr) {
-            if (expr.operator == UnaryOperator.NOT) {
-                _builder.append("!");
-            } else if (expr.operator == UnaryOperator.NEGATE) {
-                _builder.append("-");
-            }
-        }
-        
-        public void visit_ternary(TernaryExpression expr) {
-            expr.condition.accept(this);
-            _builder.append(" ? ");
-            expr.true_expression.accept(this);
-            _builder.append(" : ");
-            expr.false_expression.accept(this);
-        }
-        
-        public void visit_lambda(LambdaExpression expr) {
-            _builder.append(expr.parameter_name);
-            _builder.append(" => ");
-            expr.body.accept(this);
-        }
-        
-        public void visit_bracketed(BracketedExpression expr) {
-            _builder.append("(");
-            expr.inner.accept(this);
-            _builder.append(")");
-        }
-        
-        public void visit_variable(VariableExpression expr) {
-            // If there's a pending property name, this variable is the target of a property expression
-            // Output: variable.property_name
-            if (_pending_property_name != null) {
-                _builder.append(expr.variable_name);
-                _builder.append(".");
-                _builder.append(_pending_property_name);
-                _pending_property_name = null;  // Clear it
-            } else {
-                _builder.append(expr.variable_name);
-            }
-        }
-        
-        public void visit_function_call(FunctionCallExpression expr) {
-            expr.target.accept(this);
-            _builder.append(".");
-            _builder.append(expr.function_name);
-            _builder.append("(");
-            if (expr.arguments != null) {
-                bool first = true;
-                foreach (var arg in expr.arguments) {
-                    if (!first) _builder.append(", ");
-                    arg.accept(this);
-                    first = false;
-                }
-            }
-            _builder.append(")");
-        }
-        
-        public void visit_global_function_call(GlobalFunctionCallExpression expr) {
-            _builder.append(expr.function_name);
-            _builder.append("(");
-            if (expr.arguments != null) {
-                bool first = true;
-                foreach (var arg in expr.arguments) {
-                    if (!first) _builder.append(", ");
-                    arg.accept(this);
-                    first = false;
-                }
-            }
-            _builder.append(")");
-        }
-        
-        public void visit_lot_literal(LotLiteralExpression expr) {
-            _builder.append("[");
-            bool first = true;
-            foreach (var element in expr.elements) {
-                if (!first) _builder.append(", ");
-                element.accept(this);
-                first = false;
-            }
-            _builder.append("]");
-        }
-    }
 }

src/orm/projections/friendly-name-resolver.vala

@@ -183,9 +183,9 @@ namespace InvercargillSql.Orm.Projections {
             ensure_cache_built();
             
             // First check if it's a direct friendly name
-            Expression? expr = _friendly_name_cache.get(friendly_name);
-            if (expr != null) {
-                return expr;
+            // Use has() before get() to avoid exception on missing key
+            if (_friendly_name_cache.has(friendly_name)) {
+                return _friendly_name_cache.get(friendly_name);
             }
             
             // Check if it's a nested path (e.g., "profile.id")

src/orm/projections/projection-query.vala

@@ -309,28 +309,19 @@ namespace InvercargillSql.Orm.Projections {
         
         /**
          * Translates ORDER BY clauses for use with ProjectionSqlBuilder.
-         * 
-         * The ProjectionSqlBuilder expects OrderByClause with string expressions,
-         * so we need to convert our Expression-based OrderByClause to string-based.
-         * 
-         * @return A new vector of OrderByClause with string expressions
+         *
+         * Returns the original orderings - ProjectionSqlBuilder.translate_order_by()
+         * handles the expression translation including friendly name resolution.
+         *
+         * @return The orderings vector, or null if empty
          */
         private Vector<OrderByClause>? translate_orderings_for_builder() {
             if (_orderings.length == 0) {
                 return null;
             }
             
-            var result = new Vector<OrderByClause>();
-            
-            foreach (var clause in _orderings) {
-                string expr_str = expression_to_sql_string(clause.expression);
-                // Create a new OrderByClause with the translated string expression
-                // Note: We create a string-based OrderByClause using a LiteralExpression
-                // as a carrier for the string
-                result.add(new OrderByClause(new LiteralExpression(new Invercargill.NativeElement<string?>(expr_str)), clause.descending));
-            }
-            
-            return result;
+            // Just return the original orderings - ProjectionSqlBuilder will translate them
+            return _orderings;
         }
     }
 }

src/orm/projections/projection-sql-builder.vala

@@ -251,30 +251,18 @@ namespace InvercargillSql.Orm.Projections {
                 return build_subquery_wrapper(where_expression, order_by, limit, offset);
             }
             
-            // Translate the split parts
+            // Translate the split parts - now using Expression objects directly
             string? where_clause = null;
             string? having_clause = null;
             
-            if (split.non_aggregate_part != null) {
-                // Parse the non-aggregate part back to Expression and translate
-                try {
-                    var non_agg_expr = ExpressionParser.parse(split.non_aggregate_part);
-                    where_clause = translate_expression(non_agg_expr);
-                } catch (Error e) {
-                    // Fall back to string translation
-                    where_clause = _translator.translate_expression_string(split.non_aggregate_part);
-                }
+            if (split.non_aggregate_expression != null) {
+                // Translate the non-aggregate Expression directly
+                where_clause = translate_expression(split.non_aggregate_expression);
             }
             
-            if (split.aggregate_part != null) {
-                // Parse the aggregate part back to Expression and translate
-                try {
-                    var agg_expr = ExpressionParser.parse(split.aggregate_part);
-                    having_clause = translate_expression(agg_expr);
-                } catch (Error e) {
-                    // Fall back to string translation
-                    having_clause = _translator.translate_expression_string(split.aggregate_part);
-                }
+            if (split.aggregate_expression != null) {
+                // Translate the aggregate Expression directly
+                having_clause = translate_expression(split.aggregate_expression);
             }
             
             // If no split occurred (all aggregate or all non-aggregate)
@@ -329,12 +317,12 @@ namespace InvercargillSql.Orm.Projections {
         
         /**
          * Translates an Expression object to SQL.
-         * 
+         *
          * This method handles:
          * - Variable substitution (e.g., "u.id" -> "val_1_User.id")
          * - Friendly name resolution (e.g., "user_id" -> "val_1_User.id")
          * - SQL operator conversion (e.g., "==" -> "=")
-         * 
+         *
          * @param expression The Expression object to translate
          * @return The translated SQL expression
          */
@@ -342,15 +330,17 @@ namespace InvercargillSql.Orm.Projections {
             // Ensure aliases are assigned
             assign_aliases();
             
-            // Convert Expression to string first
-            string expression_string = expression_to_string(expression);
-            
-            // Check for simple friendly name (no dots)
+            // Check for simple friendly name (a single VariableExpression)
             Expression? expr_to_translate = expression;
-            if (!expression_string.contains(".") && _resolver.is_friendly_name(expression_string)) {
-                Expression? resolved = _resolver.resolve_to_expression(expression_string);
-                if (resolved != null) {
-                    expr_to_translate = resolved;
+            if (expression is VariableExpression) {
+                var var_expr = (VariableExpression) expression;
+                string var_name = var_expr.variable_name;
+                // Check if it's a friendly name (no dots and registered)
+                if (!var_name.contains(".") && _resolver.is_friendly_name(var_name)) {
+                    Expression? resolved = _resolver.resolve_to_expression(var_name);
+                    if (resolved != null) {
+                        expr_to_translate = resolved;
+                    }
                 }
             }
             
@@ -456,10 +446,12 @@ namespace InvercargillSql.Orm.Projections {
                     if (!first) {
                         final_sql.append(", ");
                     }
-                    // Convert the Expression to string for appending to SQL
                     // The expression was already translated in translate_order_by()
-                    string expr_str = expression_to_string(clause.expression);
-                    final_sql.append(expr_str);
+                    // and wrapped in a VariableExpression with the SQL as the variable name
+                    if (clause.expression is VariableExpression) {
+                        var var_expr = (VariableExpression) clause.expression;
+                        final_sql.append(var_expr.variable_name);
+                    }
                     if (clause.descending) {
                         final_sql.append(" DESC");
                     }
@@ -503,8 +495,9 @@ namespace InvercargillSql.Orm.Projections {
             }
             
             foreach (var clause in order_by) {
-                // Translate the expression using the translator
-                string translated_expr = _translator.translate_expression(clause.expression);
+                // Translate the expression - this handles friendly name resolution
+                // and variable translation
+                string translated_expr = translate_expression(clause.expression);
                 // Create a VariableExpression with the translated SQL as the variable name
                 // This allows the dialect to output the translated SQL when converting to string
                 var translated_expression = new VariableExpression(translated_expr);
@@ -514,19 +507,5 @@ namespace InvercargillSql.Orm.Projections {
             return result;
         }
         
-        /**
-         * Converts an Expression object to its string representation.
-         * 
-         * Uses ExpressionStringVisitor to traverse the expression tree and
-         * build a string representation.
-         * 
-         * @param expression The Expression to convert
-         * @return The string representation of the expression
-         */
-        private string expression_to_string(Expression expression) {
-            var visitor = new ExpressionStringVisitor();
-            expression.accept(visitor);
-            return visitor.get_string();
-        }
     }
 }

src/orm/projections/variable-translator.vala

@@ -155,9 +155,8 @@ namespace InvercargillSql.Orm.Projections {
         public string translate_variable(string variable_name) {
             ensure_aliases_assigned();
             
-            string? alias = _variable_to_alias[variable_name];
-            if (alias != null) {
-                return alias;
+            if (_variable_to_alias.has(variable_name)) {
+                return _variable_to_alias[variable_name];
             }
             return variable_name;  // Return unchanged if not found
         }
@@ -187,13 +186,11 @@ namespace InvercargillSql.Orm.Projections {
         public string translate_expression(Expression expression) {
             ensure_aliases_assigned();
             
-            // Convert Expression to string using ExpressionStringVisitor
-            var string_visitor = new InvercargillSql.Expressions.ExpressionStringVisitor();
-            expression.accept(string_visitor);
-            string expr_string = string_visitor.get_string();
-            
-            // Use simple string replacement for variable translation
-            return translate_expression_simple(expr_string);
+            // Use the VariableTranslationVisitor which properly handles
+            // all expression types including literals with SQL formatting
+            var visitor = new VariableTranslationVisitor(this);
+            expression.accept(visitor);
+            return visitor.get_translated_expression();
         }
         
         /**
@@ -404,14 +401,13 @@ namespace InvercargillSql.Orm.Projections {
                 }
             }
             
-            // print("  Result: '%s'\n", result.str);
             return result.str;
         }
     }
     
     /**
      * Expression visitor that translates variable references to SQL aliases.
-     * 
+     *
      * This visitor traverses an expression tree and replaces variable references
      * with their corresponding SQL aliases from the VariableTranslator.
      */
@@ -420,14 +416,35 @@ namespace InvercargillSql.Orm.Projections {
         private VariableTranslator _translator;
         private StringBuilder _builder;
         
+        /**
+         * Pending property name to append after the next variable visit.
+         *
+         * This is needed because PropertyExpression.accept() in the Invercargill library
+         * calls visit_property() FIRST, then visits the target. So we can't visit the
+         * target ourselves - we need to store the property name and append it when
+         * the variable is visited.
+         */
+        private string? _pending_property_name;
+        
+        /**
+         * Tracks expressions that have already been visited to prevent double-visiting.
+         *
+         * This is needed because some Expression.accept() implementations in the library
+         * visit children AFTER calling our visitor method. If we also visit children
+         * in our visitor method, they get visited twice.
+         */
+        private HashSet<Expression> _visited_expressions;
+        
         /**
          * Creates a new VariableTranslationVisitor.
-         * 
+         *
          * @param translator The translator to use for variable lookups
          */
         public VariableTranslationVisitor(VariableTranslator translator) {
             _translator = translator;
             _builder = new StringBuilder();
+            _pending_property_name = null;
+            _visited_expressions = new HashSet<Expression>();
         }
         
         /**
@@ -467,48 +484,90 @@ namespace InvercargillSql.Orm.Projections {
         }
         
         public void visit_property(Invercargill.Expressions.PropertyExpression expr) {
-            // First visit the target (e.g., the variable "u" in "u.id")
-            if (expr.target != null) {
-                expr.target.accept(this);
+            // IMPORTANT: Do NOT visit the target here!
+            // PropertyExpression.accept() in the Invercargill library calls
+            // visit_property() FIRST, then visits the target. If we also visit
+            // the target here, it will be visited twice, corrupting the output.
+            //
+            // Instead, store the property name and append it when the variable
+            // is visited (in visit_variable()).
+            
+            // Check if we've already processed this expression
+            if (_visited_expressions.contains(expr)) {
+                // Still set the pending property name! The library will visit the target
+                // after this method returns, and we need the property name available.
+                _pending_property_name = expr.property_name;
+                return;
             }
-            _builder.append(".");
-            _builder.append(expr.property_name);
+            _visited_expressions.add(expr);
+            
+            _pending_property_name = expr.property_name;
+            // The library will call target.accept(this) after this method returns
         }
         
         public void visit_literal(Invercargill.Expressions.LiteralExpression expr) {
             var value = expr.value;
             if (value.is_null()) {
                 _builder.append("NULL");
-            } else if (value.assignable_to_type(typeof(string))) {
+                return;
+            }
+            
+            // Get the actual type and handle it directly
+            var actual_type = value.type();
+            string type_name = value.type_name();
+            
+            // Handle string types
+            if (actual_type == typeof(string)) {
                 string? s = null;
                 if (value.try_get_as<string>(out s) && s != null) {
                     _builder.append("'");
                     _builder.append(escape_string(s));
                     _builder.append("'");
                 }
-            } else if (value.assignable_to_type(typeof(int64))) {
+                return;
+            }
+            
+            // Handle integer types - check by type name prefix since nullable types include "(nullable)" suffix
+            if (type_name.has_prefix("gint64") ||
+                actual_type == typeof(int64) || actual_type == typeof(int64?)) {
                 int64? i = null;
                 if (value.try_get_as<int64?>(out i) && i != null) {
                     _builder.append(i.to_string());
                 }
-            } else if (value.assignable_to_type(typeof(int))) {
+                return;
+            }
+            
+            if (type_name.has_prefix("gint") ||
+                actual_type == typeof(int) || actual_type == typeof(int?)) {
                 int? i = null;
                 if (value.try_get_as<int?>(out i) && i != null) {
                     _builder.append(i.to_string());
                 }
-            } else if (value.assignable_to_type(typeof(double))) {
+                return;
+            }
+            
+            // Handle double types
+            if (type_name.has_prefix("gdouble") ||
+                actual_type == typeof(double) || actual_type == typeof(double?)) {
                 double? d = null;
                 if (value.try_get_as<double?>(out d) && d != null) {
                     _builder.append(d.to_string());
                 }
-            } else if (value.assignable_to_type(typeof(bool))) {
+                return;
+            }
+            
+            // Handle boolean types
+            if (type_name.has_prefix("gboolean") ||
+                actual_type == typeof(bool) || actual_type == typeof(bool?)) {
                 bool? b = null;
                 if (value.try_get_as<bool?>(out b) && b != null) {
                     _builder.append(b ? "1" : "0");
                 }
-            } else {
-                _builder.append(value.to_string());
+                return;
             }
+            
+            // Fallback - use stringify() which handles common types
+            _builder.append(value.stringify());
         }
         
         private string escape_string(string s) {
@@ -545,9 +604,23 @@ namespace InvercargillSql.Orm.Projections {
         }
         
         public void visit_variable(Invercargill.Expressions.VariableExpression expr) {
+            // Check if we've already processed this expression
+            if (_visited_expressions.contains(expr)) {
+                return;
+            }
+            _visited_expressions.add(expr);
+            
             // Translate the variable to its SQL alias
             string translated = _translator.translate_variable(expr.variable_name);
             _builder.append(translated);
+            
+            // If there's a pending property name from visit_property(), append it now
+            // This produces "alias.property" format
+            if (_pending_property_name != null) {
+                _builder.append(".");
+                _builder.append(_pending_property_name);
+                _pending_property_name = null;
+            }
         }
         
         public void visit_function_call(Invercargill.Expressions.FunctionCallExpression expr) {
@@ -599,17 +672,93 @@ namespace InvercargillSql.Orm.Projections {
         }
         
         public void visit_global_function_call(Invercargill.Expressions.GlobalFunctionCallExpression expr) {
+            // Check if we've already processed this expression (prevents double-visiting)
+            if (_visited_expressions.contains(expr)) {
+                return;
+            }
+            _visited_expressions.add(expr);
+            
             _builder.append(expr.function_name);
             _builder.append("(");
+            
+            // Visit arguments ourselves and output them with commas
+            // After visiting, mark them as visited so when the library tries to visit
+            // them after this method returns, they get skipped
             if (expr.arguments != null) {
                 bool first = true;
                 foreach (var arg in expr.arguments) {
-                    if (!first) _builder.append(", ");
-                    arg.accept(this);
+                    if (!first) {
+                        _builder.append(", ");
+                    }
                     first = false;
+                    
+                    // Visit the argument FIRST to output its content
+                    arg.accept(this);
+                    
+                    // THEN mark the entire expression tree as visited
+                    // This prevents the library's subsequent visit from double-processing
+                    mark_expression_tree_visited(arg);
                 }
             }
+            
             _builder.append(")");
+            
+            // Note: The library will try to visit arguments after this method returns,
+            // but they'll be skipped because we marked them as visited
+        }
+        
+        /**
+         * Recursively marks an expression and all its sub-expressions as visited.
+         * This is needed to prevent the library's visitor pattern from re-visiting
+         * expressions that we've already processed.
+         */
+        private void mark_expression_tree_visited(Expression expr) {
+            _visited_expressions.add(expr);
+            
+            // Recursively mark child expressions
+            if (expr is Invercargill.Expressions.BinaryExpression) {
+                var binary = (Invercargill.Expressions.BinaryExpression)expr;
+                mark_expression_tree_visited(binary.left);
+                mark_expression_tree_visited(binary.right);
+            } else if (expr is Invercargill.Expressions.PropertyExpression) {
+                var prop = (Invercargill.Expressions.PropertyExpression)expr;
+                mark_expression_tree_visited(prop.target);
+            } else if (expr is Invercargill.Expressions.UnaryExpression) {
+                var unary = (Invercargill.Expressions.UnaryExpression)expr;
+                mark_expression_tree_visited(unary.operand);
+            } else if (expr is Invercargill.Expressions.TernaryExpression) {
+                var ternary = (Invercargill.Expressions.TernaryExpression)expr;
+                mark_expression_tree_visited(ternary.condition);
+                mark_expression_tree_visited(ternary.true_expression);
+                mark_expression_tree_visited(ternary.false_expression);
+            } else if (expr is Invercargill.Expressions.LambdaExpression) {
+                var lambda = (Invercargill.Expressions.LambdaExpression)expr;
+                mark_expression_tree_visited(lambda.body);
+            } else if (expr is Invercargill.Expressions.BracketedExpression) {
+                var bracketed = (Invercargill.Expressions.BracketedExpression)expr;
+                mark_expression_tree_visited(bracketed.inner);
+            } else if (expr is Invercargill.Expressions.FunctionCallExpression) {
+                var funcCall = (Invercargill.Expressions.FunctionCallExpression)expr;
+                mark_expression_tree_visited(funcCall.target);
+                if (funcCall.arguments != null) {
+                    foreach (var arg in funcCall.arguments) {
+                        mark_expression_tree_visited(arg);
+                    }
+                }
+            } else if (expr is Invercargill.Expressions.GlobalFunctionCallExpression) {
+                var globalFunc = (Invercargill.Expressions.GlobalFunctionCallExpression)expr;
+                if (globalFunc.arguments != null) {
+                    foreach (var arg in globalFunc.arguments) {
+                        mark_expression_tree_visited(arg);
+                    }
+                }
+            } else if (expr is Invercargill.Expressions.LotLiteralExpression) {
+                var lot = (Invercargill.Expressions.LotLiteralExpression)expr;
+                foreach (var element in lot.elements) {
+                    mark_expression_tree_visited(element);
+                }
+            }
+            // VariableExpression, LiteralExpression, ParameterExpression have no children
         }
         
         public void visit_lot_literal(Invercargill.Expressions.LotLiteralExpression expr) {
@@ -622,5 +771,72 @@ namespace InvercargillSql.Orm.Projections {
             }
             _builder.append(")");
         }
+        
+        public void visit_parameter(Invercargill.Expressions.ParameterExpression expr) {
+            // ParameterExpression holds the actual value passed via expr("...", value)
+            // We need to output the value, not the placeholder ($0, $1, etc.)
+            var value = expr.value;
+            if (value.is_null()) {
+                _builder.append("NULL");
+                return;
+            }
+            
+            // Get the actual type and handle it directly
+            var actual_type = value.type();
+            string type_name = value.type_name();
+            
+            // Handle string types
+            if (actual_type == typeof(string)) {
+                string? s = null;
+                if (value.try_get_as<string>(out s) && s != null) {
+                    _builder.append("'");
+                    _builder.append(escape_string(s));
+                    _builder.append("'");
+                }
+                return;
+            }
+            
+            // Handle integer types - check by type name prefix since nullable types include "(nullable)" suffix
+            if (type_name.has_prefix("gint64") ||
+                actual_type == typeof(int64) || actual_type == typeof(int64?)) {
+                int64? i = null;
+                if (value.try_get_as<int64?>(out i) && i != null) {
+                    _builder.append(i.to_string());
+                }
+                return;
+            }
+            
+            if (type_name.has_prefix("gint") ||
+                actual_type == typeof(int) || actual_type == typeof(int?)) {
+                int? i = null;
+                if (value.try_get_as<int?>(out i) && i != null) {
+                    _builder.append(i.to_string());
+                }
+                return;
+            }
+            
+            // Handle double types
+            if (type_name.has_prefix("gdouble") ||
+                actual_type == typeof(double) || actual_type == typeof(double?)) {
+                double? d = null;
+                if (value.try_get_as<double?>(out d) && d != null) {
+                    _builder.append(d.to_string());
+                }
+                return;
+            }
+            
+            // Handle boolean types
+            if (type_name.has_prefix("gboolean") ||
+                actual_type == typeof(bool) || actual_type == typeof(bool?)) {
+                bool? b = null;
+                if (value.try_get_as<bool?>(out b) && b != null) {
+                    _builder.append(b ? "1" : "0");
+                }
+                return;
+            }
+            
+            // Fallback - use stringify() which handles common types
+            _builder.append(value.stringify());
+        }
     }
 }

src/tests/projection-test.vala

@@ -483,12 +483,14 @@ void test_aggregate_analyzer_split_and() throws Error {
     var split = analyzer.split_expression(expr("u.id > 100 && COUNT(o.id) >= 5"));
     
     assert(split.needs_subquery == false);
-    assert(split.non_aggregate_part != null);
-    assert(split.aggregate_part != null);
+    assert(split.non_aggregate_expression != null);
+    assert(split.aggregate_expression != null);
     // The expression parser outputs property names as just the property (e.g., "id" not "u.id")
     // Just verify we have both parts split correctly
-    assert("id" in split.non_aggregate_part || "100" in split.non_aggregate_part);
-    assert("COUNT" in split.aggregate_part);
+    var non_agg_str = split.non_aggregate_expression.to_expression_string();
+    var agg_str = split.aggregate_expression.to_expression_string();
+    assert("id" in non_agg_str || "100" in non_agg_str);
+    assert("COUNT" in agg_str);
     
     print("PASSED\n");
 }
@@ -512,8 +514,8 @@ void test_aggregate_analyzer_split_all_aggregate() throws Error {
     var split = analyzer.split_expression(expr("COUNT(o.id) >= 5 && SUM(o.total) > 1000"));
     
     assert(split.needs_subquery == false);
-    assert(split.non_aggregate_part == null);
-    assert(split.aggregate_part != null);
+    assert(split.non_aggregate_expression == null);
+    assert(split.aggregate_expression != null);
     
     print("PASSED\n");
 }
@@ -525,8 +527,8 @@ void test_aggregate_analyzer_split_all_non_aggregate() throws Error {
     var split = analyzer.split_expression(expr("u.id > 100 && u.age >= 18"));
     
     assert(split.needs_subquery == false);
-    assert(split.non_aggregate_part != null);
-    assert(split.aggregate_part == null);
+    assert(split.non_aggregate_expression != null);
+    assert(split.aggregate_expression == null);
     
     print("PASSED\n");
 }
@@ -1152,9 +1154,12 @@ void test_projection_with_aggregates() throws Error {
         .build()
     );
     
-    var results = ctx.session.query<UserOrderStats>()
-        .order_by(expr("user_id"))
-        .materialise();
+    var query = ctx.session.query<UserOrderStats>()
+        .order_by(expr("user_id"));
+    string sql = query.to_sql();
+    print("\n  Generated SQL: %s\n", sql);
+    
+    var results = query.materialise();
     
     assert(results.length == 3);