Changes from PR review

src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlHstoreTranslator.cs

@@ -1,7 +1,6 @@
 using System.Collections.Immutable;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
-using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
 using static Npgsql.EntityFrameworkCore.PostgreSQL.Utilities.Statics;
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.ExpressionTranslators.Internal;
@@ -30,14 +29,15 @@ public class NpgsqlHstoreTranslator : IMethodCallTranslator, IMemberTranslator
  ImmutableDictionaryType.GetMethod(nameof(ImmutableDictionary<string, string>.ContainsValue))!;
 
  private static readonly MethodInfo Dictionary_Item_Getter =
- DictionaryType.GetProperty("Item")!.GetMethod!;
+ DictionaryType.FindIndexerProperty()!.GetMethod!;
 
  private static readonly MethodInfo ImmutableDictionary_Item_Getter =
- ImmutableDictionaryType.GetProperty("Item")!.GetMethod!;
+ ImmutableDictionaryType.FindIndexerProperty()!.GetMethod!;
 
  private static readonly MethodInfo Enumerable_Any =
- typeof(Enumerable).GetMethod(nameof(Enumerable.Any),
- BindingFlags.Public | BindingFlags.Static, new[] { typeof(IEnumerable<>).MakeGenericType(Type.MakeGenericMethodParameter(0)) })!
+ typeof(Enumerable).GetMethod(
+ nameof(Enumerable.Any), BindingFlags.Public | BindingFlags.Static,
+ [typeof(IEnumerable<>).MakeGenericType(Type.MakeGenericMethodParameter(0))])!
  .MakeGenericMethod(typeof(KeyValuePair<string, string>));
 
  private static readonly PropertyInfo Dictionary_Count = DictionaryType.GetProperty(nameof(Dictionary<string, string>.Count))!;
@@ -48,8 +48,20 @@ public class NpgsqlHstoreTranslator : IMethodCallTranslator, IMemberTranslator
  private static readonly PropertyInfo ImmutableDictionary_IsEmpty =
  ImmutableDictionaryType.GetProperty(nameof(ImmutableDictionary<string, string>.IsEmpty))!;
 
+ private static readonly PropertyInfo Dictionary_Keys = DictionaryType.GetProperty(nameof(Dictionary<string, string>.Keys))!;
+
+ private static readonly PropertyInfo ImmutableDictionary_Keys =
+ ImmutableDictionaryType.GetProperty(nameof(ImmutableDictionary<string, string>.Keys))!;
+
+ private static readonly PropertyInfo Dictionary_Values = DictionaryType.GetProperty(nameof(Dictionary<string, string>.Values))!;
+
+ private static readonly PropertyInfo ImmutableDictionary_Values =
+ ImmutableDictionaryType.GetProperty(nameof(ImmutableDictionary<string, string>.Values))!;
+
  private readonly RelationalTypeMapping _stringListTypeMapping;
+ private readonly RelationalTypeMapping _stringTypeMapping;
  private readonly NpgsqlSqlExpressionFactory _sqlExpressionFactory;
