Round of feedback.

cincuranet · cincuranet · commit 3a5f304417ef · 2025-03-13T10:43:39.000+01:00
diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs
@@ -628,7 +628,7 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres
     protected override Expression VisitSqlConstant(SqlConstantExpression sqlConstantExpression)
     {
         _relationalCommandBuilder
-            .Append(sqlConstantExpression.TypeMapping!.GenerateSqlLiteral(sqlConstantExpression.Value), sqlConstantExpression.Sensitive);
+            .Append(sqlConstantExpression.TypeMapping!.GenerateSqlLiteral(sqlConstantExpression.Value), sqlConstantExpression.IsSensitive);
 
         return sqlConstantExpression;
     }
diff --git a/src/EFCore.Relational/Query/SqlExpressions/SqlConstantExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SqlConstantExpression.cs
@@ -50,7 +50,7 @@ public SqlConstantExpression(object? value, Type type, bool sensitive, Relationa
         : base(type.UnwrapNullableType(), typeMapping)
     {
         Value = value;
-        Sensitive = sensitive;
+        IsSensitive = sensitive;
     }
 
     /// <summary>
@@ -71,7 +71,7 @@ public SqlConstantExpression(object value, bool sensitive, RelationalTypeMapping
     /// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
     [Obsolete("Call the constructor accepting a value (and possibly a Type) instead")]
     public SqlConstantExpression(ConstantExpression constantExpression, RelationalTypeMapping? typeMapping)
-        : this(constantExpression.Value, constantExpression.Type, false, typeMapping)
+        : this(constantExpression.Value, constantExpression.Type, sensitive: false, typeMapping)
     {
     }
 
@@ -83,15 +83,15 @@ public SqlConstantExpression(ConstantExpression constantExpression, RelationalTy
     /// <summary>
     ///    Whether the expression contains sensitive values.
     /// </summary>
-    public virtual bool Sensitive { get; }
+    public virtual bool IsSensitive { get; }
 
     /// <summary>
     ///     Applies supplied type mapping to this expression.
     /// </summary>
     /// <param name="typeMapping">A relational type mapping to apply.</param>
     /// <returns>A new expression which has supplied type mapping.</returns>
     public virtual SqlExpression ApplyTypeMapping(RelationalTypeMapping? typeMapping)
-        => new SqlConstantExpression(Value, Type, Sensitive, typeMapping);
+        => new SqlConstantExpression(Value, Type, IsSensitive, typeMapping);
 
     /// <inheritdoc />
     protected override Expression VisitChildren(ExpressionVisitor visitor)
@@ -107,7 +107,7 @@ public override Expression Quote()
                     Constant(Value, Type), typeof(object))
                 : Constant(Value, Type),
             Constant(Type),
-            Constant(Sensitive),
+            Constant(IsSensitive),
             RelationalExpressionQuotingUtilities.QuoteTypeMapping(TypeMapping));
 
     /// <inheritdoc />
diff --git a/src/EFCore.Relational/Storage/RelationalCommandBuilder.cs b/src/EFCore.Relational/Storage/RelationalCommandBuilder.cs
@@ -127,9 +127,7 @@ private void InitializeLogCommandTextBuilderIfNeeded(bool redact)
             && _logCommandTextBuilder is null
             && !Dependencies.LoggingOptions.IsSensitiveDataLoggingEnabled)
         {
-            _logCommandTextBuilder = new();
-            _logCommandTextBuilder.Append(_commandTextBuilder.ToString());
-            _logCommandTextBuilder.IndentCount = _commandTextBuilder.IndentCount;
+            _logCommandTextBuilder = _commandTextBuilder.Clone();
         }
     }
 }
diff --git a/src/EFCore/Infrastructure/IndentedStringBuilder.cs b/src/EFCore/Infrastructure/IndentedStringBuilder.cs
@@ -27,14 +27,11 @@ public class IndentedStringBuilder
     private readonly StringBuilder _stringBuilder = new();
 
     /// <summary>
-    ///     Gets or sets the current indent level.
+    ///     Gets the current indent level.
     /// </summary>
     /// <value>The current indent level.</value>
     public virtual int IndentCount
-    {
-        get => _indent;
-        set => _indent = value;
-    }
+        => _indent;
 
     /// <summary>
     ///     The current length of the built string.
@@ -293,6 +290,18 @@ public virtual IDisposable Indent()
     public virtual IDisposable SuspendIndent()
         => new IndentSuspender(this);
 
