Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: Translate string.Format and string interpolation #6532

Closed
rowanmiller opened this issue Sep 14, 2016 · 14 comments
Closed

Query: Translate string.Format and string interpolation #6532

rowanmiller opened this issue Sep 14, 2016 · 14 comments
Labels
area-query closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement

Comments

@rowanmiller
Copy link
Contributor

Spawned from #6517 where string interpolation was causing extensive client side eval.

_context.Participants
    .Where(m => m.FirstName.ToLower().Contains(searchTerm) 
                || m.LastName.ToLower().Contains(searchTerm) 
                || $"#{m.Id}" == searchTerm);
@colotiline
Copy link

Excuse me for boring you, but where I can read why compiler translates interpolation differently for IEnumerable and IQueryable?

I'm asking because compiler converts string interpolation to concatenation for IEnumerable and kept it in a LINQ expression.

  • I've created a console app (dotnet new console).
  • Create an array of objects and use a where clause with string interpolation.
  • After looking on compiled code with ILSpy I've got string concatenation Where((Func<Something, bool>)((Something item) => item.Prop1 + "-" + item.Prop2 == "prop1.1-prop1.2")).
  • But when I use DbContext - compiler kept string interpolation in where clause Where((Expression<Func<Something, bool>>)((Something item) => $"{item.Prop1}-{item.Prop2}" == "prop1.1-prop1.2")).

Why?

IEnumerable
Source code

using System;
using System.Linq;

namespace intrp
{
    class Program
    {
        static void Main(string[] args)
        {
            var temp = new Something[] 
            {  
                new Something
                {
                    Prop1 = "prop1.1",
                    Prop2 = "prop1.2"
                },
                new Something
                {
                    Prop1 = "prop2.1",
                    Prop2 = "prop2.2"
                }
            };

            var query =
                from item in temp
                where
                    $"{item.Prop1}-{item.Prop2}" == "prop1.1-prop1.2"
                select item;

            var items = query.ToArray();

            foreach (var item in items)
            {
                Console.WriteLine(item.Prop1);
                Console.WriteLine(item.Prop2);
            }
        }

        public class Something 
        {
            public string Prop1 { get; set; }
            public string Prop2 { get; set; }
        }
    }
}

ILSpy result

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.Versioning;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.DisableOptimizations | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue)]
[assembly: TargetFramework(".NETCoreApp,Version=v3.0", FrameworkDisplayName = "")]
[assembly: AssemblyCompany("intrp")]
[assembly: AssemblyConfiguration("Debug")]
[assembly: AssemblyFileVersion("1.0.0.0")]
[assembly: AssemblyInformationalVersion("1.0.0")]
[assembly: AssemblyProduct("intrp")]
[assembly: AssemblyTitle("intrp")]
[assembly: AssemblyVersion("1.0.0.0")]
namespace intrp
{
	internal class Program
	{
		public class Something
		{
			public string Prop1
			{
				get;
				set;
			}

			public string Prop2
			{
				get;
				set;
			}
		}

		private static void Main(string[] args)
		{
			Something[] source = new Something[2]
			{
				new Something
				{
					Prop1 = "prop1.1",
					Prop2 = "prop1.2"
				},
				new Something
				{
					Prop1 = "prop2.1",
					Prop2 = "prop2.2"
				}
			};
			IEnumerable<Something> source2 = ((IEnumerable<Something>)source).Where((Func<Something, bool>)((Something item) => item.Prop1 + "-" + item.Prop2 == "prop1.1-prop1.2"));
			Something[] array = source2.ToArray();
			Something[] array2 = array;
			foreach (Something something in array2)
			{
				Console.WriteLine(something.Prop1);
				Console.WriteLine(something.Prop2);
			}
		}
	}
}

IQueryable
Source code

using System;
using Microsoft.EntityFrameworkCore;
using System.Linq;

namespace intrp
{
    class Program
    {
        static void Main(string[] args)
        {
            var db = new Db();

            var query =
                from item in db.Somethings
                where
                    $"{item.Prop1}-{item.Prop2}" == "prop1.1-prop1.2"
                select item;

            var items = query.ToArray();

            foreach (var item in items)
            {
                Console.WriteLine(item.Prop1);
                Console.WriteLine(item.Prop2);
            }
        }

