Parameterize multi-level closure captures and support nav OrderBy via JOIN

Author: mikasoukhov

Data.ORM/Sql/Expression/Context.cs

@@ -29,7 +29,7 @@ class Context
 
 	public long? Skip;
 	public long? Take;
-	public readonly List<(MemberInfo member, bool asc)> OrderBy = [];
+	public readonly List<(string joinAlias, MemberInfo member, bool asc)> OrderBy = [];
 	public string SelectAlias;
 	public bool Count;
 	public bool Distinct;
@@ -356,14 +356,16 @@ public Query Build(Schema schema)
 
 				var isFirstColumn = true;
 
-				foreach (var (member, asc) in OrderBy)
+				foreach (var (joinAlias, member, asc) in OrderBy)
 				{
 					if (isFirstColumn)
 						isFirstColumn = false;
 					else
 						query.Comma();
 
-					if (schema.EntityType == member.DeclaringType)
+					if (joinAlias is not null)
+						query.Column(joinAlias, member.Name);
+					else if (schema.EntityType == member.DeclaringType)
 						query.Column(TableAlias, member.Name);
 					else
 						query.Column(member.Name);

Data.ORM/Sql/Expression/ExpressionQueryTranslator.cs

@@ -1083,6 +1083,17 @@ protected override Expression VisitMember(MemberExpression m)
 				return exp;
 			}
 
+			// Compiler-generated DisplayClass chains of arbitrary depth (e.g. async
+			// state machine + nested if-block locals) must be evaluated up-front
+			// and emitted as parameters; otherwise the chain falls through to the
+			// column-emission branches and the captured local name (e.g. `tenantId`)
+			// leaks as a raw SQL column on the default alias.
+			if (TryEvaluateClosureCapture(m, out var capturedValue))
+			{
+				EmitClosureCaptureValue(m.Member, capturedValue);
+				return m;
+			}
+
 			if (m.Expression is ConstantExpression ce)
 			{
 				var memberValue = m.Member.GetMemberValue(ce.Value);
@@ -1329,11 +1340,19 @@ private void ParseOrderByExpression(MethodCallExpression expression, bool asc)
 					&& body.Expression is MemberExpression navExpr
 					&& !navExpr.Member.DeclaringType.IsAutoGenerated())
 				{
-					Context.OrderBy.Insert(0, (navExpr.Member, asc));
+					Context.OrderBy.Insert(0, (null, navExpr.Member, asc));
+				}
+				else if (body.Expression is MemberExpression navOwner
+					&& TryEnsureImplicitJoin(navOwner) is { } navAlias)
+				{
+					// OrderBy(t => t.Person.Name): register implicit JOIN to Person
+					// and qualify the column as [Person].[Name] so it resolves at
+					// SQL level instead of emitting bare [Name].
+					Context.OrderBy.Insert(0, (navAlias, body.Member, asc));
 				}
 				else
 				{
-					Context.OrderBy.Insert(0, (body.Member, asc));
+					Context.OrderBy.Insert(0, (null, body.Member, asc));
 				}
 			}
 			else if (lambdaBody is MethodCallExpression call)
@@ -1354,4 +1373,91 @@ private void ParseOrderByExpression(MethodCallExpression expression, bool asc)
 			Curr = curr;
 		}
 	}