+ private readonly RelationalTypeMapping _dictionaryKeyCollectionMapping;
 
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -61,6 +73,8 @@ public NpgsqlHstoreTranslator(IRelationalTypeMappingSource typeMappingSource, Np
  {
  _sqlExpressionFactory = sqlExpressionFactory;
  _stringListTypeMapping = typeMappingSource.FindMapping(typeof(List<string>))!;
+ _stringTypeMapping = typeMappingSource.FindMapping(typeof(string))!;
+ _dictionaryKeyCollectionMapping = typeMappingSource.FindMapping(typeof(Dictionary<string, string>.KeyCollection))!;
  }
 
  /// <summary>
@@ -75,19 +89,14 @@ public NpgsqlHstoreTranslator(IRelationalTypeMappingSource typeMappingSource, Np
  IReadOnlyList<SqlExpression> arguments,
  IDiagnosticsLogger<DbLoggerCategory.Query> logger)
  {
- if (method == Enumerable_Any)
+ if (method == Enumerable_Any && arguments[0].TypeMapping?.StoreType == "hstore")
  {
- var value = instance ?? arguments[0];
- if (value.TypeMapping?.StoreType == NpgsqlHstoreTypeMapping.HstoreType)
- {
- return _sqlExpressionFactory.NotEqual(
- Translate(value, Dictionary_Count, typeof(int), logger)!,
- _sqlExpressionFactory.Constant(0));
- }
- return null;
+ return _sqlExpressionFactory.NotEqual(
+ Translate(arguments[0], Dictionary_Count, typeof(int), logger)!,
+ _sqlExpressionFactory.Constant(0));
  }
 
- if (instance?.TypeMapping is null || instance.TypeMapping.StoreType != NpgsqlHstoreTypeMapping.HstoreType)
+ if (instance?.TypeMapping?.StoreType != "hstore")
  {
  return null;
  }
@@ -101,15 +110,15 @@ public NpgsqlHstoreTranslator(IRelationalTypeMappingSource typeMappingSource, Np
  {
  return _sqlExpressionFactory.Any(
  arguments[0],
- _sqlExpressionFactory.Function(
- "avals", new[] { instance }, false, FalseArrays[1], typeof(List<string>), _stringListTypeMapping),
+ Translate(instance, Dictionary_Values, typeof(List<string>), logger)!,
  PgAnyOperatorType.Equal);
  }
 
  if (method == Dictionary_Item_Getter || method == ImmutableDictionary_Item_Getter)
  {
- return _sqlExpressionFactory.MakePostgresBinary(PgExpressionType.HStoreValueForKey, instance, arguments[0]);
+ return _sqlExpressionFactory.MakePostgresBinary(PgExpressionType.HStoreValueForKey, instance, arguments[0], _stringTypeMapping);
  }
+
  return null;
  }
 
@@ -126,19 +135,30 @@ public NpgsqlHstoreTranslator(IRelationalTypeMappingSource typeMappingSource, Np
  IDiagnosticsLogger<DbLoggerCategory.Query> logger)
  {
 
- if (instance?.TypeMapping is null || instance.TypeMapping.StoreType != NpgsqlHstoreTypeMapping.HstoreType)
+ if (instance?.TypeMapping?.StoreType != "hstore")
  {
  return null;
  }
 
  if (member == Dictionary_Count || member == ImmutableDictionary_Count)
  {
- return _sqlExpressionFactory.Function("array_length", new []
- {
- _sqlExpressionFactory.Function(
- "akeys", new[] { instance }, false, FalseArrays[1], typeof(List<string>), _stringListTypeMapping),
+ return _sqlExpressionFactory.Function("array_length",
+ [
+ Translate(instance, Dictionary_Keys, null!, logger)!,
  _sqlExpressionFactory.Constant(1)
- }, false, FalseArrays[2], typeof(int));
+ ], false, TrueArrays[2], typeof(int));
+ }
+
+ if (member == Dictionary_Keys || member == ImmutableDictionary_Keys)
+ {
+ return _sqlExpressionFactory.Function(
+ "akeys", [instance], true, TrueArrays[1], typeof(List<string>), _stringListTypeMapping);
+ }
+
+ if (member == Dictionary_Values || member == ImmutableDictionary_Values)
+ {
+ return _sqlExpressionFactory.Function(
+ "avals", [instance], true, TrueArrays[1], typeof(List<string>), _stringListTypeMapping);
  }
 
  if (member == ImmutableDictionary_IsEmpty)
@@ -147,6 +167,7 @@ public NpgsqlHstoreTranslator(IRelationalTypeMappingSource typeMappingSource, Np
  Translate(instance, Dictionary_Count, typeof(int), logger)!,
  _sqlExpressionFactory.Constant(0));
  }
+
  return null;
  }
 }

src/EFCore.PG/Query/NpgsqlSqlExpressionFactory.cs

@@ -316,10 +316,6 @@ public virtual SqlExpression MakePostgresBinary(
  case PgExpressionType.Distance:
  returnType = typeof(double);
  break;
-
- case PgExpressionType.HStoreValueForKey:
- returnType = typeof(string);
- break;
  }
 
  return (PgBinaryExpression)ApplyTypeMapping(

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlHstoreTypeMapping.cs

@@ -5,7 +5,7 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
 
 /// <summary>
 /// The type mapping for the PostgreSQL hstore type. Supports both <see cref="Dictionary{TKey,TValue} " />
-/// and <see cref="ImmutableDictionary{TKey,TValue}" /> over strings.
+/// and <see cref="ImmutableDictionary{TKey,TValue}" /> where TKey and TValue are both strings.
 /// </summary>
 /// <remarks>
 /// See: https://www.postgresql.org/docs/current/static/hstore.html
@@ -14,11 +14,6 @@ public class NpgsqlHstoreTypeMapping : NpgsqlTypeMapping
 {
  private static readonly HstoreMutableComparer MutableComparerInstance = new();
 
- /// <summary>
- /// The database store type of the Hstore type
- /// </summary>
- public const string HstoreType = "hstore";
-
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
  /// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -37,7 +32,7 @@ public NpgsqlHstoreTypeMapping(Type clrType)
  : base(
  new RelationalTypeMappingParameters(
  new CoreTypeMappingParameters(clrType, comparer: GetComparer(clrType)),
- HstoreType),
+ "hstore"),
  NpgsqlDbType.Hstore)
  {
  }