npgsql / efcore.pg

Entity Framework Core provider for PostgreSQL
PostgreSQL License
1.54k stars 225 forks source link

LINQ queries should respect System.Text.Json property names #2037

Closed olucafont6 closed 3 years ago

olucafont6 commented 3 years ago

Basically I'm trying to do something like this:

using Microsoft.EntityFrameworkCore;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using System.Text.Json.Serialization;

public class ColumnType {
   [JsonPropertyName("value-kebab-case")]
   public string Value { get; set; }
}

public class DatabaseRow {
   [Column("column1", TypeName = "jsonb")]
   public ColumnType Column1 { get; set; }
}

public class DatabaseContext : DbContext {
   public DbSet<DatabaseRow> Rows { get; set; }

   public DatabaseContext(DbContextOptions<DatabaseContext> contextOptions) : base(contextOptions) { }

   protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) {
      optionsBuilder.UseNpgsql("{CONNECTION_STRING}");
   }

   protected override void OnModelCreating(ModelBuilder modelBuilder) {
      modelBuilder.Entity<DatabaseRow>()
            .ToTable("table", "dbo");
   }

   public DatabaseRow FindMatchingEntry(string value) {
      return Rows.Where(entry => entry.Column1.Value == value)
         .FirstOrDefault();
   }

   public DatabaseRow FindMatchingEntryExplicitQuery(string value) {
      return Rows.FromSqlInterpolated($"SELECT * FROM dbo.table WHERE column1->>'value-kebab-case'={value}")
         .FirstOrDefault();
   }
}

(This is a genericized version of my actual application code, but I didn't actually run it, so some of the details might be slightly off.)

I expected FindMatchingEntry() to essentially generate a request like this:

SELECT b.column1
FROM dbo.table AS b
WHERE b.column1->>'value-kebab-case' = @__value_0

But instead it generates something like this:

SELECT b.column1
FROM dbo.table AS b
WHERE b.column1->>'Value' = @__value_0

It's weird because specifying the JSON property name works when loading data into the class I created ([JsonPropertyName("value-kebab-case")]), but it doesn't seem to have any effect when translating a LINQ expression to a SQL query.

I can manually specify the SQL query as a workaround (FindMatchingEntryExplicitQuery()), but it would be nice if the SQL method worked with custom JSON property names. Maybe I'm just missing something - I didn't see anything about it in the documentation though.

Let me know if there's any other information I can provide - thanks!

roji commented 3 years ago

This doesn't repro for me with 5.0.10. Using the code sample below, which is based on your snippets above, I get the following SQL query:

SELECT r."Id", r.column1
FROM "Rows" AS r
WHERE r.column1->>'value-kebab-case' = 'foo'
LIMIT 1

Which version are you using? Can you tweak the code sample to make it fail, or submit your own minimal code sample?

Attempted repro ```c# await using var ctx = new BlogContext(); await ctx.Database.EnsureDeletedAsync(); await ctx.Database.EnsureCreatedAsync(); _ = ctx.Rows.Where(r => r.Column1.Value == "foo").FirstOrDefault(); public class BlogContext : DbContext { public DbSet Rows { get; set; } protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder .UseNpgsql(@"Host=localhost;Username=test;Password=test") .LogTo(Console.WriteLine, LogLevel.Information) .EnableSensitiveDataLogging(); protected override void OnModelCreating(ModelBuilder modelBuilder) { } } public class ColumnType { [JsonPropertyName("value-kebab-case")] public string Value { get; set; } } public class DatabaseRow { public int Id { get; set; } [Column("column1", TypeName = "jsonb")] public ColumnType Column1 { get; set; } } ```
olucafont6 commented 3 years ago

@roji Oh, right. Yeah the project I'm using it from is only on .NET Core 3.1, so I was using 3.1.18. It's possible I could upgrade to .NET 5 if this works in the 5.x version of the library.

Can you try your reproducer with 3.1.18 and see if it still succeeds? If so, maybe I'm doing something wrong. I'll try creating a minimal reproducer also - not sure if I need an actual Postgres database for that.

olucafont6 commented 3 years ago

Here, I think this is a reproducer for 3.1.18 - not sure if the LogTo() function / extension method was available so I included some code I found online that will output the SQL query.

I get this output when I run it - let me know if it doesn't work for you.

SELECT r."Id", r.column1
FROM "Rows" AS r
WHERE r.column1->>'Value' = 'foo'
3.1.18 Reproducer ```c# using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Query; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using System.Reflection; using System.Text.Json.Serialization; using System.Threading.Tasks; namespace NpgsqlReproducer { class Program { static async Task Main(string[] args) { await using var ctx = new BlogContext(); var sqlStatement = ctx.Rows.Where(r => r.Column1.Value == "foo") .ToSql(); Console.WriteLine(sqlStatement); } } public class BlogContext : DbContext { public DbSet Rows { get; set; } protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder .UseNpgsql(@"Host=localhost;Username=test;Password=test") .EnableDetailedErrors() .EnableSensitiveDataLogging(); protected override void OnModelCreating(ModelBuilder modelBuilder) { } } public class ColumnType { [JsonPropertyName("value-kebab-case")] public string Value { get; set; } } public class DatabaseRow { public int Id { get; set; } [Column("column1", TypeName = "jsonb")] public ColumnType Column1 { get; set; } } public static class IQueryableExtensions { public static string ToSql(this IQueryable query) where TEntity : class { using var enumerator = query.Provider.Execute>(query.Expression).GetEnumerator(); var relationalCommandCache = enumerator.Private("_relationalCommandCache"); var selectExpression = relationalCommandCache.Private("_selectExpression"); var factory = relationalCommandCache.Private("_querySqlGeneratorFactory"); var sqlGenerator = factory.Create(); var command = sqlGenerator.GetCommand(selectExpression); string sql = command.CommandText; return sql; } private static object Private(this object obj, string privateField) => obj?.GetType().GetField(privateField, BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(obj); private static T Private(this object obj, string privateField) => (T)obj?.GetType().GetField(privateField, BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(obj); } } ```
roji commented 3 years ago

Duplicate of #1419

roji commented 3 years ago

Yeah, this was implemented for 5.0. This isn't something I can really backport to 3.1, especially since it may breaking existing users of 3.1 depending on that behavior - you'll have to update to 5.0 or 6.0 (an rc2 of the latter is out, with the final release in around a month).

olucafont6 commented 3 years ago

@roji Okay cool - yeah that's fine. I think I should be able to upgrade this project to .NET 5, and if not I can just use the explicit SQL query for now.

Is this feature (respecting JsonPropertyName when generating a query from LINQ) in the documentation anywhere (I couldn't find it). Seems like it could be helpful for future readers, and there could be a disclaimer about it only starting in version 5.x.

roji commented 3 years ago

Yeah, we could add a note on the JSON page - opened https://github.com/npgsql/doc/issues/131 to track.

Gamerwithprime commented 7 months ago

YEAH YOUR RIGHT ROJI