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

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

Closed
olucafont6 opened this issue Oct 13, 2021 · 8 comments
Closed

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

olucafont6 opened this issue Oct 13, 2021 · 8 comments

Comments

@olucafont6
Copy link

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
Copy link
Member

roji commented Oct 15, 2021

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
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<DatabaseRow> 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
Copy link
Author

@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
Copy link
Author

olucafont6 commented Oct 15, 2021

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
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<DatabaseRow> 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<TEntity>(this IQueryable<TEntity> query) where TEntity : class {
         using var enumerator = query.Provider.Execute<IEnumerable<TEntity>>(query.Expression).GetEnumerator();
         var relationalCommandCache = enumerator.Private("_relationalCommandCache");
         var selectExpression = relationalCommandCache.Private<SelectExpression>("_selectExpression");
         var factory = relationalCommandCache.Private<IQuerySqlGeneratorFactory>("_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<T>(this object obj, string privateField) => (T)obj?.GetType().GetField(privateField, BindingFlags.Instance | BindingFlags.NonPublic)?.GetValue(obj);
   }
}

@roji
Copy link
Member

roji commented Oct 15, 2021

Duplicate of #1419

@roji roji marked this as a duplicate of #1419 Oct 15, 2021
@roji
Copy link
Member

roji commented Oct 15, 2021

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).

@roji roji closed this as completed Oct 15, 2021
@olucafont6
Copy link
Author

@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
Copy link
Member

roji commented Oct 16, 2021

Yeah, we could add a note on the JSON page - opened npgsql/doc#131 to track.

@Gamerwithprime
Copy link

YEAH YOUR RIGHT ROJI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants