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

Invalid column name 'key' in SQL Server JSON query #32910

Closed
roji opened this issue Jan 24, 2024 · 5 comments
Closed

Invalid column name 'key' in SQL Server JSON query #32910

roji opened this issue Jan 24, 2024 · 5 comments
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@roji
Copy link
Member

roji commented Jan 24, 2024

The repro below causes the following exception to be thrown in release/8.0, but not in main:

Unhandled exception. Microsoft.Data.SqlClient.SqlException (0x80131904): Invalid column name 'key'.
Invalid column name 'key'.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
Repro
await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

_ = ctx.Parent
    .Select
    (
        x => new ParentDto
        {
            CalculatedValue2 = x.Child.InfoData.Fields.FirstOrDefault(x => x.Code == "Total2").Value,
            FullNumber = x.Phone.FullNumber
        }
    )
    .OrderBy(x => x.CalculatedValue2)
    .Take(10)
    .ToList();

public class BlogContext : DbContext
{
    public DbSet<Parent> Parent { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer("Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Child>().ToTable(nameof(Child)).OwnsOne
        (
            child => child.InfoData, builder =>
            {
                builder.ToJson();
                builder.OwnsMany(x => x.Fields);
            }
        );
    }
}

public class Parent
{
    public long Id { get; set; }

    public long? ChildId { get; set; }
    public long? PhoneId { get; set; }

    public Child Child { get; set; }
    public Phone Phone { get; set; }
}

public class Child
{
    public long Id { get; set; }
    public InfoData InfoData { get; set; }
}

public class Phone
{
    public long Id { get; set; }
    public string? FullNumber { get; set; }
}

public class InfoData
{
    public List<Field> Fields { get; set; }
}

public class Field
{
    public string? Code { get; set; }
    public int? Value { get; set; }
}

public class ParentDto
{
    public long ParentId { get; set; }

    public int? SomeValue1 { get; set; }
    public int? SomeValue2 { get; set; }
    public int? CalculatedValue { get; set; }
    public int? CalculatedValue2 { get; set; }

    public string FullNumber { get; set; }
}

The issue disappears in main after #32812, which was a big query architecture rewrite. This is most likely another mutability-related referential integrity bug, similar to #32234.

Originally opened by @SergMarkovych in npgsql/efcore.pg#3066

@roji
Copy link
Member Author

roji commented Jan 26, 2024

Confirmed that this is another referential integrity bug, where TableReferenceExpression is shared by two columns within a multi-referenced SelectExpression; this is very similar to #32234, but when tables get replaced via SelectExpression.Update instead of via VisitChildren.

This is not a regression: the repro requires composing a query on top of a collection instead a JSON owned entity, which is a feature that we added in 8.0. I have a fix standing by (https://github.com/roji/efcore/tree/UpdateReferentialIntegrity) - if many users run into this, we can consider patching.

(once again, this is already fixed for 9.0)

@roji roji added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed Servicing-consider regression labels Jan 26, 2024
@ajcvickers ajcvickers added this to the 9.0.0-preview1 milestone Jan 31, 2024
@roji roji modified the milestones: 9.0.0-preview1, 9.0.0 Oct 12, 2024
@diablo2947
Copy link

Hi can we have a fix for this in EF Core 8 as well since that is supposed to be LTS. I have just upgraded my app to dotnet 8 to take advantage of JSON columns.

Similar to the original issue, getting Invalid column name 'key' when using Skip/Take on objects with JSON columns. Should I add a key field to my JSON schema as a workaround?

@roji
Copy link
Member Author

roji commented Nov 26, 2024

@diablo2947 the fix here is a pretty huge rearchitecting in EF's query pipeline that happened for EF 9.0 (#32812), and that's certainly not something we'd backport to 8.0. Unfortunately, not all fixes can be backported to the LTS, and we haven't received user requests for a backport so far (yours is the first).

@diablo2947
Copy link

thanks, for your reply @roji. I had upgraded to EF8.0 pretty recently anyways.

I just upgraded to EF9.0 and I can confirm that the bug went away.
I was also affected by the bug #32234 for Contains/JSON, which also seems to have been fixed in EF9.0. Cheers!

@roji
Copy link
Member Author

roji commented Nov 26, 2024

@diablo2947 that's great to hear. Yeah, I'd absolutely recommend going to 9.0 unless you have some very strict policy to be on LTS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

No branches or pull requests

3 participants