+    /// <summary>
+    ///    Clones this <see cref="IndentedStringBuilder" />, copying the built string and current indent level.
+    /// </summary>
+    /// <returns>New instance of <see cref="IndentedStringBuilder" />.</returns>
+    public virtual IndentedStringBuilder Clone()
+    {
+        var result = new IndentedStringBuilder();
+        result._stringBuilder.Append(_stringBuilder);
+        result._indent = _indent;
+        return result;
+    }
+
     /// <summary>
     ///     Returns the built string.
     /// </summary>
diff --git a/test/EFCore.Relational.Specification.Tests/Query/AdHocMiscellaneousQueryRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/AdHocMiscellaneousQueryRelationalTestBase.cs
@@ -241,19 +241,25 @@ public static DateTime Modify(DateTime date)
 
         #region Inlined redacting
 
+        protected abstract DbContextOptionsBuilder SetTranslateParameterizedCollectionsToConstants(DbContextOptionsBuilder optionsBuilder);
+
         [ConditionalTheory]
         [MemberData(nameof(InlinedRedactingData))]
         public virtual async Task Check_inlined_constants_redacting(bool async, bool enableSensitiveDataLogging)
         {
             var contextFactory = await InitializeAsync<InlinedRedactingContext>(
-                onConfiguring: o => o.EnableSensitiveDataLogging(enableSensitiveDataLogging));
+                onConfiguring: o =>
+                {
+                    SetTranslateParameterizedCollectionsToConstants(o);
+                    o.EnableSensitiveDataLogging(enableSensitiveDataLogging);
+                });
             using var context = contextFactory.CreateContext();
 
             var id = 1;
             var ids = new[] { id, 2, 3 };
-            var query1 = context.TestEntities.Where(x => EF.Constant(ids).Contains(x.Id));
-            var query2 = context.TestEntities.Where(x => EF.Constant(id) == x.Id);
-            var query3 = context.TestEntities.Where(x => EF.Constant(ids).Where(y => y == x.Id).Any());
+            var query1 = context.TestEntities.Where(x => ids.Contains(x.Id));
+            var query2 = context.TestEntities.Where(x => ids.Where(y => y == x.Id).Any());
+            var query3 = context.TestEntities.Where(x => EF.Constant(id) == x.Id);
 
             if (async)
             {
@@ -280,7 +286,7 @@ public class TestEntity
             }
         }
 
-        public readonly static IEnumerable<object[]> InlinedRedactingData = [[true, true], [true, false], [false, true], [false, false]];
+        public static readonly IEnumerable<object[]> InlinedRedactingData = [[true, true], [true, false], [false, true], [false, false]];
 
         #endregion
     }
diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocMiscellaneousQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocMiscellaneousQuerySqlServerTest.cs
@@ -19,6 +19,13 @@ public class AdHocMiscellaneousQuerySqlServerTest : AdHocMiscellaneousQueryRelat
     protected override ITestStoreFactory TestStoreFactory
         => SqlServerTestStoreFactory.Instance;
 
