efcore icon indicating copy to clipboard operation
efcore copied to clipboard

Consider removing the CAST to BIT for boolean literal values

Open roji opened this issue 4 years ago • 11 comments

Our SQL representation of boolean literal values is CAST(1 AS BIT). However, in at least some cases, the CAST produces an inferior query plan. The following example shows this when calling hierarchyid's IsDescendantOf:

Hierarchyid plan without CAST

SELECT COUNT(*) FROM Data WHERE Hid.IsDescendantOf('/') = 1;

Plan:

SELECT COUNT(*) FROM Data WHERE Hid.IsDescendantOf('/') = 1,1,1,0,,,1,,1,,,,4.456616,,,SELECT,false,
"  |--Compute Scalar(DEFINE:([Expr1002]=CONVERT_IMPLICIT(int,[Expr1003],0)))",1,2,1,Compute Scalar,Compute Scalar,"DEFINE:([Expr1002]=CONVERT_IMPLICIT(int,[Expr1003],0))","[Expr1002]=CONVERT_IMPLICIT(int,[Expr1003],0)",1,0,0,11,4.456616,[Expr1002],,PLAN_ROW,false,1
       |--Stream Aggregate(DEFINE:([Expr1003]=Count(*))),1,3,2,Stream Aggregate,Aggregate,,[Expr1003]=Count(*),1,0,0.6000005,11,4.456616,[Expr1003],,PLAN_ROW,false,1
"            |--Index Seek(OBJECT:([master].[dbo].[Data].[IX_name]), SEEK:([master].[dbo].[Data].[Hid] >= / AND [master].[dbo].[Data].[Hid] <= Showplan: failed to convert to string from hierarchyid) ORDERED FORWARD)",1,4,3,Index Seek,Index Seek,"OBJECT:([master].[dbo].[Data].[IX_name]), SEEK:([master].[dbo].[Data].[Hid] >= / AND [master].[dbo].[Data].[Hid] <= Showplan: failed to convert to string from hierarchyid) ORDERED FORWARD",,1000000,2.7564583,1.100157,9,3.8566153,,,PLAN_ROW,false,1

Hierarchyid plan with CAST

SELECT COUNT(*) FROM Data WHERE Hid.IsDescendantOf('/') = CAST(1 AS bit);

Plan:

SELECT COUNT(*) FROM Data WHERE Hid.IsDescendantOf('/') = CAST(1 AS bit),1,1,0,,,1,,1,,,,4.6366158,,,SELECT,false,
"  |--Compute Scalar(DEFINE:([Expr1002]=CONVERT_IMPLICIT(int,[Expr1004],0)))",1,2,1,Compute Scalar,Compute Scalar,"DEFINE:([Expr1002]=CONVERT_IMPLICIT(int,[Expr1004],0))","[Expr1002]=CONVERT_IMPLICIT(int,[Expr1004],0)",1,0,0,11,4.6366158,[Expr1002],,PLAN_ROW,false,1
       |--Stream Aggregate(DEFINE:([Expr1004]=Count(*))),1,3,2,Stream Aggregate,Aggregate,,[Expr1004]=Count(*),1,0,0.6000005,11,4.6366158,[Expr1004],,PLAN_ROW,false,1
            |--Filter(WHERE:([master].[dbo].[Data].[Hid].IsDescendantOf(/)=(1))),1,4,3,Filter,Filter,WHERE:([master].[dbo].[Data].[Hid].IsDescendantOf(/)=(1)),,1000000,0,0.18,9,4.0366154,,,PLAN_ROW,false,1
                 |--Index Scan(OBJECT:([master].[dbo].[Data].[IX_name])),1,5,4,Index Scan,Index Scan,OBJECT:([master].[dbo].[Data].[IX_name]),[master].[dbo].[Data].[Hid],1000000,2.7564583,1.100157,13,3.8566153,[master].[dbo].[Data].[Hid],,PLAN_ROW,false,1

Test SQL

hierarchyid test
CREATE TABLE Data (Id INT IDENTITY(1,1) PRIMARY KEY, Hid HIERARCHYID);
CREATE INDEX IX_name ON Data(Hid);

BEGIN TRANSACTION;

DECLARE @i INT = 0;
WHILE @i < 50000
BEGIN
    INSERT INTO Data (Hid) VALUES ('/1/3/');
    SET @i = @i + 1;
END;
COMMIT;

SET SHOWPLAN_ALL ON;
SELECT COUNT(*) FROM Data WHERE Hid.IsDescendantOf('/') = 1;
SELECT COUNT(*) FROM Data WHERE Hid.IsDescendantOf('/') = CAST(1 AS bit);
SET SHOWPLAN_ALL OFF;

Note that in other scenarios, e.g. simply comparing a regular BIT column to literal true/false, the degradation does not occur (this could be specific to hierarchyid, though who knows):

Regular column test
CREATE TABLE Data (Id INT IDENTITY(1,1) PRIMARY KEY, IsSomething BIT);
CREATE INDEX IX_name ON Data(IsSomething);

BEGIN TRANSACTION;

DECLARE @i INT = 0;
WHILE @i < 500000
BEGIN
    INSERT INTO Data (IsSomething) VALUES (1);
    SET @i = @i + 1;
END;

WHILE @i < 1000000
BEGIN
    INSERT INTO Data (IsSomething) VALUES (0);
    SET @i = @i + 1;
END;

COMMIT;

SET SHOWPLAN_ALL ON;
SELECT COUNT(*) FROM Data WHERE IsSomething = 1;
SELECT COUNT(*) FROM Data WHERE IsSomething = CAST(1 AS bit);
SET SHOWPLAN_ALL OFF;

Flagged by @diogonborges in https://github.com/dotnet/efcore/issues/23472#issuecomment-1007618566