+
+	/// <summary>
+	/// Tries to evaluate a chain of MemberExpression accesses that terminates at
+	/// a <see cref="ConstantExpression"/> rooted on a compiler-generated closure
+	/// DisplayClass. Returns true and the captured value when every link in the
+	/// chain is a closure access (intermediate types are <c>[CompilerGenerated]</c>).
+	/// Used to short-circuit nested closure captures (e.g. inner DC -> outer DC
+	/// -> field) that would otherwise leak through column-emission branches and
+	/// produce SQL like <c>[e].[tenantId]</c>.
+	/// </summary>
+	private static bool TryEvaluateClosureCapture(MemberExpression m, out object value)
+	{
+		value = null;
+
+		var stack = new Stack<MemberInfo>();
+		Expression current = m;
+
+		while (current is MemberExpression cur)
+		{
+			// Reject the chain as soon as a non-closure type appears: anything that is
+			// not [CompilerGenerated] belongs to user/entity territory and must continue
+			// through the existing visitor branches (Members substitution, implicit JOIN,
+			// inner-schema flattening, etc.).
+			if (cur.Member.DeclaringType is null || !cur.Member.DeclaringType.IsAutoGenerated())
+				return false;
+
+			stack.Push(cur.Member);
+			current = cur.Expression;
+		}
+
+		if (current is not ConstantExpression ce)
+			return false;
+
+		var instance = ce.Value;
+
+		while (stack.Count > 0)
+		{
+			var member = stack.Pop();
+
+			if (instance is null)
+				return false;
+
+			instance = member.GetMemberValue(instance);
+		}
+
+		value = instance;
+		return true;
+	}
+
+	/// <summary>
+	/// Emits a value resolved from a closure-capture chain into the current query
+	/// builder, mirroring <see cref="Context.TryAddParam(MemberInfo, object)"/> so
+	/// scalars become parameters, arrays expand into comma-separated parameter
+	/// lists, IQueryable values are re-visited to inline their expression tree,
+	/// and nulls collapse to <c>NULL</c>.
+	/// </summary>
+	private void EmitClosureCaptureValue(MemberInfo member, object value)
+	{
+		if (value is IQueryable q && q.Expression is not ConstantExpression)
+		{
+			Visit(q.Expression);
+			return;
+		}
+
+		if (value is null)
+		{
+			Curr.Null();
+			return;
+		}
+
+		if (value is Array arr && value is not byte[])
+		{
+			var itemType = arr.GetType().GetItemType();
+
+			for (var i = 0; i < arr.Length; i++)
+			{
+				if (i > 0)
+					Curr.Comma();
+
+				Curr.Param(Context.TryAddParam(member.Name, itemType, arr.GetValue(i)));
+			}
+			return;
+		}
+
+		Curr.Param(Context.TryAddParam(member.Name, member.GetMemberType(), value));
+	}
+
 }

Tests/Data/ExpressionQueryTranslatorTests.cs

@@ -645,6 +645,51 @@ public void OrderBy_NavigationPropertyIdConvertToObject_ShouldUseFkColumn()
 		sql.Contains("[Person]").AssertTrue($"Expected FK column [Person] in ORDER BY, got: {sql}");
 	}
 
+	/// <summary>
+	/// Sorting by a non-Id member of a [RelationSingle] navigation
+	/// (e.g., t.Person.Name) used to leak the leaf column unaliased,
+	/// producing SQL like ORDER BY [Name] which fails on the host
+	/// table that doesn't have a [Name] column. The translator must
+	/// register an implicit JOIN to the navigation table and qualify
+	/// the column with the join alias.
+	/// </summary>
+	[TestMethod]
+	public void OrderBy_NavigationPropertyMember_ShouldRegisterJoinAndQualifyColumn()
+	{
+		EnsureFkEntitiesRegistered();
+		var tasks = CreateQueryable<TestTask>();
+
+		var query = tasks.OrderBy(t => t.Person.Name);
+
+		var sql = GenerateSql<TestTask>(query);
+
+		sql.ContainsIgnoreCase("ORDER BY").AssertTrue($"Expected ORDER BY clause, got: {sql}");
+		sql.Contains("[Person].[Name]").AssertTrue(
+			$"Expected qualified [Person].[Name] in ORDER BY (implicit JOIN required), got: {sql}");
+		sql.ContainsIgnoreCase("INNER JOIN").AssertTrue(
+			$"Expected INNER JOIN to Person table for navigation OrderBy, got: {sql}");
+	}
+
+	/// <summary>
+	/// OrderByDescending variant of <see cref="OrderBy_NavigationPropertyMember_ShouldRegisterJoinAndQualifyColumn"/>.
+	/// </summary>
+	[TestMethod]
+	public void OrderByDescending_NavigationPropertyMember_ShouldRegisterJoinAndQualifyColumn()
+	{
+		EnsureFkEntitiesRegistered();
+		var tasks = CreateQueryable<TestTask>();
+
+		var query = tasks.OrderByDescending(t => t.Person.Name);
+
+		var sql = GenerateSql<TestTask>(query);
+
+		sql.Contains("[Person].[Name]").AssertTrue(
+			$"Expected qualified [Person].[Name] in ORDER BY DESC, got: {sql}");
+		sql.ContainsIgnoreCase("DESC").AssertTrue($"Expected DESC, got: {sql}");
+		sql.ContainsIgnoreCase("INNER JOIN").AssertTrue(
+			$"Expected INNER JOIN to Person table, got: {sql}");
+	}
+
 	#endregion
 
 	#region String Parameterization Tests
