dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.8k stars 3.2k forks source link

EF generates sql statement for non existing column in a table #35044

Closed RealCoda closed 1 week ago

RealCoda commented 3 weeks ago

The framework generates a sql statement that selects a column from a table where this column does not exists.

Possibly the framework links a very different table just because of naming.

Code to reproduce

Here is the code i´m using to reproduce.

            using var ctx = new MyContext();
            await ctx.Database.EnsureDeletedAsync();
            await ctx.Database.EnsureCreatedAsync();
            await ctx.Database.ExecuteSqlRawAsync(@"CREATE VIEW [dbo].[AccountBalance]
                    AS (select 1 as Id,  2  as AccountId, 13.4 as Balance) ");

            // this query fails due to a column that does not exist:
            var acc = ctx.Accounts
                .Include(a => a.Balances)
                .FirstOrDefault(a => a.Id == 1);

        public class MyContext : DbContext
        {
            public DbSet<AccountDb> Accounts => Set<AccountDb>();
            public DbSet<AccountBalanceDb> AccountBalances => Set<AccountBalanceDb>();

            // REMOVE THIS LINE to STOP FAILING
            // ADD THIS LINE TO FAIL 
            public DbSet<PermanentBalanceDb> AccountPermanentBalances => Set<PermanentBalanceDb>();

            protected override void OnModelCreating(ModelBuilder modelBuilder)
            {
                modelBuilder.Entity<AccountDb>(entity =>
                {
                    entity.HasKey(a => a.Id);

                    // Configure the relationship
                    entity.HasMany(a => a.Balances)
                        .WithOne(b => b.Account)
                        .HasForeignKey(b => b.AccountId);
                });

                modelBuilder
                    .Entity<AccountBalanceDb>(
                        eb =>
                        {
                            eb.HasKey(a => a.Id);
                            eb.ToView("AccountBalance");
                            eb.HasOne(b => b.Account)
                                .WithMany(a => a.Balances)
                                .HasForeignKey(b => b.AccountId);
                        });

            }

            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            {
                optionsBuilder
                    .UseSqlServer(
                        @"Server=(localdb)\mssqllocaldb;Database=Repro;Trusted_Connection=True;MultipleActiveResultSets=true")
                    .LogTo(Console.WriteLine, LogLevel.Information)
                    .EnableSensitiveDataLogging();

            }
        }

        /// <summary>
        /// Represents an Account in the database
        /// </summary>
        [Table("Accounts")]
        public sealed class AccountDb
        {
            /// <summary>
            /// Id is the primary key of an account, this one is secret
            /// </summary>
            [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
            public int Id { get; set; }

            /// <summary>
            /// Concurrency Token
            /// </summary>
            [Timestamp]
            public byte[]? Version { get; set; }

            /// <summary>
            /// Balances from the View
            /// </summary>
            public List<AccountBalanceDb>? Balances { get; set; }

        }

        [PrimaryKey(nameof(Id))]
        public class AccountBalanceDb
        {

            public int Id { get; set; }

            public int AccountId { get; set; }

            [Precision(18, 4)]
            public decimal Balance { get; set; }

            /// <summary>
            /// Property corresponds to AccountId
            /// </summary>
            [ForeignKey(nameof(AccountId))]
            public AccountDb? Account { get; set; }
        }

        [Table("AccountBalances")]
        public class PermanentBalanceDb
        {
            [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
            public int Id { get; set; }

            public int AccountId { get; set; }

            [ForeignKey(nameof(AccountId))]
            public AccountDb? Account { get; set; }

            /// <summary>
            /// Concurrency Token
            /// </summary>
            [Timestamp]
            public byte[]? Version { get; set; }

        }

SQL Fail

[a0].[Version] is the problem here:

fail: 05.11.2024 09:08:42.254 RelationalEventId.CommandError[20102] (Microsoft.EntityFrameworkCore.Database.Command) Failed executing DbCommand (8ms) [Parameters=[], CommandType='Text', CommandTimeout='30'] SELECT [t].[Id], [t].[Version], [a0].[Id], [a0].[AccountId], [a0].[Balance], [a0].[Version] FROM ( SELECT TOP(1) [a].[Id], [a].[Version] FROM [Accounts] AS [a] WHERE [a].[Id] = 1 ) AS [t] LEFT JOIN [AccountBalance] AS [a0] ON [t].[Id] = [a0].[AccountId] ORDER BY [t].[Id]

Stack Trace

Microsoft.Data.SqlClient.SqlException
  HResult=0x80131904
  Message=Invalid column name 'Version'.
  Source=Core Microsoft SqlClient Data Provider
  StackTrace:
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at Microsoft.Data.SqlClient.SqlDataReader.get_MetaData()
   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteDbDataReader(CommandBehavior behavior)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReader(RelationalCommandParameterObject parameterObject)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.InitializeReader(Enumerator enumerator)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.<>c.<MoveNext>b__21_0(DbContext _, Enumerator enumerator)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.TryGetSingle[TSource](IEnumerable`1 source, Boolean& found)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at EfCoreDemo.Program.<Main>d__0.MoveNext() in C:\Users\real\source\repos\EfCoreDemo\Program.cs:line 19

Include provider and version information

EF Core version: 8.0.10 Database provider: Microsoft.EntityFrameworkCore.SqlServer (8.0.10) Target framework: .NET 8 Operating system: Windows 11 IDE: (Visual Studio 2022 17.11.5)

RealCoda commented 3 weeks ago

There is another possible change to avoid the problem: Change the line

 [Table("AccountBalances")]
        public class PermanentBalanceDb

to something different

 [Table("RandomTableName")]
        public class PermanentBalanceDb

So i´m not even sure if it is a bug or a misconfiguration

AndriySvyryd commented 2 weeks ago

What is the generated SQL for that query when you use [Table("RandomTableName")]?

RealCoda commented 2 weeks ago

Well: just like i would have been expecting:

 SELECT [t].[Id], [t].[Version], [a0].[Id], [a0].[AccountId], [a0].[Balance]
      FROM (
          SELECT TOP(1) [a].[Id], [a].[Version]
          FROM [Accounts] AS [a]
          WHERE [a].[Id] = 1
      ) AS [t]
      LEFT JOIN [AccountBalance] AS [a0] ON [t].[Id] = [a0].[AccountId]
      ORDER BY [t].[Id]

So it is the same like before but without the [a0].Version

ChrisJollyAU commented 2 weeks ago

Just eyeballing this and I'm a little confused (and maybe EF core is getting the same confusion)

We have this

public DbSet<AccountBalanceDb> AccountBalances => Set<AccountBalanceDb>();

If you look at the class itself there is nothing specifically setting the name for the table so will just use AccountBalances

Then your other one

public DbSet<PermanentBalanceDb> AccountPermanentBalances => Set<PermanentBalanceDb>();

We have

[Table("AccountBalances")]
        public class PermanentBalanceDb

by the class definition. This is exactly the same name as the other table. Possibly overriding the previous

So to me it looks like: When tables are created AccountBalances is linked/mapped to AccountBalanceDb class and it uses that for the model to create the tables. But when you are on the query AccountBalances is actually against the PermanentBalanceDb class for the model

You obviously don't run into that situation if both tables have unique names

RealCoda commented 2 weeks ago

Well, your analysis is potentially correct, so where is the possible bug in here ? (if there is) Possibly, that .ToView("someview") does not overwrite the assumed tablename (mapping) ?

I mean, the developer clearly states, that AccountBalances object is mapped to a view, while another entity should be mapped to a certain table. if the developer makes the mistake here, what is the correct code for this mapping?

roji commented 1 week ago

Following @ChrisJollyAU's analysis just above, you basically have two CLR types - PermanentBalanceDb and AccountBalanceDb - mapped to the same table in the database (AccountBalances); these two CLR types differ (one has a Version field, the other does not), and this causes trouble. The eb.ToView("AccountBalance") configuration on AccountBalanceDb indeed maps the latter to the AccountBalance view for querying, but AccountBalanceDb is still mapped to the AccountBalances table for updating - EF Core supports mapping the same entity type to both a table (for updating) and to a view (for querying).

I highly recommend cleaning the model and mapping each entity type to its own, clearly-named and separate table, to avoid confusion.

Assigning to @AndriySvyryd to confirm that the above seems right and in case he has any further comments.

RealCoda commented 1 week ago

Dear posters, i agree that the model is not well defined.
My point was somehow different from that:

1) i defined "AccountBalanceDb" as queryable from a database view ("AccountBalance") - at least i thought i did this 2) which works nice. 3) i defined a complete other table disconnected (from my pov) from any other table. PermanentBalanceDb connected to table of "AccountBalances" 4) suddenly the - still completely disconnected from the new table - query fails.

so now:

and there is another possibility, where the error is completely on my side: (and this is what i read from @roji s statement): "ToView" is not meant to be "MapToDatabaseView", but meant to be "ForQueryUseThisDatabaseObject". Still, when it is mapped to the "AccountBalances" table for updating: . the code above does not update, and fails

var acc = ctx.Accounts.Include(a => a.Balances).FirstOrDefault(a => a.Id == 1);

ChrisJollyAU commented 1 week ago

@RealCoda

  1. Yes you have the query side coming from the view. But that is only for the querying. As mentioned above, for updating (so UPDATE or INSERT) that is still mapped to a normal table. And that defaults to the name used when you defined the DbSet. So that is AccountBalances.
  2. Now you have defined a new object, but told it to use AccountBalances for its name in the database. This now is the same name as your other table
roji commented 1 week ago

and there is another possibility, where the error is completely on my side: (and this is what i read from @roji s statement): "ToView" is not meant to be "MapToDatabaseView", but meant to be "ForQueryUseThisDatabaseObject". Still, when it is mapped to the "AccountBalances" table for updating: . the code above does not update, and fails

I suggest trying to be explicit with your mappings, using ToTable() for the updating mapping and ToView() for the querying (assuming you want these to be different. Once you do that - and ensure that each table is mapped to one entity type and not multiple - things should work.

I'll go ahead and close this for now, but if you still see behavior which you think is wrong, post a new minimal code sample and I can reopen as needed.

RealCoda commented 1 week ago

So, thanks to your points and the information from https://weblogs.asp.net/ricardoperes/read-only-entities-in-ef-core i finally solved it with one extra line of code:

 modelBuilder.Entity<AccountBalanceDb>(
                        eb =>
                        {
                            eb.HasKey(a => a.Id);
                            eb.Metadata.RemoveAnnotation(RelationalAnnotationNames.TableName); // NEW LINE
                            eb.ToView("AccountBalance");
                            eb.HasOne(b => b.Account)
                                .WithMany(a => a.Balances)
                                .HasForeignKey(b => b.AccountId);
                        });

For some reason i thought the framework was thinking of this itself when calling ToView.