From 0dd36a37f65f48dd8e9b79ccac5a87327a204fed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Mar=C5=A1=C3=A1lek?= Date: Thu, 17 Jul 2025 16:52:37 +0200 Subject: [PATCH 1/3] fix #1664 --- .../src/Linq/SubtreeEvaluator.cs | 5 +- .../Linq/ConstantEvaluatorTests.cs | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs diff --git a/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs b/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs index 4d67fbd67d..3a424ae8b1 100644 --- a/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs +++ b/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs @@ -5,6 +5,7 @@ namespace Microsoft.Azure.Cosmos.Linq { using System; using System.Collections.Generic; + using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -40,7 +41,9 @@ public override Expression Visit(Expression expression) protected override Expression VisitMemberInit(MemberInitExpression node) { - return node; + return Expression.MemberInit( + node.NewExpression, + node.Bindings.Select(binding => this.VisitMemberBinding(binding))); } private Expression EvaluateMemberAccess(Expression expression) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs new file mode 100644 index 0000000000..59b58fb78f --- /dev/null +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs @@ -0,0 +1,46 @@ +//------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +//------------------------------------------------------------ + +namespace Microsoft.Azure.Cosmos.Linq +{ + using System; + using System.Collections.Generic; + using System.Globalization; + using System.IO; + using System.Linq; + using System.Linq.Expressions; + using System.Reflection; + using System.Text.Json.Serialization; + using global::Azure.Core.Serialization; + using Microsoft.Azure.Cosmos.Serializer; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using Newtonsoft.Json; + using Newtonsoft.Json.Converters; + + [TestClass] + public class ConstantEvaluatorTests + { + [TestMethod] + public void ClosuresAreEvaluated() + { + int a = 1; + // In class member init + Expression> closureInClassExpression = x => new TestClass { Property = x + a }; + Expression> closureInClassExpressionExpected = x => new TestClass { Property = x + 1 }; + Expression closureInClassExpressionActual = ConstantEvaluator.PartialEval(closureInClassExpression.Body); + Assert.AreEqual(closureInClassExpressionExpected.Body.ToString(), closureInClassExpressionActual.ToString()); + + // In anonymous object + Expression> closureInAnonymousObjectExpression = x => new { Property = x + a }; + Expression> closureInAnonymousObjectExpressionExpected = x => new { Property = x + 1 }; + Expression closureInAnonymousObjectExpressionActual = ConstantEvaluator.PartialEval(closureInAnonymousObjectExpression.Body); + Assert.AreEqual(closureInAnonymousObjectExpressionExpected.Body.ToString(), closureInAnonymousObjectExpressionActual.ToString()); + } + + private class TestClass + { + public int Property { get; set; } + } + } +} From 8fc8b9ce25ac0b41f892e5173272b9ff4e729975 Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Mon, 11 May 2026 12:48:09 -0700 Subject: [PATCH 2/3] Address PR review feedback for MemberInit constant folding fix SubtreeEvaluator.cs - Add load-bearing intent comment in VisitMemberInit explaining why node.NewExpression is intentionally NOT visited (parameterless ew T() would be nominated and folded to a ConstantExpression, breaking the Expression.MemberInit contract). - Short-circuit no-change case using the protected static ExpressionVisitor.Visit(...) helper to avoid allocating a new MemberInitExpression on every LINQ query when nothing folded. ConstantEvaluatorTests.cs - Trim unused using directives (8 of 12 were unused template scaffolding). - Split the single ClosuresAreEvaluated test into per-scenario test methods so a regression names the actual broken shape. - Replace fragile Expression.ToString() string-equality assertions with structural shape assertions (MemberInit -> MemberAssignment -> Binary ...) via small typed helpers. - Add coverage for shapes the fix newly affects but the original test missed: dictionary indexer keyed by a closure variable (the exact shape from issue #1664), nested MemberInit, MemberMemberBinding ( ew Outer { Inner = { ... } }), and MemberListBinding ( ew Outer { Items = { ... } }). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Linq/SubtreeEvaluator.cs | 20 +- .../Linq/ConstantEvaluatorTests.cs | 202 ++++++++++++++++-- 2 files changed, 197 insertions(+), 25 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs b/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs index 3a424ae8b1..7b6c8e57c9 100644 --- a/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs +++ b/Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs @@ -5,7 +5,7 @@ namespace Microsoft.Azure.Cosmos.Linq { using System; using System.Collections.Generic; - using System.Linq; + using System.Collections.ObjectModel; using System.Linq.Expressions; using System.Reflection; @@ -41,9 +41,21 @@ public override Expression Visit(Expression expression) protected override Expression VisitMemberInit(MemberInitExpression node) { - return Expression.MemberInit( - node.NewExpression, - node.Bindings.Select(binding => this.VisitMemberBinding(binding))); + // Rebuild the MemberInit manually and intentionally do NOT visit node.NewExpression. + // The Nominator nominates a parameterless `new T()` as a candidate (CanBeEvaluated + // returns true for any non-Parameter / non-Lambda expression). Routing it through + // our overridden Visit would fold it into a ConstantExpression of the constructed + // CLR instance. Expression.MemberInit requires a NewExpression as its first argument, + // not a ConstantExpression, so that path would throw InvalidOperationException at + // runtime. We only need to recurse into the bindings to fold closure-captured + // variables (and other independent sub-trees) in initializers — see issue #1664. + ReadOnlyCollection newBindings = Visit(node.Bindings, this.VisitMemberBinding); + if (newBindings == node.Bindings) + { + return node; + } + + return Expression.MemberInit(node.NewExpression, newBindings); } private Expression EvaluateMemberAccess(Expression expression) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs index 59b58fb78f..255ea1ddb5 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Linq/ConstantEvaluatorTests.cs @@ -1,4 +1,4 @@ -//------------------------------------------------------------ +//------------------------------------------------------------ // Copyright (c) Microsoft Corporation. All rights reserved. //------------------------------------------------------------ @@ -6,41 +6,201 @@ namespace Microsoft.Azure.Cosmos.Linq { using System; using System.Collections.Generic; - using System.Globalization; - using System.IO; using System.Linq; using System.Linq.Expressions; - using System.Reflection; - using System.Text.Json.Serialization; - using global::Azure.Core.Serialization; - using Microsoft.Azure.Cosmos.Serializer; using Microsoft.VisualStudio.TestTools.UnitTesting; - using Newtonsoft.Json; - using Newtonsoft.Json.Converters; [TestClass] public class ConstantEvaluatorTests { [TestMethod] - public void ClosuresAreEvaluated() + public void ClosuresInsideMemberInitExpressionAreFolded() + { + int captured = 1; + Expression> expression = x => new TestClass { Property = x + captured }; + + Expression folded = ConstantEvaluator.PartialEval(expression.Body); + + MemberInitExpression memberInit = AssertMemberInit(folded, typeof(TestClass)); + MemberAssignment assignment = AssertSingleMemberAssignment(memberInit, nameof(TestClass.Property)); + BinaryExpression binary = AssertBinary(assignment.Expression, ExpressionType.Add); + AssertParameter(binary.Left, "x"); + AssertConstant(binary.Right, 1); + } + + [TestMethod] + public void ClosuresInsideAnonymousObjectAreFolded() { - int a = 1; - // In class member init - Expression> closureInClassExpression = x => new TestClass { Property = x + a }; - Expression> closureInClassExpressionExpected = x => new TestClass { Property = x + 1 }; - Expression closureInClassExpressionActual = ConstantEvaluator.PartialEval(closureInClassExpression.Body); - Assert.AreEqual(closureInClassExpressionExpected.Body.ToString(), closureInClassExpressionActual.ToString()); + int captured = 1; + Expression> expression = x => new { Property = x + captured }; + + Expression folded = ConstantEvaluator.PartialEval(expression.Body); - // In anonymous object - Expression> closureInAnonymousObjectExpression = x => new { Property = x + a }; - Expression> closureInAnonymousObjectExpressionExpected = x => new { Property = x + 1 }; - Expression closureInAnonymousObjectExpressionActual = ConstantEvaluator.PartialEval(closureInAnonymousObjectExpression.Body); - Assert.AreEqual(closureInAnonymousObjectExpressionExpected.Body.ToString(), closureInAnonymousObjectExpressionActual.ToString()); + NewExpression newExpression = AssertNew(folded); + Assert.AreEqual(1, newExpression.Arguments.Count); + BinaryExpression binary = AssertBinary(newExpression.Arguments[0], ExpressionType.Add); + AssertParameter(binary.Left, "x"); + AssertConstant(binary.Right, 1); + } + + // Regression test for https://github.com/Azure/azure-cosmos-dotnet-v3/issues/1664. + // Mirrors the original bug report shape: a dictionary indexer keyed by a closure-captured + // variable, used inside a class member initializer. Before the fix this folded to + // `{"k": "Test"}["k"]` instead of `"Test"`, producing invalid SQL at the Cosmos backend. + // The parameter reference `q.StringProperty` anchors the MemberInit so the Nominator + // cannot collapse the entire expression to a single constant — only the closure-only + // dictionary indexer sub-tree should fold. + [TestMethod] + public void ClosuresUsedAsDictionaryIndexerInsideMemberInitAreFolded() + { + Dictionary map = new Dictionary { ["k"] = "Test" }; + string capturedKey = "k"; + Expression> expression = + q => new TestClass { StringProperty = q.StringProperty + map[capturedKey] }; + + Expression folded = ConstantEvaluator.PartialEval(expression.Body); + + MemberInitExpression memberInit = AssertMemberInit(folded, typeof(TestClass)); + MemberAssignment assignment = AssertSingleMemberAssignment(memberInit, nameof(TestClass.StringProperty)); + BinaryExpression binary = AssertBinary(assignment.Expression, ExpressionType.Add); + // Left side stays as `q.StringProperty` (parameter-bound member access). + MemberExpression leftMember = binary.Left as MemberExpression; + Assert.IsNotNull(leftMember, $"Expected MemberExpression on left of Add but got {binary.Left?.NodeType.ToString() ?? ""}."); + AssertParameter(leftMember.Expression, "q"); + Assert.AreEqual(nameof(TestClass.StringProperty), leftMember.Member.Name); + // Right side must be the folded literal — not a `Dictionary[indexer]` expression. + AssertConstant(binary.Right, "Test"); + } + + [TestMethod] + public void ClosuresInsideNestedMemberInitAreFolded() + { + int captured = 7; + Expression> expression = x => new OuterTestClass + { + Inner = new TestClass { Property = x + captured } + }; + + Expression folded = ConstantEvaluator.PartialEval(expression.Body); + + MemberInitExpression outerInit = AssertMemberInit(folded, typeof(OuterTestClass)); + MemberAssignment innerAssignment = AssertSingleMemberAssignment(outerInit, nameof(OuterTestClass.Inner)); + MemberInitExpression innerInit = AssertMemberInit(innerAssignment.Expression, typeof(TestClass)); + MemberAssignment propertyAssignment = AssertSingleMemberAssignment(innerInit, nameof(TestClass.Property)); + BinaryExpression binary = AssertBinary(propertyAssignment.Expression, ExpressionType.Add); + AssertParameter(binary.Left, "x"); + AssertConstant(binary.Right, 7); + } + + [TestMethod] + public void ClosuresInsideMemberMemberBindingAreFolded() + { + int captured = 9; + Expression> expression = x => new OuterTestClass + { + Inner = { Property = x + captured } + }; + + Expression folded = ConstantEvaluator.PartialEval(expression.Body); + + MemberInitExpression outerInit = AssertMemberInit(folded, typeof(OuterTestClass)); + Assert.AreEqual(1, outerInit.Bindings.Count); + MemberMemberBinding nested = outerInit.Bindings[0] as MemberMemberBinding; + Assert.IsNotNull(nested, "Expected a MemberMemberBinding for nested initializer syntax."); + Assert.AreEqual(nameof(OuterTestClass.Inner), nested.Member.Name); + Assert.AreEqual(1, nested.Bindings.Count); + MemberAssignment propertyAssignment = nested.Bindings[0] as MemberAssignment; + Assert.IsNotNull(propertyAssignment, "Expected MemberAssignment inside MemberMemberBinding."); + Assert.AreEqual(nameof(TestClass.Property), propertyAssignment.Member.Name); + BinaryExpression binary = AssertBinary(propertyAssignment.Expression, ExpressionType.Add); + AssertParameter(binary.Left, "x"); + AssertConstant(binary.Right, 9); + } + + [TestMethod] + public void ClosuresInsideMemberListBindingAreFolded() + { + int captured = 11; + Expression> expression = x => new OuterWithListTestClass + { + Items = { x + captured } + }; + + Expression folded = ConstantEvaluator.PartialEval(expression.Body); + + MemberInitExpression outerInit = AssertMemberInit(folded, typeof(OuterWithListTestClass)); + Assert.AreEqual(1, outerInit.Bindings.Count); + MemberListBinding listBinding = outerInit.Bindings[0] as MemberListBinding; + Assert.IsNotNull(listBinding, "Expected a MemberListBinding for collection initializer syntax."); + Assert.AreEqual(nameof(OuterWithListTestClass.Items), listBinding.Member.Name); + Assert.AreEqual(1, listBinding.Initializers.Count); + Assert.AreEqual(1, listBinding.Initializers[0].Arguments.Count); + BinaryExpression binary = AssertBinary(listBinding.Initializers[0].Arguments[0], ExpressionType.Add); + AssertParameter(binary.Left, "x"); + AssertConstant(binary.Right, 11); + } + + private static MemberInitExpression AssertMemberInit(Expression expression, Type expectedType) + { + MemberInitExpression memberInit = expression as MemberInitExpression; + Assert.IsNotNull(memberInit, $"Expected MemberInitExpression but got {expression?.NodeType.ToString() ?? ""}."); + Assert.AreEqual(expectedType, memberInit.Type); + return memberInit; + } + + private static NewExpression AssertNew(Expression expression) + { + NewExpression newExpression = expression as NewExpression; + Assert.IsNotNull(newExpression, $"Expected NewExpression but got {expression?.NodeType.ToString() ?? ""}."); + return newExpression; + } + + private static MemberAssignment AssertSingleMemberAssignment(MemberInitExpression memberInit, string memberName) + { + Assert.AreEqual(1, memberInit.Bindings.Count, $"Expected a single binding for member '{memberName}'."); + MemberAssignment assignment = memberInit.Bindings[0] as MemberAssignment; + Assert.IsNotNull(assignment, $"Expected MemberAssignment for member '{memberName}' but got {memberInit.Bindings[0].BindingType}."); + Assert.AreEqual(memberName, assignment.Member.Name); + return assignment; + } + + private static BinaryExpression AssertBinary(Expression expression, ExpressionType nodeType) + { + BinaryExpression binary = expression as BinaryExpression; + Assert.IsNotNull(binary, $"Expected BinaryExpression but got {expression?.NodeType.ToString() ?? ""}."); + Assert.AreEqual(nodeType, binary.NodeType); + return binary; + } + + private static void AssertParameter(Expression expression, string parameterName) + { + ParameterExpression parameter = expression as ParameterExpression; + Assert.IsNotNull(parameter, $"Expected ParameterExpression '{parameterName}' but got {expression?.NodeType.ToString() ?? ""}."); + Assert.AreEqual(parameterName, parameter.Name); + } + + private static void AssertConstant(Expression expression, T expectedValue) + { + ConstantExpression constant = expression as ConstantExpression; + Assert.IsNotNull(constant, $"Expected ConstantExpression with value '{expectedValue}' but got {expression?.NodeType.ToString() ?? ""}."); + Assert.AreEqual(expectedValue, constant.Value); } private class TestClass { public int Property { get; set; } + + public string StringProperty { get; set; } + } + + private class OuterTestClass + { + public TestClass Inner { get; set; } = new TestClass(); + } + + private class OuterWithListTestClass + { + public List Items { get; } = new List(); } } } From 960135dd8695cb87bcbfa62a42a96740fd20999c Mon Sep 17 00:00:00 2001 From: Nalu Tripician Date: Wed, 13 May 2026 14:01:29 -0700 Subject: [PATCH 3/3] Changelog: Adds Unreleased entry for LINQ MemberInit constant-folding fix Address @kushagraThapar review feedback requesting a changelog entry for PR #5298. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- changelog.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changelog.md b/changelog.md index a0ad03bf50..2e8ee42268 100644 --- a/changelog.md +++ b/changelog.md @@ -15,6 +15,12 @@ Preview features are treated as a separate branch and will not be included in th The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +### Unreleased + +#### Fixed + +- [5298](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/5298) LINQ: Fixes constant folding for closure-captured variables inside MemberInitExpression (resolves #1664). Previously, the recursion that partially evaluates expressions terminated whenever it encountered a `MemberInitExpression` node, so captured variables inside object initializers were not folded, producing invalid translated SQL. + ### [3.60.0-preview.0](https://www.nuget.org/packages/Microsoft.Azure.Cosmos/3.60.0-preview.0) - 2026-4-24 #### Added