        public class Something 
        {
            public string Prop1 { get; set; }
            public string Prop2 { get; set; }
        }

        public class Db : DbContext
        {
            public DbSet<Something> Somethings { get; set; }
        }
    }
}

ILSpy result

using Microsoft.EntityFrameworkCore;
using System;
using System.Diagnostics;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.Versioning;

[assembly: CompilationRelaxations(8)]
[assembly: RuntimeCompatibility(WrapNonExceptionThrows = true)]
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default | DebuggableAttribute.DebuggingModes.DisableOptimizations | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints | DebuggableAttribute.DebuggingModes.EnableEditAndContinue)]
[assembly: TargetFramework(".NETCoreApp,Version=v3.0", FrameworkDisplayName = "")]
[assembly: AssemblyCompany("intrp")]
[assembly: AssemblyConfiguration("Debug")]
[assembly: AssemblyFileVersion("1.0.0.0")]
[assembly: AssemblyInformationalVersion("1.0.0")]
[assembly: AssemblyProduct("intrp")]
[assembly: AssemblyTitle("intrp")]
[assembly: AssemblyVersion("1.0.0.0")]
namespace intrp
{
	internal class Program
	{
		public class Something
		{
			public string Prop1
			{
				get;
				set;
			}

			public string Prop2
			{
				get;
				set;
			}
		}

		public class Db : DbContext
		{
			public DbSet<Something> Somethings
			{
				get;
				set;
			}
		}

		private static void Main(string[] args)
		{
			Db db = new Db();
			IQueryable<Something> source = ((IQueryable<Something>)db.Somethings).Where((Expression<Func<Something, bool>>)((Something item) => $"{item.Prop1}-{item.Prop2}" == "prop1.1-prop1.2"));
			Something[] array = source.ToArray();
			Something[] array2 = array;
			foreach (Something something in array2)
			{
				Console.WriteLine(something.Prop1);
				Console.WriteLine(something.Prop2);
			}
		}
	}
}

@smitpatel
Copy link
Contributor

Enumerable methods creates an enumerable with deferred execution, hence it contains actual method funcs. Since it is func, it will be converted to appropriate methods to remove string interpolation. Queryable methods builds the expression tree. So expression tree contains whatever original code was in hence it contains string interpolation.

@michalczerwinski
Copy link
Contributor

There is a FORMATMESSAGE available in all supported versions (both SQL Server and Azure SQL): https://docs.microsoft.com/en-us/sql/t-sql/functions/formatmessage-transact-sql?view=sql-server-ver15

By replacing string.Format parameter placeholders ({0}, {1}, etc.) with proper FORMATMESSAGE equivalents (%s, %i, %d, etc.) based on the argument type similar behaviour can be obtained.

Similar technique can be used for interpolated strings.

Draft solution:

  • add new class SqlServerStringFormatTranslator and register in SqlServerMethodCallTranslatorProvider
  • implement translation in Translate method
  • add unit tests in GearsOfWarQueryTestBase.cs to cover this

@smitpatel, @ajcvickers, @roji: does it make sense? Would you be interested in such a contribution?

@roji
Copy link
Member

roji commented May 4, 2021

@michalczerwinski if I'm understanding your proposal, you'd be parsing the format string to replace the placeholders (which most probably means only constant format strings would be supported). Am personally a bit skeptical - .NET format strings have many different options, only a (small) subset of which would be translatable... And it generally seems like users can use regular string concatenation and conversions, so this doesn't seem to have huge value. IMHO it makes more sense to just provide EF.Functions.FormatMessage for SQL Server - that's trivial to implement, and would also be more powerful because it would support anything FORMATMESSAGE supports.

Would be good to hear what others think though.

@michalczerwinski
Copy link
Contributor

michalczerwinski commented May 4, 2021

@roji: yes, you are right. In string.Format scenario dynamic format (first parameter) would not be supported.
But string interpolation could be fully supported as even in .net core dynamic values for format are not supported.

In the meantime I verfied there is build-in printf function in SQLite which can be used to mimic the behaviour of the SQL Server provider: https://sqlite.org/lang_corefunc.html#printf

EF.Functions.FormatMessage: I can add it along my change for completeness, as well ;)

@michalczerwinski
Copy link
Contributor

I did some further tests and even created simple POC. From this EF query:

Set<Weapon>().Where(w => w.IsAutomatic.ToString() == $"Prefix{w.Id}"));

