From cb48b639d82a8f918c3f8a3b849a6e04f1eed63f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Mon, 26 Oct 2026 18:10:27 +0100 Subject: [PATCH 1/3] fix: reject a string set compared with a value under a collation that may not compare bytes (#507) PostgreSQL derives an operator's collation from both sides, and #483 only checks the set's: a value from a case-insensitive column, or collated by EF.Functions.Collate, made @>, && and array_remove compare case-insensitively with the set column uncollated, as the remedy leaves it. Members.Contains(g.Owner) matched {A,b} for "a", and ExecuteUpdate with Members.Remove(g.Owner) stored {b}, where memory keeps {A,b}. The guard's walker now derives the collation of a string set operator's operands as PostgreSQL does (an explicit COLLATE wins; a column's; a subquery or set-operation column's projection; what a string-typed function, operator or array takes from its string inputs; a CASE's results, not its tests) at @>, <@, &&, array_remove, == and the text set placeholders, and throws for one that may not compare bytes, naming it. EF.Functions.Collate(value, "C") compares as memory does. The read-optimized model a query runs on keeps no collation, so the convention that rejects a collated string set column names every other non-byte collation on its property (CodoMetis:ValueRanges:Collation), which a compiled model carries. StringSetCollationIntegrationTests rejects Contains, !Contains, Remove, Add().Count, ToLower(), EF.Functions.Collate, a subquery's column and ExecuteUpdate with Remove on a live server and leaves the row unchanged, runs "C" and a CASE that only tests the column as memory answers, and holds the rejection on a compiled model; StringSetCollationTests adds a derived table's and a set operation's column, a concatenation and a CASE result. Without the walker check every rejection runs and answers. Found-By: CA Defect-Class: SQL Co-Authored-By: Claude Opus 5.5 --- .../Infrastructure/StringSetCollation.cs | 20 +++- .../Query/ValueSetSqlGuard.cs | 107 +++++++++++++++++ .../CompiledModelJsonTests.cs | 19 ++- .../StringSetCollationIntegrationTests.cs | 108 ++++++++++++++++++ .../StringSetCollationTests.cs | 39 +++++++ 5 files changed, 286 insertions(+), 7 deletions(-) diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs index d0267bd1..5467b196 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs @@ -35,6 +35,15 @@ internal static class StringSetCollation public static bool IsStringSet(Type type) => type == typeof(StringSet) || type is { IsGenericType: true } && type.GetGenericTypeDefinition() == typeof(StringSet<>); + /// + /// The annotation on a property whose collation may not compare bytes, naming it, for the query guard (#507). The + /// read-optimized model a query runs on drops collations, which only migrations need. + /// + public const string ElementAnnotation = "CodoMetis:ValueRanges:Collation"; + + /// The collation names on a property, or . + public static string? OfElement(IReadOnlyProperty property) => property.FindAnnotation(ElementAnnotation)?.Value as string; + public static string Remedy => "Remove the collation, or use \"C\" (docs/efcore.md#store-a-value-set-in-an-array-column)."; } @@ -70,9 +79,16 @@ private static void Check(IConventionTypeBase type) foreach (var property in type.GetDeclaredProperties()) { - if (!StringSetCollation.IsStringSet(property.ClrType) || property.GetCollation() is not { } collation - || StringSetCollation.ComparesBytes(collation)) + if (property.GetCollation() is not { } collation || StringSetCollation.ComparesBytes(collation)) + continue; + + // A value from this column compared with a string set compares under its collation too, and the query + // guard has only the read-optimized model, which keeps no collation (#507). + if (!StringSetCollation.IsStringSet(property.ClrType)) + { + property.Builder.HasAnnotation(StringSetCollation.ElementAnnotation, collation); continue; + } throw new InvalidOperationException( $"'{type.DisplayName()}.{property.Name}' is a {property.ClrType.ShortDisplayName()} column with the collation " diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs index c6ea1aef..a148b455 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs @@ -164,6 +164,9 @@ private sealed class Walker /// The columns found to project a derived value, for . public IReadOnlySet<(string Table, string Column)> DerivedColumns => _derivedColumns; + /// Subquery and set-operation columns that project a collated string, by table alias and column name. + private readonly Dictionary<(string Table, string Column), (string Name, bool Explicit)> _collatedColumns = []; + public void Root(Expression expression) { switch (expression) @@ -223,6 +226,7 @@ private void Table(TableExpressionBase table) { case SelectExpression subquery: Register(subquery.Alias, subquery, Select(subquery)); + RegisterCollations(subquery.Alias, subquery, subquery); break; case SetOperationBase setOperation: @@ -230,6 +234,7 @@ private void Table(TableExpressionBase table) if (setOperation.IsDistinct && either.Contains(true)) throw Refusal.For(Use.SetOperation); Register(setOperation.Alias, setOperation.Source1, either); + RegisterCollations(setOperation.Alias, setOperation.Source1, setOperation.Source2); break; case PredicateJoinExpressionBase join: @@ -259,6 +264,14 @@ private void Register(string? alias, SelectExpression source, bool[] derived) if (derived[i]) _derivedColumns.Add((alias, source.Projection[i].Alias)); } + private void RegisterCollations(string? alias, SelectExpression first, SelectExpression second) + { + if (alias is null) return; + for (var i = 0; i < first.Projection.Count; i++) + if (Collation([first.Projection[i].Expression, second.Projection[i].Expression]) is { } collation) + _collatedColumns[(alias, first.Projection[i].Alias)] = collation; + } + /// Refuses a derived value in a position that does not tolerate one. private void Consume(Expression? expression, Use use = Use.Other) { @@ -299,11 +312,13 @@ private bool Derived(SqlExpression? expression) // A text Union, Add, Intersect or Except: unsorted, and a Union or Add may repeat an element. // Its operands need not be sorted either, since the result isn't. case SqlFunctionExpression { Name: var unguarded, Arguments: { } arguments } when IsUnguarded(unguarded) && IsSet(expression): + Compared([.. arguments]); foreach (var argument in arguments) Derived(argument); return true; // array_remove preserves canonical form rather than establishing it. case SqlFunctionExpression { Name: "array_remove", Arguments: [var input, var element] } when IsSet(expression): + Compared(input, element); Consume(element); return Derived(input); @@ -355,6 +370,7 @@ when CardinalityComparedWithZero(comparison) is { } counted: // == and != over a derived set: the expander writes them as mutual containment (#485). case SqlBinaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } equality: + Compared(equality.Left, equality.Right); Derived(equality.Left); Derived(equality.Right); return false; @@ -367,6 +383,7 @@ when CardinalityComparedWithZero(comparison) is { } counted: // @>, <@ and && ignore order and multiplicity: the reason array_cat is a valid // translation of Union at all. case PgBinaryExpression { OperatorType: PgExpressionType.Contains or PgExpressionType.ContainedBy or PgExpressionType.Overlaps } containment: + Compared(containment.Left, containment.Right); Derived(containment.Left); Derived(containment.Right); return false; @@ -390,6 +407,89 @@ when CardinalityComparedWithZero(comparison) is { } counted: } } + /// + /// Refuses a string set operator whose comparison PostgreSQL would make under a collation that may not compare + /// bytes (#507). It derives the operator's collation from both sides, so a value from a case-insensitive column, + /// or collated by EF.Functions.Collate, makes @>, && and array_remove + /// compare case-insensitively however the set column is collated (#483 rejects that one when the model is built). + /// + /// + /// An explicit COLLATE wins over the implicit collations of columns, as PostgreSQL's rule is, so + /// EF.Functions.Collate(value, "C") is the way to compare such a value as memory does. + /// + private void Compared(params SqlExpression?[] operands) + { + if (!operands.Any(operand => operand is not null && StringSetCollation.IsStringSet(operand.Type))) return; + + // EF.Functions.Collate over the set itself keeps its own message (#483). + foreach (var operand in operands) + if (operand is CollateExpression { Operand: var collated, Collation: var own } + && StringSetCollation.IsStringSet(collated.Type) && !StringSetCollation.ComparesBytes(own)) + throw Refusal.Collation(own); + + if (Collation(operands) is { Name: var name } && !StringSetCollation.ComparesBytes(name)) + throw Refusal.ElementCollation(name); + } + + /// + /// The collation PostgreSQL derives for the operands of one operator: an explicit one if any operand has one, + /// otherwise an implicit one, one that may not compare bytes first. for the default. + /// + private (string Name, bool Explicit)? Collation(IEnumerable operands) + { + (string Name, bool Explicit)? result = null; + foreach (var operand in operands) + { + if (Collation(operand) is not { } found) continue; + if (result is null + || found.Explicit && !result.Value.Explicit + || found.Explicit == result.Value.Explicit && StringSetCollation.ComparesBytes(result.Value.Name) && !StringSetCollation.ComparesBytes(found.Name)) + result = found; + } + + return result; + } + + /// + /// The collation an expression carries into a comparison: a COLLATE, a mapped column's, a subquery + /// column's, or what a string-typed function, operator or array takes from its string-typed inputs. A + /// CASE takes it from its results only, and a non-string expression has none. + /// + private (string Name, bool Explicit)? Collation(SqlExpression? expression) => expression switch + { + null => null, + CollateExpression collate => (collate.Collation, true), + ColumnExpression column => ColumnCollation(column), + CaseExpression conditional => Collation([.. conditional.WhenClauses.Select(clause => clause.Result), conditional.ElseResult]), + ScalarSubqueryExpression { Subquery.Projection: [var only] } => Collation(only.Expression), + PgNewArrayExpression array => Collation(array.Expressions), + _ when !Collatable(expression) => null, + SqlFunctionExpression { Arguments: { } arguments } => Collation(arguments.Where(Collatable)), + SqlBinaryExpression binary => Collation(new[] { binary.Left, binary.Right }.Where(Collatable)), + SqlUnaryExpression { OperatorType: ExpressionType.Convert } convert when Collatable(convert.Operand) => Collation(convert.Operand), + _ => null + }; + + private (string Name, bool Explicit)? ColumnCollation(ColumnExpression column) + { + // The read-optimized model keeps no collation; the convention that rejects a collated string set column + // names the others on their properties. + var mapped = column.Column?.PropertyMappings.Select(mapping => StringSetCollation.OfElement(mapping.Property)) + .FirstOrDefault(name => name is not null); + + return mapped is not null ? (mapped, false) + : _collatedColumns.TryGetValue((column.TableAlias, column.Name), out var derived) ? derived + : null; + } + + /// Text, or a set or array of it: what carries a collation. + private static bool Collatable(SqlExpression? expression) + => expression is not null + && (expression.Type == typeof(string) || expression.Type == typeof(string[]) || StringSetCollation.IsStringSet(expression.Type) + || expression.TypeMapping?.StoreType is { } store + && (store.StartsWith("text", StringComparison.Ordinal) || store.StartsWith("character", StringComparison.Ordinal) + || store.StartsWith("varchar", StringComparison.Ordinal))); + /// /// Refuses to_jsonb(set) (or to_json) inside an UPDATE setter, naming the /// JSON path from the enclosing jsonb_set where there is one. @@ -789,6 +889,13 @@ private static InvalidOperationException Create(string subject, string operation + "compares its elements ordinally in memory, and under a case- or accent-insensitive collation Contains, the " + "subset operators, == and Remove would answer differently in the database. Use \"C\", or compare in memory."); + public static InvalidOperationException ElementCollation(string collation) => new( + $"A string set cannot be compared with a value under the collation '{collation}', which the value takes from its " + + "column or from EF.Functions.Collate. PostgreSQL compares under the collation of either side, so under a case- " + + "or accent-insensitive one Contains, the subset operators, == and Remove would answer differently from memory, " + + "which compares ordinally, and Remove in ExecuteUpdate would delete elements memory keeps. Collate the value " + + "with EF.Functions.Collate(value, \"C\"), or compare in memory."); + public static InvalidOperationException NullJsonSetter(string column, string? path) => new( $"'ExecuteUpdate' cannot set the value set '{column}{(path is null ? "" : "." + path)}' inside a JSON column " + "to null. EF Core writes it as to_jsonb(NULL), and PostgreSQL cannot type the untyped NULL (42804). " diff --git a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/CompiledModelJsonTests.cs b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/CompiledModelJsonTests.cs index 4bbe7bd8..bc6a89e8 100644 --- a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/CompiledModelJsonTests.cs +++ b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/CompiledModelJsonTests.cs @@ -269,14 +269,23 @@ private static IReadOnlyList SetValues() private static IModel CompileModel(string connectionString) { using var context = new CompiledJsonContext(connectionString); - var assembly = typeof(CompiledJsonContext).Assembly; + return CompileModel(context, "CodoMetis.ValueRanges.CompiledJson"); + } + + /// + /// The compiled model dotnet ef dbcontext optimize writes for , compiled and loaded. + /// + internal static IModel CompileModel(DbContext context, string modelNamespace) + { + var contextType = context.GetType(); + var assembly = contextType.Assembly; var services = new DesignTimeServicesBuilder(assembly, assembly, new OperationReporter(handler: null), []) .Build(context); var options = new CompiledModelCodeGenerationOptions { - ContextType = typeof(CompiledJsonContext), - ModelNamespace = "CodoMetis.ValueRanges.CompiledJson", + ContextType = contextType, + ModelNamespace = modelNamespace, Language = "C#", UseNullableReferenceTypes = true }; @@ -285,7 +294,7 @@ private static IModel CompileModel(string connectionString) .GenerateModel(context.GetService().Model, options); var compilation = CSharpCompilation.Create( - $"CompiledJsonModel_{Guid.NewGuid():N}", + $"CompiledModel_{Guid.NewGuid():N}", files.Select(file => CSharpSyntaxTree.ParseText(file.Code, path: file.Path)), AppDomain.CurrentDomain.GetAssemblies() .Where(loaded => !loaded.IsDynamic && loaded.Location.Length > 0) @@ -302,7 +311,7 @@ private static IModel CompileModel(string connectionString) image.Position = 0; var model = AssemblyLoadContext.Default.LoadFromStream(image) .GetTypes() - .Single(type => type.Name == $"{nameof(CompiledJsonContext)}Model") + .Single(type => type.Name == $"{contextType.Name}Model") .GetProperty("Instance", BindingFlags.Public | BindingFlags.Static)! .GetValue(null); return (IModel)model!; diff --git a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/StringSetCollationIntegrationTests.cs b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/StringSetCollationIntegrationTests.cs index 10205400..7d8d80ed 100644 --- a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/StringSetCollationIntegrationTests.cs +++ b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests/StringSetCollationIntegrationTests.cs @@ -1,5 +1,6 @@ using CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests.Composition; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata; using Npgsql; namespace CodoMetis.ValueRanges.EFCore.PostgreSQL.IntegrationTests; @@ -44,6 +45,113 @@ public async Task AStringSetColumnCollatedC_ComparesElementsAsMemoryDoes() NpgsqlConnection.ClearPool(connection); } + /// + /// PostgreSQL takes an operator's collation from both sides, so an element from a case-insensitive column, or + /// collated by EF.Functions.Collate, made @> and array_remove compare case-insensitively with + /// the set column uncollated, as #483's fix leaves it: Contains(owner) was true for {A,b}, and + /// ExecuteUpdate with Remove(owner) stored {b} (#507). Each throws now, naming the collation. + /// + [TestMethod] + public async Task AnElementWithACaseInsensitiveCollation_IsRejected_WhereItWouldCompareAsTheCollationDoes() + { + ContainerLifecycle.RequireServer(); + var connectionString = await CompositionDatabase.Create("collation_element"); + var stored = StringSet.From("A", "b"); + + await using (var seed = new MembersContext(connectionString)) + { + await seed.Database.EnsureCreatedAsync(); + seed.Groups.Add(new MemberGroup { Id = 1, Members = stored, Owner = "a", Login = "a" }); + await seed.SaveChangesAsync(); + } + + await using (var context = new MembersContext(connectionString)) + { + async Task Rejected(Func query, string what) + { + var e = await Assert.ThrowsExactlyAsync(query, what); + StringAssert.Contains(e.Message, "'case_insensitive'", what); + } + + await Rejected(() => context.Groups.AnyAsync(g => g.Members.Contains(g.Owner)), "Contains(Owner)"); + await Rejected(() => context.Groups.AnyAsync(g => !g.Members.Contains(g.Owner)), "!Contains(Owner)"); + await Rejected(() => context.Groups.Select(g => g.Members.Remove(g.Owner)).SingleAsync(), "Remove(Owner)"); + await Rejected(() => context.Groups.Select(g => g.Members.Add(g.Owner).Count).SingleAsync(), "Add(Owner).Count"); + await Rejected(() => context.Groups.AnyAsync(g => g.Members.Contains(g.Owner.ToLower())), "Contains(Owner.ToLower())"); + await Rejected(() => context.Groups.AnyAsync(g => g.Members.Contains(EF.Functions.Collate("a", "case_insensitive"))), + "Contains(Collate(\"a\"))"); + await Rejected(() => context.Groups.AnyAsync(g => g.Members.Contains(context.Groups.Select(o => o.Owner).First())), + "Contains(a subquery's Owner)"); + await Rejected(() => context.Groups.ExecuteUpdateAsync(s => s.SetProperty(g => g.Members, g => g.Members.Remove(g.Owner))), + "ExecuteUpdate with Remove(Owner)"); + + // A byte-order collation on the element's column, or one EF.Functions.Collate puts on it, which wins over + // the column's: both answer as memory does. So does a CASE that only tests a case-insensitive column. + Assert.AreEqual(stored.Contains("a"), await context.Groups.AnyAsync(g => g.Members.Contains(g.Login)), "Contains(Login)"); + Assert.AreEqual(stored.Contains("a"), await context.Groups.AnyAsync(g => g.Members.Contains(EF.Functions.Collate(g.Owner, "C"))), + "Contains(Collate(Owner, \"C\"))"); + Assert.AreEqual(stored.Contains("a"), await context.Groups.AnyAsync(g => g.Members.Contains(g.Owner.Length > 0 ? "a" : "b")), + "Contains(a CASE whose test reads Owner)"); + } + + await using (var reread = new MembersContext(connectionString)) + Assert.AreEqual(stored, (await reread.Groups.SingleAsync()).Members, "ExecuteUpdate was rejected, so the row is unchanged"); + + await using var connection = new NpgsqlConnection(connectionString); + NpgsqlConnection.ClearPool(connection); + } + + /// + /// The read-optimized model a query runs on keeps no collation, so the convention names a collated column's on its + /// property; a compiled model carries that annotation too, or the guard would see no collation at all. + /// + [TestMethod] + public void AnElementWithACaseInsensitiveCollation_IsRejected_OnACompiledModelToo() + { + const string connectionString = "Host=localhost;Database=collation_compiled;Username=postgres"; + IModel compiled; + using (var design = new MembersContext(connectionString)) + compiled = CompiledModels.CompiledModelJsonTests.CompileModel(design, "CodoMetis.ValueRanges.CompiledCollation"); + + using var context = new MembersContext(connectionString, compiled); + var e = Assert.ThrowsExactly(() => context.Groups.Where(g => g.Members.Contains(g.Owner)).ToQueryString()); + StringAssert.Contains(e.Message, "'case_insensitive'"); + StringAssert.Contains(context.Groups.Where(g => g.Members.Contains(g.Login)).ToQueryString(), "@> ARRAY[g.\"Login\"]"); + } + + public sealed class MemberGroup + { + public int Id { get; set; } + + public StringSet Members { get; set; } = StringSet.Empty; + + /// Case-insensitive. + public string Owner { get; set; } = ""; + + /// Byte order. + public string Login { get; set; } = ""; + } + + /// Public, so that a compiled model of it compiles. + public sealed class MembersContext(string connectionString, IModel? model = null) : DbContext + { + public DbSet Groups => Set(); + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + CompositionDatabase.Configure(optionsBuilder, connectionString); + if (model is not null) optionsBuilder.UseModel(model); + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.HasCollation("case_insensitive", locale: "und-u-ks-level2", provider: "icu", deterministic: false); + modelBuilder.Entity().Property(g => g.Id).ValueGeneratedNever(); + modelBuilder.Entity().Property(g => g.Owner).UseCollation("case_insensitive"); + modelBuilder.Entity().Property(g => g.Login).UseCollation("C"); + } + } + public sealed class CollatedPost { public int Id { get; set; } diff --git a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/StringSetCollationTests.cs b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/StringSetCollationTests.cs index 7ee34ad1..7c6307d2 100644 --- a/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/StringSetCollationTests.cs +++ b/test/CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests/StringSetCollationTests.cs @@ -81,12 +81,51 @@ public void EfFunctionsCollate_OverAStringSet_IsRefusedUnlessItComparesBytes() "(p.\"Tags\" COLLATE \"C\") @> ARRAY['a']::text[]"); } + /// + /// A value compared with a string set brings its collation to the comparison, since PostgreSQL derives the + /// operator's from both sides (#507). Through a derived table's column, a set operation's and a scalar subquery + /// the query is rejected as for the column itself; a byte-order collation, an explicit "C" over the column, and a + /// CASE that only tests the column run. + /// + [TestMethod] + public void AnElementWithACollationThatMayNotCompareBytes_IsRejected_WhereverItComesFrom() + { + using var context = new CollationContext(model => model.Entity().Property(p => p.Owner).UseCollation("case_insensitive")); + + void Rejected(Func query, string what) + { + var e = Assert.ThrowsExactly(() => query().ToQueryString(), what); + StringAssert.Contains(e.Message, "with a value under the collation 'case_insensitive'", what); + } + + Rejected(() => context.Posts.Where(p => p.Tags.Contains(p.Owner)), "a column"); + Rejected(() => context.Posts.Where(p => p.Tags.Contains(p.Owner + "!")), "a concatenation"); + Rejected(() => context.Posts.Where(p => p.Tags.Contains(p.Id > 0 ? p.Owner : "a")), "a CASE result"); + Rejected(() => context.Posts.Select(p => new { p.Tags, Name = p.Owner }).OrderBy(x => x.Name).Take(5).Where(x => x.Tags.Contains(x.Name)), + "a derived table's column"); + Rejected(() => context.Posts.Select(p => new { p.Tags, Name = p.Owner }) + .Concat(context.Posts.Select(p => new { p.Tags, Name = "x" })) + .Where(x => x.Tags.Contains(x.Name)), "a set operation's column"); + Rejected(() => context.Posts.Select(p => p.Tags.Remove(p.Owner)), "Remove in a projection"); + Rejected(() => context.Posts.Select(p => p.Tags.Add(p.Owner).Count), "Add, counted"); + Rejected(() => context.Posts.Where(p => p.Tags.Contains(EF.Functions.Collate("a", "case_insensitive"))), "EF.Functions.Collate"); + + StringAssert.Contains(context.Posts.Where(p => p.Tags.Contains(EF.Functions.Collate(p.Owner, "C"))).ToQueryString(), + "@> ARRAY[p.\"Owner\" COLLATE \"C\"]"); + StringAssert.Contains(context.Posts.Where(p => p.Tags.Contains(p.Owner.Length > 0 ? "a" : "b")).ToQueryString(), "@> ARRAY[CASE"); + StringAssert.Contains(context.Posts.Where(p => p.Tags.Contains(p.Plain)).ToQueryString(), "@> ARRAY[p.\"Plain\"]"); + } + public sealed class Post { public int Id { get; set; } public StringSet Tags { get; set; } = StringSet.Empty; + public string Owner { get; set; } = ""; + + public string Plain { get; set; } = ""; + public StringSet Keys { get; set; } = StringSet.Empty; public PostDoc Doc { get; set; } = new(); From 4b4f08b88d1da3c637f535fb8bef01076bdfc646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Mon, 26 Oct 2026 18:11:19 +0100 Subject: [PATCH 2/3] test: register StringSetCollationTests and seed the element-side collation check (#507) Two seeds, both caught: the guard's element check removed, and the convention leaving a collated column unnamed, which the read-optimized model would then hide from the guard. Co-Authored-By: Claude Opus 5.5 --- test/oracles/oracles.json | 6 ++++++ ...ring-set-collation-annotation-missing.patch | 17 +++++++++++++++++ ...ing-set-collation-ignores-the-element.patch | 18 ++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 test/oracles/seeds/string-set-collation-annotation-missing.patch create mode 100644 test/oracles/seeds/string-set-collation-ignores-the-element.patch diff --git a/test/oracles/oracles.json b/test/oracles/oracles.json index 4a05cff0..dbfe7874 100644 --- a/test/oracles/oracles.json +++ b/test/oracles/oracles.json @@ -508,5 +508,11 @@ "needs": [ "docker" ] + }, + { + "class": "StringSetCollationTests", + "project": "CodoMetis.ValueRanges.EFCore.PostgreSQL.Tests", + "kind": "exception", + "against": "the documented rejections of a collation that may not compare bytes on a string set comparison: on the set column when the model is built, EF.Functions.Collate over the set, and a value compared with the set under its column's collation or EF.Functions.Collate, directly, through a derived table's or a set operation's column, a concatenation or a CASE result, on the runtime model; a byte-order collation, an explicit \"C\" and a CASE that only tests the column translate" } ] diff --git a/test/oracles/seeds/string-set-collation-annotation-missing.patch b/test/oracles/seeds/string-set-collation-annotation-missing.patch new file mode 100644 index 00000000..2a013a29 --- /dev/null +++ b/test/oracles/seeds/string-set-collation-annotation-missing.patch @@ -0,0 +1,17 @@ +Seed: the convention leaves a collated column unnamed, and the read-optimized model keeps no collation, so the guard sees none on the column +Oracles: StringSetCollationTests +Fails: Contains, Remove and Add over a value from a case_insensitive column, directly or through a derived table, a set operation, a concatenation or a CASE result; EF.Functions.Collate is still rejected +History: never shipped; the annotation in #507's fix + +diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs +index 5467b19..54752fa 100644 +--- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs ++++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Infrastructure/StringSetCollation.cs +@@ -86,7 +86,6 @@ internal sealed class StringSetCollationConvention : IModelFinalizingConvention + // guard has only the read-optimized model, which keeps no collation (#507). + if (!StringSetCollation.IsStringSet(property.ClrType)) + { +- property.Builder.HasAnnotation(StringSetCollation.ElementAnnotation, collation); + continue; + } + diff --git a/test/oracles/seeds/string-set-collation-ignores-the-element.patch b/test/oracles/seeds/string-set-collation-ignores-the-element.patch new file mode 100644 index 00000000..9b105be6 --- /dev/null +++ b/test/oracles/seeds/string-set-collation-ignores-the-element.patch @@ -0,0 +1,18 @@ +Seed: the guard checks only the string set's own collation, so a value collated case-insensitively makes @> and array_remove compare by it +Oracles: StringSetCollationTests +Fails: Contains, Remove and Add over a value from a case_insensitive column, a concatenation, a CASE result, a derived table's or set operation's column, or EF.Functions.Collate: translated where it must throw +History: shipped until 10.4.0 (#483 checked the set side only); fixed by #507 + +diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs +index a148b45..6ca3fd3 100644 +--- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs ++++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueSetSqlGuard.cs +@@ -427,8 +427,6 @@ internal static class ValueSetSqlGuard + && StringSetCollation.IsStringSet(collated.Type) && !StringSetCollation.ComparesBytes(own)) + throw Refusal.Collation(own); + +- if (Collation(operands) is { Name: var name } && !StringSetCollation.ComparesBytes(name)) +- throw Refusal.ElementCollation(name); + } + + /// From 6e4be223467e062ad2832d8f7ad51ccb90ebe05f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Mon, 26 Oct 2026 18:12:33 +0100 Subject: [PATCH 3/3] docs: say a string set compared with a collated value throws, with the migration and changelog (#507) The value set entry in efcore.md gains the value's side and the EF.Functions.Collate(value, "C") remedy; the v10.x migration list and Check string set columns with a collation gain it, with its before and after table and the GIN index cost of an explicit "C" (measured on PostgreSQL 16 over an en_US database: a sequential scan); the Breaking lines of 11.0.0-preview.1 in the root and EF Core changelogs; claim E89. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + docs/efcore.md | 6 ++++ docs/migration.md | 31 ++++++++++++++++--- .../CHANGELOG.md | 1 + test/oracles/claims.json | 16 ++++++++++ 5 files changed, 51 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce7c034d..b5ce5f26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ The first preview of the 11 line: the same public API as 10.0.0, on .NET 11. Not - EF Core: a value set `Union` with a NULL set, or `Remove` with a NULL set or element, is NULL in a query, as `Where(… == null)` already said. A `Select` loaded the other set instead, for a nullable column, a missing outer-join child or a `null` parameter ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-value-set-union-and-remove-over-null-in-queries)). - EF Core: any `Compare(x, y)` over ranges or sets, `RangeLowerBoundComparer.Compare` and every other `IComparer` included, runs in memory only in a query, as the method called. A filter or an ordering fails translation. In SQL it compared in the range order, so `Compare(r.Period, p) == 0` missed rows with the same lower bound ([#464](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/464), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#move-rangelowerboundcomparer-out-of-queries)). - EF Core: a `StringSet` or `StringSet` property with a collation that may not compare bytes throws when the model is built, and so does `EF.Functions.Collate` over one in a query. Under a case-insensitive collation, `Contains("a")` matched `{A}` and `Remove("a")` deleted `A`, unlike in memory ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-string-set-columns-with-a-collation)). +- EF Core: a query that compares a `StringSet` or `StringSet` with a value whose column has a collation that may not compare bytes, or with `EF.Functions.Collate(value, …)` under one, throws and names the collation. PostgreSQL compares under the collation of either side, so `Contains(g.Owner)` matched `{A}` for `"a"` on a case-insensitive `Owner`, and `Remove(g.Owner)` in `ExecuteUpdate` deleted `A` ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-string-set-columns-with-a-collation)). - EF Core: on a `DateRange`, a `LocalDateRange` and their multiranges, `LowerBound()`, `UpperBound()`, `LowerBoundInclusive()` and `Clamp` read an exclusive `infinity` or `-infinity` in a query as the row loads. On `[2026-01-01,infinity)`, `UpperBound()` returned 9999-12-31 where memory gives 9999-12-30 ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-date-bounds-at-an-exclusive-infinity-in-queries)). - EF Core: on a `YearMonthRange` and its multirange, `UpperBound()` and `Clamp` compare with a month in a query as in memory. They read the last day of the month, so on `[2026-01,2026-01]` `UpperBound() == 2026-01` was `false` and `UpperBound() > 2026-01` was `true` ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-yearmonthrange-upper-bounds-compared-in-queries)). diff --git a/docs/efcore.md b/docs/efcore.md index 9ae70bd7..5e2e18fc 100644 --- a/docs/efcore.md +++ b/docs/efcore.md @@ -551,6 +551,12 @@ Good to know: `pg_c_utf8` or `default`. `EF.Functions.Collate` over a string set throws the same way. A column that something outside your model gave a collation can't be checked, so keep that collation deterministic. +- **The value you compare with a string set brings its collation too.** PostgreSQL compares under the + collation of either side. So a query throws when it compares a string set with a value from a column + whose collation may not compare bytes, or with `EF.Functions.Collate(value, …)` under one, and the + message names the collation. That covers `Contains`, `Add`, `Remove`, `==` and the subset operators, + in `ExecuteUpdate` too. To compare such a value as memory does, write + `g.Members.Contains(EF.Functions.Collate(g.Owner, "C"))`. - **Plain arrays keep their own mapping.** `T[]` and `List` properties keep their native Npgsql mapping, and both can sit next to value sets in one model. Scaffolding a database gives you plain arrays, because choosing a set type is a decision you make in your model. diff --git a/docs/migration.md b/docs/migration.md index 5ee5d50b..b10e89ee 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -831,6 +831,10 @@ where, so check these: - **EF Core: a string set column with a collation that may not compare bytes is rejected.** Building the model throws. Under a case-insensitive collation, `Contains("a")` matched `{A}`, and `Remove("a")` in `ExecuteUpdate` deleted `A` ([details](#check-string-set-columns-with-a-collation)). +- **EF Core: a string set compared with a value under a collation that may not compare bytes throws.** + The value's column or `EF.Functions.Collate` gave the comparison its collation, so `Contains(g.Owner)` + matched `{A}` for `"a"` on a case-insensitive `Owner` + ([details](#check-string-set-columns-with-a-collation)). - **EF Core: a date bound at an exclusive infinity reads in a query as the row loads.** On `[2026-01-01,infinity)`, `UpperBound()` returned 9999-12-31 in a query and 9999-12-30 in memory ([details](#check-date-bounds-at-an-exclusive-infinity-in-queries)). @@ -838,9 +842,9 @@ where, so check these: `[2026-01,2026-01]`, `UpperBound() == 2026-01` was `false` in a query and `true` in memory ([details](#check-yearmonthrange-upper-bounds-compared-in-queries)). -The first four also shipped in 10.2.0, the next two in 10.3.0, and the last three in 10.4.0. If you +The first four also shipped in 10.2.0, the next two in 10.3.0, and the last four in 10.4.0. If you upgrade from 10.2.0 or later, you've already met the first four, from 10.3.0 or later the first six, -and from 10.4.0 or later all nine. +and from 10.4.0 or later all ten. ### Check bound comparisons with `null` in queries @@ -998,8 +1002,9 @@ that can be NULL gets `CASE WHEN … IS NULL THEN NULL ELSE array_cat(…) END`. ### Check string set columns with a collation -Check `StringSet` and `StringSet` properties configured with `UseCollation`, and queries -that call `EF.Functions.Collate` on one. +Check `StringSet` and `StringSet` properties configured with `UseCollation`, queries that +call `EF.Functions.Collate` on one, and queries that compare one with a value from a column configured +with `UseCollation`. PostgreSQL compares the elements of a `text[]` column with the column's collation, and memory compares them ordinally. A deterministic collation breaks every tie by the bytes, so the two agree. A @@ -1026,6 +1031,24 @@ compare bytes are allowed: `"C"`, `"POSIX"`, `ucs_basic`, `pg_c_utf8` and `defau answers as memory does, but the model can't tell it from a nondeterministic one by its name. - **A collation given to the column outside your model isn't seen.** Keep it deterministic. +The value you compare with a string set counts too, because PostgreSQL compares under the collation of +either side. With `Members` holding `{A,b}`, no collation on it, and `Owner` holding `"a"` under a +case-insensitive collation: + +| Query | Memory | Before 10.4.0 | +|---|---|---| +| `Where(g => g.Members.Contains(g.Owner))` | no match | matched | +| `Where(g => g.Members.Contains(EF.Functions.Collate("a", "case_insensitive")))` | no match | matched | +| `ExecuteUpdate(… g => g.Members.Remove(g.Owner))` | — | stored `{b}` | + +Now such a query throws `InvalidOperationException` and names the collation. The same five collations +are allowed on the value's column. + +- **Compare with `EF.Functions.Collate(g.Owner, "C")`.** An explicit collation wins over the column's, + so the query compares bytes, as memory does. Unless the set column is collated `"C"` too, PostgreSQL + then can't use a GIN index on it for that comparison. +- **Or load the value first.** A value your code passes in has no collation. + ### Check date bounds at an exclusive infinity in queries Check EF Core queries that call `LowerBound()`, `UpperBound()`, `LowerBoundInclusive()` or `Clamp` on diff --git a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md index 5ecba419..67f1f48a 100644 --- a/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md +++ b/src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md @@ -18,6 +18,7 @@ The first preview of the 11 line: the same public API as 10.0.0, on .NET 11, EF - EF Core: a value set `Union` with a NULL set, or `Remove` with a NULL set or element, is NULL in a query, as `Where(… == null)` already said. A `Select` loaded the other set instead, for a nullable column, a missing outer-join child or a `null` parameter ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-value-set-union-and-remove-over-null-in-queries)). - EF Core: any `Compare(x, y)` over ranges or sets, `RangeLowerBoundComparer.Compare` and every other `IComparer` included, runs in memory only in a query, as the method called. A filter or an ordering fails translation. In SQL it compared in the range order, so `Compare(r.Period, p) == 0` missed rows with the same lower bound ([#464](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/464), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#move-rangelowerboundcomparer-out-of-queries)). - EF Core: a `StringSet` or `StringSet` property with a collation that may not compare bytes throws when the model is built, and so does `EF.Functions.Collate` over one in a query. Under a case-insensitive collation, `Contains("a")` matched `{A}` and `Remove("a")` deleted `A`, unlike in memory ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-string-set-columns-with-a-collation)). +- EF Core: a query that compares a `StringSet` or `StringSet` with a value whose column has a collation that may not compare bytes, or with `EF.Functions.Collate(value, …)` under one, throws and names the collation. PostgreSQL compares under the collation of either side, so `Contains(g.Owner)` matched `{A}` for `"a"` on a case-insensitive `Owner`, and `Remove(g.Owner)` in `ExecuteUpdate` deleted `A` ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-string-set-columns-with-a-collation)). - EF Core: on a `DateRange` and its multirange, `LowerBound()`, `UpperBound()`, `LowerBoundInclusive()` and `Clamp` read an exclusive `infinity` or `-infinity` one day inside in a query, as the row loads. On `[2026-01-01,infinity)`, which another tool or the database's own `Except` writes, `UpperBound()` returned 9999-12-31 where memory gives 9999-12-30, and on `(-infinity,2026-01-01)` `LowerBound()` returned 0001-01-01 and `LowerBoundInclusive()` `false` ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-date-bounds-at-an-exclusive-infinity-in-queries)). ### Added diff --git a/test/oracles/claims.json b/test/oracles/claims.json index 983e59db..da2a4785 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -1544,6 +1544,22 @@ "NodaQueryTranslationTests" ] }, + { + "id": "E89-string-set-compared-with-a-collated-value", + "source": "docs/efcore.md#store-a-value-set-in-an-array-column", + "quote": "So a query throws when it compares a string set with a value from a column whose collation may not compare bytes, or with `EF.Functions.Collate(value, …)` under one, and the message names the collation.", + "claim": "A query comparing a StringSet with a value whose column is configured with a collation other than C, POSIX, ucs_basic, pg_c_utf8 or default, or with EF.Functions.Collate(value, such a collation) - through Contains, Add, Remove, ==, the subset operators and ExecuteUpdate with Remove, directly or through a derived table's or set operation's column, a concatenation, a CASE result or a scalar subquery - throws InvalidOperationException naming the collation, on a runtime and a compiled model, and leaves the row unchanged; EF.Functions.Collate(value, \"C\"), a byte-order column and a CASE that only tests the column translate and answer as memory does.", + "oracle": "exception", + "surfaces": [ + "ValueSetExtensions.Contains", + "ValueSetExtensions.Remove", + "ValueSetExtensions.Add" + ], + "heldBy": [ + "StringSetCollationTests", + "StringSetCollationIntegrationTests" + ] + }, { "id": "J01-tostring-is-postgres-literal", "source": "docs/serialization.md#print-a-range-as-a-literal",