+    protected override DbContextOptionsBuilder SetTranslateParameterizedCollectionsToConstants(DbContextOptionsBuilder optionsBuilder)
+    {
+        new SqlServerDbContextOptionsBuilder(optionsBuilder).TranslateParameterizedCollectionsToConstants();
+
+        return optionsBuilder;
+    }
+
     protected override Task Seed2951(Context2951 context)
         => context.Database.ExecuteSqlRawAsync(
             """
@@ -2422,16 +2429,16 @@ WHERE [t].[Id] IN (?, ?, ?)
                 """
 SELECT [t].[Id], [t].[Name]
 FROM [TestEntities] AS [t]
-WHERE ? = [t].[Id]
+WHERE EXISTS (
+    SELECT 1
+    FROM (VALUES (?), (?), (?)) AS [i]([Value])
+    WHERE [i].[Value] = [t].[Id])
 """,
                 //
                 """
 SELECT [t].[Id], [t].[Name]
 FROM [TestEntities] AS [t]
-WHERE EXISTS (
-    SELECT 1
-    FROM (VALUES (?), (?), (?)) AS [i]([Value])
-    WHERE [i].[Value] = [t].[Id])
+WHERE ? = [t].[Id]
 """);
         }
         else
@@ -2446,16 +2453,16 @@ WHERE [t].[Id] IN (1, 2, 3)
                 """
 SELECT [t].[Id], [t].[Name]
 FROM [TestEntities] AS [t]
-WHERE 1 = [t].[Id]
-""",
-                //
-                """
-SELECT [t].[Id], [t].[Name]
-FROM [TestEntities] AS [t]
 WHERE EXISTS (
     SELECT 1
     FROM (VALUES (1), (2), (3)) AS [i]([Value])
     WHERE [i].[Value] = [t].[Id])
+""",
+            //
+            """
+SELECT [t].[Id], [t].[Name]
+FROM [TestEntities] AS [t]
+WHERE 1 = [t].[Id]
 """);
         }
     }
diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/AdHocMiscellaneousQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/AdHocMiscellaneousQuerySqliteTest.cs
@@ -10,6 +10,13 @@ public class AdHocMiscellaneousQuerySqliteTest : AdHocMiscellaneousQueryRelation
     protected override ITestStoreFactory TestStoreFactory
         => SqliteTestStoreFactory.Instance;
 
+    protected override DbContextOptionsBuilder SetTranslateParameterizedCollectionsToConstants(DbContextOptionsBuilder optionsBuilder)
+    {
+        new SqliteDbContextOptionsBuilder(optionsBuilder).TranslateParameterizedCollectionsToConstants();
+
+        return optionsBuilder;
+    }
+
     protected override Task Seed2951(Context2951 context)
         => context.Database.ExecuteSqlRawAsync(
             """
@@ -99,16 +106,16 @@ public override async Task Check_inlined_constants_redacting(bool async, bool en
                 """
 SELECT "t"."Id", "t"."Name"
 FROM "TestEntities" AS "t"
-WHERE ? = "t"."Id"
+WHERE EXISTS (
+    SELECT 1
+    FROM (SELECT ? AS "Value" UNION ALL VALUES (?), (?)) AS "i"
+    WHERE "i"."Value" = "t"."Id")
 """,
                 //
                 """
 SELECT "t"."Id", "t"."Name"
 FROM "TestEntities" AS "t"
-WHERE EXISTS (
-    SELECT 1
-    FROM (SELECT ? AS "Value" UNION ALL VALUES (?), (?)) AS "i"
-    WHERE "i"."Value" = "t"."Id")
+WHERE ? = "t"."Id"
 """);
         }
         else
@@ -123,16 +130,16 @@ SELECT 1
                 """
 SELECT "t"."Id", "t"."Name"
 FROM "TestEntities" AS "t"
-WHERE 1 = "t"."Id"
+WHERE EXISTS (
+    SELECT 1
+    FROM (SELECT 1 AS "Value" UNION ALL VALUES (2), (3)) AS "i"
+    WHERE "i"."Value" = "t"."Id")
 """,
                 //
                 """
 SELECT "t"."Id", "t"."Name"
 FROM "TestEntities" AS "t"
-WHERE EXISTS (
-    SELECT 1
-    FROM (SELECT 1 AS "Value" UNION ALL VALUES (2), (3)) AS "i"
-    WHERE "i"."Value" = "t"."Id")
+WHERE 1 = "t"."Id"
 """);
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -628,7 +628,7 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres`
`628`	`628`	`protected override Expression VisitSqlConstant(SqlConstantExpression sqlConstantExpression)`
`629`	`629`	`{`
`630`	`630`	`_relationalCommandBuilder`
`631`		`- .Append(sqlConstantExpression.TypeMapping!.GenerateSqlLiteral(sqlConstantExpression.Value), sqlConstantExpression.Sensitive);`
	`631`	`+ .Append(sqlConstantExpression.TypeMapping!.GenerateSqlLiteral(sqlConstantExpression.Value), sqlConstantExpression.IsSensitive);`
`632`	`632`
`633`	`633`	`return sqlConstantExpression;`
`634`	`634`	`}`
Original file line number	Diff line number	Diff line change
`@@ -127,9 +127,7 @@ private void InitializeLogCommandTextBuilderIfNeeded(bool redact)`
`127`	`127`	`&& _logCommandTextBuilder is null`
`128`	`128`	`&& !Dependencies.LoggingOptions.IsSensitiveDataLoggingEnabled)`
`129`	`129`	`{`
`130`		`- _logCommandTextBuilder = new();`
`131`		`- _logCommandTextBuilder.Append(_commandTextBuilder.ToString());`
`132`		`- _logCommandTextBuilder.IndentCount = _commandTextBuilder.IndentCount;`
	`130`	`+ _logCommandTextBuilder = _commandTextBuilder.Clone();`
`133`	`131`	`}`
`134`	`132`	`}`
`135`	`133`	`}`