I managed to get this in SQL Server:

SELECT [w].[Id], [w].[AmmunitionType], [w].[IsAutomatic], [w].[Name], [w].[OwnerFullName], [w].[SynergyWithId]
FROM [Weapons] AS [w]
WHERE CASE
    WHEN [w].[IsAutomatic] = CAST(0 AS bit) THEN N'False'
    ELSE N'True'
END = (N'Prefix' + CAST([w].[Id] AS nvarchar(max)))

I used the following condition to detect the case in IMethodCallTranslator.Translate:

            if (instance == null
                && method.Name == nameof(string.Format)
                && arguments.Count > 0
                && arguments[0] is SqlConstantExpression formatExpression
                && formatExpression.Type == typeof(string))

It includes the aspect mentioned by @roji (format argument being constant).

At this stage I am sure I can deliver this for both SQL Server and SQLite. I also think it's better to use manual string concatenation rather than using FORMATMESSAGE or printf approach.

Please share some thoughts and more importantly please confirm this is something you would allow for EF Core.

@ajcvickers ajcvickers removed this from the Backlog milestone May 5, 2021
@michalczerwinski
Copy link
Contributor

@ajcvickers : I noticed you added needs-design label for this one.

One thing which can be potentially interesting is a new Roslyn proposal for dotnet/csharplang#2302
Tweaking it a bit can be beneficial as would allow EF Core to avoid reparsing string.Format message.

@roji
Copy link
Member

roji commented May 12, 2021

@michalczerwinski it's unlikely EF Core would be able to support the new optimized string interpolation, since the compiler generates multiple statements out of it (see the sample here), and that's not going to be compatible with LINQ expression trees (e.g. you can't put an assignment inside an expression tree).

@roji
Copy link
Member

roji commented May 12, 2021

The needs-design is for the team to discuss whether this feature is something we think should be implemented or not - we'll post back with the feedback.

@michalczerwinski
Copy link
Contributor

@roji: if I understand the situation the discussed change won't modify expression tree behaviour.
Thanks for clarifying on "needs-design", to be honest I was counting on being able to participate is some discussion here ;)

But ok, waiting patiently for your decision then.

@roji
Copy link
Member

roji commented May 12, 2021

Design decision: we don't think that a translation of string.Format (which is what interpolated strings usually compile into) makes sense for EF Core. However, we'd accept a PR adding an EF.Functions.FormatMessage for SQL Server.

  • We don't think we can reliably translate anything beyond constant format strings which purely express concatenation (i.e. no alignment or format strings within the curly braces).
    • This is because format strings have very versatile and frequently locale-sensitive behavior which we wouldn't be able to translate to SQL without considerable risk of discrepancies.
    • Even if some subset can be translated faithfully, we don't want to introduce translations which work for some restricted cases, and then fail for others; this is a problematic experience for users etc.
  • For the purely constant concatenation scenario, the value of using interpolated strings for expressing that (rather than just C# concatenation) seems limited, especially given that EF would need to correctly parse the format string, etc.
  • We generally avoid translating native .NET methods where the correspondence with SQL isn't very close; providing a function under EF.Functions makes it very clear exactly what is supported and what isn't, etc.

@michalczerwinski we can certainly continue the discussion here, and if any new arguments or information come up, the decision can be changed. I'm going to close this for now, though again that doesn't mean the discussion shouldn't continue.

@roji roji closed this as completed May 12, 2021
@roji roji added closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed needs-design labels May 12, 2021
@michalczerwinski
Copy link
Contributor

@roji: thanks for being transparent, appreciated!

@seikosantana
Copy link

seikosantana commented Feb 4, 2024

Just had an issue with string interpolation, I see that string interpolation is not (yet) possible at the moment

[Projectable]
public string FullName => $"{Name} {ThicknessMm}mm {WidthCm}cm x {HeightCm}cm";

But is there any recommendations what to do instead of the ones like above?
Is this the only way for that?

[Projectable] public string FullName => Name + " " + ThicknessMm + "mm " + WidthCm + "cm x " + HeightCm + "cm";

Thanks in advance

@roji
Copy link
Member

roji commented Feb 5, 2024

@seikosantana yes, read the above comments for context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement
Projects
None yet
Development

No branches or pull requests

7 participants