Misleading exception text: FromSqlRaw returns “missing column” error for wrong column

[simonmckenzie]

When using FromSqlRaw, selecting insufficient columns, and one of the correct columns varies in case from the entity's field name, the exception thrown will indicate that the miscased column is missing, even though it's present.

Given this entity class:

[Keyless]
public class TwoFieldEntity
{
    public string FirstProperty { get; set; }
    public string SecondProperty { get; set; }
}

... and this query:

await context
    .Set<TwoFieldEntity>()
    .FromSqlRaw("select 'x' FirstPROPERTY")
    .ToListAsync();

An exception will be thrown (as expected):

System.InvalidOperationException: 'The required column 'FirstProperty' was not present in the results of a 'FromSql' operation.'

The exception message is incorrect, as that property is not missing. It's the second field (SecondProperty) that’s missing.

Note that EF does not mind about mismatched case in general, i.e. this query works fine:

await context
    .Set<TwoFieldEntity>()
    .FromSqlRaw("select 'x' FirstPROPERTY, 'y' SecondPROPERTY")
    .ToListAsync();

EF Core version: 8.0.4
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 8.0
Operating system: Windows 10/11
IDE: Visual Studio 2022 17.9.7)

[simonmckenzie]

I have raised a pull request for this change (sorry, I missed the part about asking for permission first).

My change is very simple - it replaces a .Except used to find missing columns (which naturally used exact string value matching) with use of the existing _fieldNameLookup, which uses a case-insensitive key comparer.

I was a little concerned that using the lookup means I'm now evaluating a Lazy and that this could have a performance impact (assuming that's why it was lazy). I did a blame on the existing code and couldn't see anything about performance being the original reason (the history ran out before I found the original commit). I am assuming the Lazy exists for memory usage reasons, and thus it's acceptable to evaluate it when the query is invalid, as this is not part of normal execution anyway.

[simonmckenzie]

I had another thought - while I feel my current approach is a very readable solution, if there's any concern about performance (not that I really expect there will be), I can always add a new property for the column name comparer, and use it from both the lookup builder and the error builder, e.g.:

private StringComparer ColumnNameComparer => StringComparer.OrdinalIgnoreCase;

// ...

Dictionary<string, int> CreateNameLookup()
{
    var index = new Dictionary<string, int>(ColumnNameComparer);
    for (var i = 0; i < _columnNames.Length; i++)
    {
        index[_columnNames[i]] = i;
    }

    return index;
}

// ...

var firstMissingColumn = _columns
    .Select(c => c?.Name)
    .Where(c => c != null)
    .Except(_columnNames, ColumnNameComparer)
    .FirstOrDefault();

I do still feel this isn't as nice as what I have in the PR, but it will have the same performance as the original code.

[roji]

Note that EF does not mind about mismatched case in general, i.e. this query works fine:

This seems quite suspect to me. Although SQL Server specifically doesn't seem to allow two columns with names that differ only in casing, that is by no means a universal thing (PG allows it, for instance). I can see that our FromSql handling assumes case-insensitive logic because of this (e.g. FromSqlQueryingEnumerable.BuildIndexMap uses StringComparer.OrdinalIgnoreCase), leading to errors on PG when two such properties are defined.

I think we should consider making the logic fully case-sensitive, though that would be a breaking change. I'll bring this up with the team.

[simonmckenzie]

Hi @roji,

It's an interesting point about whether EF should be strict around column case, but perhaps that should be a new issue...?

My issue is just about the incorrect error messaging - perhaps some background will help, as this caused a fair bit of confusion in my team, and it would be great if this could be fixed soon (as changing case rules in EF sounds like it will be a big discussion that will take a while to resolve):

• We had a stored procedure and an EF model to hold the output of that proc. This worked fine, even though one of the output columns had a column whose casing did not match that of the EF model's property.
• A new field was added to the model.
• The query started failing, saying that one of the existing columns was not present in the output (this was the column with the mismatched casing).
• Much time was wasted looking in the wrong place for the problem.

Thanks,
Simon

[roji]

but perhaps that should be a new issue...?

It definitely should (based on the design discussion we'll have in the team), but what we decide there will affect what we do here, of course. I don't think we'd go into fixing this (which is, after all, just a misleading exception text and not even an actual behavioral bug) if we decide to just go case-sensitive for column names.

[simonmckenzie]

@roji, I know this is just the opinion of a random person on the internet (me), but it seems that while some SQL flavours do allow queries to return multiple columns with the same name but different casing, actually doing it sounds like a code smell.

I'm not convinced that EF should support it, although I come from a Sql Server background, and I may be missing the use cases where reusing a name could be helpful 🤷

I'm definitely interested in hearing the results of the EF team's discussion!

[roji]

while some SQL flavours do allow queries to return multiple columns with the same name but different casing, actually doing it sounds like a code smell.

Regardless of whether it's a code smell or not, I don't think EF should be using case-insensitive logic and assume that names which are identical except for case are impossible; that simply doesn't correspond to reality. Programming languages like C# are case-sensitive as well, regardless of whether it's a smell to have two variables with names which are identical except for case.

[roji]

Design discussion: re EF using case-insensitive logic on columns names where the database is case-sensitive, we consider this a bug; opened #33885 to fix this.

Am leaving this issue in the backlog to track improving the error message for providers where the case-insensitive logic will be kept (we're unlikely to get to this any time soon).