SqlServer: Translate bool ToString

[MaxG117]

What's the idea behind converting values to string (using either object.ToString() or Convert.ToString(object))? Should the resulting string be compatible with the C# or the server-side string representation of the object?

For example, a C# bool is stored in a SqlServer bit column. Should conversion to string result in "0" and "1" or "true" and "false"?

[smitpatel]

"true" or "false". C# representation. It could be achieved via conditional for bool case or for other case where it is not possible it should not try to convert to string on server. Especially in scenarios where there are value converters applied.

[MaxG117]

I don't follow. SqlServerObjectToStringTranslator translates boolValue.ToString() into "CONVERT(VARCHAR(5), boolValue)". I believe the result of this is "0" and "1", not "true" and "false". I do not see any "conditional for bool case".

[smitpatel]

Then it's a bug.

[MaxG117]

You sound uncertain, and I too could be wrong (just because I don't see a "conditional for bool case" doesn't mean it's not there). Could you have someone look at the source code, confirm whether this is indeed a bug and explain what the fix should be? If it's a bug, then there's probably a missing test.

[smitpatel]

                var query = db.Blogs.Where(b => b.Value.ToString() == "true").ToList();
                Console.WriteLine(query.Count);

outputs following SQL

      SELECT [b].[Id], [b].[Value]
      FROM [Blogs] AS [b]
      WHERE CONVERT(VARCHAR(5), [b].[Value]) = N'true'

Which of course produces incorrect results. EF Core 2.2.0

[MaxG117]

What about other types, like DateTime? Should dateTime.ToString() and Convert.ToString(dateTime) also return the C# representation of those strings?

[smitpatel]

@MaxG117 - Isn't that covered in #14193 already?

[MaxG117]

@smitpatel That's the issue where I pointed out an inconsistency between dateTime.ToString() and Convert.ToString(dateTime), but it does not describe the expected string format.

Additionally, DateTimeOffset has a bigger problem, because there's no Convert.ToString(DateTimeOffset) method. You may want to split this up into three separate bugs. For now, I'm looking for guidance regarding expected string formats.

[smitpatel]

@MaxG117 - For expected string format you can read discussion here #7364

Essentially, we are not going to do anything other than default unless there is really strong demand for it.

#14193 - Will track to make all possible conversions (initiated through instance.ToString or Convert.ToString), work in default format. It is an enhancement.
This issue is bug - at the very least, the bool values needs to be stopped from going to server translation.
We are not going to look at all formatters. Any specific formatter would be high user feedback based.

If you want your custom formatter to be used server side, then you can add DbFunction in EF Core which would map to server with your specified formatter and we will translate into it during query.

[MaxG117]

@smitpatel

For expected string format you can read discussion here #7364

Actually I think #6264 is more relevant to my question.

This issue is bug - at the very least, the bool values needs to be stopped from going to server translation

I believe it is not ideal to stop bool values from going to server translation. For example, if a bool is used in a Where clause, the client would end up fetching and processing the entire result set. I realize nobody in their right mind should be converting booleans to strings in the Where clause (or anywhere, really), but as long as conversion of boolean to string is supported, it may as well be translated on the server.

[smitpatel]

I believe it is not ideal to stop bool values from going to server translation.

Yes, it is not ideal. But doing client eval rather than giving incorrect result is much better. Incorrect results could cause data corruption.

[MaxG117]

doing client eval rather than giving incorrect result is much better

Why would translating a boolean to a string on the server produce incorrect results?

By the way, your example above:

var query = db.Blogs.Where(b => b.Value.ToString() == "true").ToList();

wouldn't work, because true.ToString() returns "True" (capitalized).

I'm suggesting doing everything on the server, so that this query:

      bool myBool = true;
      var query = db.Blogs.Where(b => b.Value.ToString() == myBool.ToString()).ToList();

produces the following SQL:

      SELECT [b].[Id], [b].[Value]
      FROM [Blogs] AS [b]
      WHERE [b].[Value] = 1

[smitpatel]

Why would translating a boolean to a string on the server produce incorrect results?

Translating a Boolean to sever in current way produces incorrect results. Stopping it from going to server at least avoid incorrect results. Translating it correctly to server would be most efficient. So is all other pending translation which a lot of customers ask for. At the end of the day, we have limited resources and time. We cannot translate everything to server side in next release. Hence I posted, we can translate correctly to server at some point, but for the next release, we need to stop translating to avoid incorrect result.

[afranioce]

I have a similar issue!

Steps to reproduce

1. Create an entity with a boolean nullable property:

public class EntryDocument
{
    public bool? IsCompleted { get; private set; }
}

2. Add a property configuration

public class SupplyEntryDocumentMap : IEntityTypeConfiguration<EntryDocument>
{
    public void Configure(EntityTypeBuilder<EntryDocument> builder)
    {
        builder.Property(r => r.IsCompleted)
            .HasColumnName("Ent_Concluido")
            .HasMaxLength(1)
            .IsFixedLength()
            .HasConversion(new BoolToStringConverter("N", "S")); // Not or Yes in pt-BR ;)
    }
}

3. Create an filter into repository

public class EntryDocumentRepository
{
    public IQueryable<EntryDocument> GetByStatus(bool? isCompleted)
    {
        return from entryDocument in DbSet.AsNoTracking()
               where isCompleted == null || (entryDocument.IsCompleted ?? false == isCompleted)
               select entryDocument;
    }
}

4. should result in SQL similar to this

WHERE (COALESCE([entryDocument].[Ent_Concluido], CASE
    WHEN N'N' = @__isCompleted_0
    THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)

OR change filter to this

where isCompleted == null || (entryDocument.IsCompleted ?? false) == isCompleted

SQL executed...

WHERE COALESCE([entryDocument].[Ent_Concluido], N'N', @__isCompleted_0)

Exception:

System.Data.SqlClient.SqlException (0x80131904): Conversion failed when converting the varchar value 'N' to data type bit.
   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlDataReader.TryHasMoreRows(Boolean& moreRows)
   at System.Data.SqlClient.SqlDataReader.TryReadInternal(Boolean setTimeout, Boolean& more)
   at System.Data.SqlClient.SqlDataReader.<>c__DisplayClass190_0.<ReadAsync>b__1(Task t)
   at System.Data.SqlClient.SqlDataReader.InvokeRetryable[T](Func`2 moreFunc, TaskCompletionSource`1 source, IDisposable objectToDispose)

It works normally if not set default value to false like: (entryDocument.IsCompleted == isCompleted)

Further technical details

EF Core version: 2.2
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: MacOS Mojave 10.14.4
IDE: Visual Studio 2019 for mac

[michalczerwinski]

Is this something I can help with? It looks like we need to add some case in SqlServerObjectToStringTranslator, right?
Probably converting to IIF(boolExpression, 'true', 'false') would be the most succinct?

[MaxG117]

I added something like this to Translate in my provider's ObjectToStringTranslator:

if (instance.Type.UnwrapNullableType() == typeof(bool))
{
    return _sqlExpressionFactory.Case(
        new[]
        {
            new CaseWhenClause(
                _sqlExpressionFactory.Equal(instance, _sqlExpressionFactory.Constant(false)),
                _sqlExpressionFactory.Constant(false.ToString()))
        },
        _sqlExpressionFactory.Constant(true.ToString()));
}
else
{
    // handle everything else
}

[michalczerwinski]

Yes, thanks @MaxG117 . I was thinking about something similar.
@ajcvickers: Is this something I can take care of? Would you accept PR for this?

[michalczerwinski]

I created #24793 for this. I am not sure about the new test location and testing approach (wasn't able to find anything for SqlServerObjectToStringTranslator). @ajcvickers, @smitpatel: please let me know what you think.

[MaxG117]

By the way, a similar fix should also go into SqlServerConvertTranslator.cs. In my provider's implementation I added typeof(Boolean) to _supportedTypes and changed the Translate method to look something like the following. I also improved on the original by removing an unnecessary CONVERT that was being added for Convert.ToString(String).

public SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
{
    if (_supportedMethods.Contains(method))
    {
        if (method.Name == nameof(Convert.ToString))
        {
            if (arguments[0]?.Type == typeof(string))
            {
                // avoid unnecessarily conversion of string to string
                return arguments[0];
            }
            else if (arguments[0]?.Type == typeof(bool))
            {
                // Convert a bool to its string representation
                return _sqlExpressionFactory.Case(
                    new[] 
                    { 
                        new CaseWhenClause(
                            _sqlExpressionFactory.Equal(arguments[0], _sqlExpressionFactory.Constant(false)),
                            _sqlExpressionFactory.Constant(false.ToString())) 
                    },
                    _sqlExpressionFactory.Constant(true.ToString()));
            }
        }

        // original return here:
        return _sqlExpressionFactory.Function(
            "CONVERT",
            new[] { _sqlExpressionFactory.Fragment(_typeMapping[method.Name]), arguments[0] },
            method.ReturnType);
    }
    else
    {
        return null;
    }
}