@@ -692,6 +737,73 @@ public void StringConstant_WithMultipleSingleQuotes_ShouldBeParameterized()
 
 	#endregion
 
+	#region Closure-capture Chain Parameterization Tests
+
+	// Reproduces the EXACT closure shape generated by an async method with two levels
+	// of nested if-blocks that capture an outer-method local. Mirrors
+	// <c>BrokerPortfolioRepository.SearchAsync</c>: outer state-machine field
+	// (tenantId), first if-block DC (referencing the state-machine), and a nested
+	// else-block DC (referencing the first DC). The lambda inside the inner block
+	// then reads tenantId through a 3-link chain: innerDC → outerDC → methodDC →
+	// tenantId. The compiler-generated tree shape is:
+	//   MemberAccess(tenantId)
+	//     MemberAccess(CS$&lt;&gt;8__locals1)
+	//       MemberAccess(CS$&lt;&gt;8__locals2)
+	//         ConstantExpression(innerDC instance)
+	// Pre-fix VisitMember handled only 1- and 2-link closure chains; this 3-link
+	// chain fell through to column-emission branches and leaked raw field names
+	// (e.g. <c>[e].[tenantId]</c>) as if they were SQL columns.
+	private static IQueryable<TestTask> BuildDeepClosureQueryProductionShape(IQueryable<TestTask> tasks, long ownerId, string user)
+	{
+		var dummyToken = string.IsNullOrEmpty(user) ? null : user;
+		if (dummyToken is not null)
+		{
+			if (long.TryParse(dummyToken, out var uid))
+			{
+				return tasks.Where(t => t.Id == uid);
+			}
+			else
+			{
+				var like = "%" + dummyToken + "%";
+				// This Where lambda captures BOTH `ownerId` (method scope) and `like` (else-block scope).
+				// Compiler emits: innerDC has `like`, parent is firstIfDC which parents the methodDC.
+				// → 3-level MemberExpression chain to reach `ownerId` from the constant.
+				return tasks.Where(t => t.Person.Id == ownerId && (t.Title == like || t.Title == like));
+			}
+		}
+		return tasks;
+	}
+
+	/// <summary>
+	/// Captures across three nested DisplayClass levels used to fall through every
+	/// short-circuit branch in VisitMember and emit raw field names like
+	/// <c>[ownerId]</c> as if they were SQL columns on the default alias.
+	/// The translator must walk the full <c>[CompilerGenerated]</c> chain,
+	/// evaluate via reflection, and parameterize the captured values.
+	/// </summary>
+	[TestMethod]
+	public void Where_NestedClosureCaptureChain_ShouldParameterizeAndNotLeakFieldNames()
+	{
+		EnsureFkEntitiesRegistered();
+		var tasks = CreateQueryable<TestTask>();
+
+		var query = BuildDeepClosureQueryProductionShape(tasks, 42L, "alice");
+
+		var (sql, parameters) = TranslateSql<TestTask>(query);
+
+		sql.Contains("[ownerId]").AssertFalse(
+			$"Captured local 'ownerId' must not appear as a SQL column, got: {sql}");
+		sql.Contains("[like]").AssertFalse(
+			$"Captured local 'like' must not appear as a SQL column, got: {sql}");
+
+		parameters.Values.Any(p => Equals(p.Item2, 42L)).AssertTrue(
+			$"Expected captured ownerId=42 to be parameterized, got params: {string.Join(", ", parameters.Select(p => $"{p.Key}={p.Value.Item2}"))}");
+		parameters.Values.Any(p => Equals(p.Item2, "%alice%")).AssertTrue(
+			$"Expected captured like='%alice%' to be parameterized, got params: {string.Join(", ", parameters.Select(p => $"{p.Key}={p.Value.Item2}"))}");
+	}
+
+	#endregion
+
 	#region Anonymous-type Select Projection Tests
 
 	/// <summary>