roji avatar Jan 10 '22 11:01 roji

Is hierarchyid officially supported in the SQL Server provider?

ErikEJ avatar Jan 10 '22 12:01 ErikEJ

There's an unofficial extension we maintain.

But my fear is that if there's a different plan in this case, there could be other cases we don't know about. Yet another example that generating minimal/lighter SQL is always a good idea.

roji avatar Jan 10 '22 12:01 roji

I'm curious as if, in the meanwhile, there anything that our code can tap into to hotfix, before this issue gets to see daylight? Does EFCore allow customization of default representation for literal values? I'm looking to avoid using interceptors, so I'm hoping that that isn't the last resource.

diogonborges avatar Jan 10 '22 13:01 diogonborges

A command interceptor is probably the reasonable thing to do here: you would identify commands that do hierarchy operations (either by tagging them with EF Core, or simply by searching for hierarchy ID function names), and replace CAST(1 AS BIT) with 1. You could also write an expression visitor to remove the casting inside the query pipeline, but I'm honestly not sure that's better in this case.

roji avatar Jan 10 '22 13:01 roji

Blocked on #15586

smitpatel avatar Jan 10 '22 17:01 smitpatel

The general objective here is to end up with 1 as the bool literal on SQL Server instead of CAST(1 AS BIT).

If I understand correctly, the approach blocked on #15586 would involve comparing to an int (requiring knowledge on what's comparable to what in SQL).

An alternative would be to simply change the literal representation in SqlServerBoolTypeMapping to simply not have the CAST. This would cause issues when a bool constant is being projected (#24075), since a bare 1 or 0 can't be read into a .NET bool.

I think that should be considered as part of the more global problem of projecting constant representations with evaluate to a different type - continuing this thought in #24075.

roji avatar Jan 11 '22 19:01 roji

I am currently running in this issue, is there a work around for this ?

iduras3 avatar May 04 '22 08:05 iduras3

@iduras3 see the command interceptor approach discussed above.

roji avatar May 04 '22 09:05 roji

We're seeing a very similar issue however we're using external tables in Azure SQL. It's supposed to push the WHERE clause to the remote database server but having a CAST(0 AS BIT) in the query is causing it to instead pull the whole table in, then do the WHERE filter locally instead.

In the example below, we're using EF global filters to not pull back any soft deleted records

Expression<Func<object, bool>> IsSoftDeletedFilter => e => ((ISoftDeletable)e) == null || !((ISoftDeletable)e).IsDeleted;

SELECT [d1].[Id], [d1].[CompanyId], [d1].[IsDeleted], [d1].[Name], [d1].[OrderBy]
    FROM [Directory_Nationality] AS [d1]
    WHERE ([d1].[IsDeleted] = CAST(0 AS bit))

image

SELECT [d1].[Id], [d1].[CompanyId], [d1].[IsDeleted], [d1].[Name], [d1].[OrderBy]
    FROM [Directory_Nationality] AS [d1]
    WHERE ([d1].[IsDeleted] = 0)

image

scp-mb avatar Oct 05 '22 15:10 scp-mb

For a detailed look at the bool/bit cast problem on the global QueryFilter.

https://github.com/dotnet/efcore/issues/29930

hgedik avatar May 18 '23 23:05 hgedik

For folks who come to this issue and are interested, this is what I came up with for the Interceptor suggestion. It handles the projection problem by checking to see if the CAST is happening right before AS [someColumn]. If it is, then don't change it. Not sure if this will cover every scenario, so use at your own risk/discretion.

/// <summary>
/// Intercepter to replace the <c>CAST(0 AS bit)</c> with <c>0</c> and <c>CAST(1 AS bit)</c> with <c>1</c>.
/// This works around https://github.com/dotnet/efcore/issues/27150 which can cause performance issues.
/// </summary>
/// <remarks>Does not replace <c>CAST(0 AS bit)</c> when followed by <c>AS [somename]</c> so as not to break projection.  Also doesn't do the replacement in the scenario of CASE THEN ELSE results.</remarks>
internal partial class BooleanCommandInterceptor : DbCommandInterceptor
{
    [GeneratedRegex(@"(?<!\bTHEN\s+|\bELSE\s+)CAST\((0|1) AS bit\)(?!\s+AS\s+\[\w+\])")]
    private static partial Regex BitCastRegex();
    
    public override InterceptionResult<DbDataReader> ReaderExecuting(
        DbCommand command,
        CommandEventData eventData,
        InterceptionResult<DbDataReader> result)
    {
        ManipulateCommand(command);

        return result;
    }

    public override ValueTask<InterceptionResult<DbDataReader>> ReaderExecutingAsync(
        DbCommand command,
        CommandEventData eventData,
        InterceptionResult<DbDataReader> result,
        CancellationToken cancellationToken = default)
    {
        ManipulateCommand(command);

        return new ValueTask<InterceptionResult<DbDataReader>>(result);
    }

    private static void ManipulateCommand(DbCommand command)
    {
        var matches = BitCastRegex().Matches(command.CommandText);
        if (matches.Count != 0)
        {
            command.CommandText = BitCastRegex().Replace(command.CommandText, "$1");
        }
    }
}

Edit: 10/15, fixed a problem where CASE THEN ELSE wouldn't work because it was removing the cast from the results. Fixed that.

jwyza-pi avatar Oct 07 '24 15:10 jwyza-pi

Hi! We've are experiencing a similar issue! Are there any plans to fix it?

uittorio avatar Nov 01 '24 11:11 uittorio

This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 8.0). We will re-assess the backlog following the this release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources. Make sure to vote (👍) for this issue if it is important to you.

roji avatar Nov 05 '24 00:11 roji