fix(668): reach accented facet values via a registered Unicode fold on SQLite
SQLite's LOWER() folds ASCII only -- lower('Édith') is 'Édith' unchanged --
so the EF-sourced facet fields UNDER-matched any stored value whose prefix
carries an uppercase non-ASCII character. An under-match is unrecoverable:
no later stage can reintroduce a row SQL never returned.
Adds a SECOND, ADDITIVE query taken only when the provider is SQLite and q
contains a non-ASCII character: raw Dapper SQL folding through etv_upper(),
a SqliteConnection.CreateFunction scalar implementing ToUpperInvariant.
Every other case -- all-ASCII q, and MySQL for all q -- runs the existing
EF query byte-identically.
MySQL needed no change and gets none: verified on MySQL 8.4 that its LOWER()
is Unicode-aware and its ci collation makes the predicate OVER-match, which
the existing ordinal filter already discards.
The fold is ToUpperInvariant because OrdinalIgnoreCase equality is a strict
SUBSET of invariant-uppercase equality, so the SQL stage yields a superset of
the final filter's matches and can never under-match. Note OrdinalIgnoreCase
is NOT "invariant-upper then ordinal": ToUpperInvariant('ſ') is 'S', yet
"ſweet".StartsWith("S", OrdinalIgnoreCase) is false. Tests pin that.
No migration, no model change; both provider snapshots are untouched.
Refs #668
Decisions-Edit: yes
This commit is contained in:
@@ -129,6 +129,22 @@ public class GetSearchFieldValuesHandler(IDbContextFactory<TvContext> dbContextF
|
||||
.ToListAsync(cancellationToken));
|
||||
}
|
||||
|
||||
// ersatztv#668. The query above prefix-matches through SQL LOWER(), and SQLite's LOWER() folds ASCII
|
||||
// ONLY -- lower('Édith') is 'Édith' unchanged -- so it cannot reach a stored value whose prefix
|
||||
// carries an uppercase non-ASCII character, from ANY query. It UNDER-matches, and an under-match is
|
||||
// unrecoverable downstream: no later stage can reintroduce a row SQL never returned. So for the only
|
||||
// queries that can be affected (those containing a non-ASCII character) run a second, Unicode-correct
|
||||
// pass and merge it in. This is ADDITIVE on purpose -- the SQL pass above still contributes, so a
|
||||
// value already reachable today cannot stop being reachable.
|
||||
//
|
||||
// MySQL needs none of this: its LOWER() is Unicode-aware, and its ci collations make the predicate
|
||||
// OVER-match, which the ordinal FilterSortTake below already corrects. Verified against MySQL 8.4.
|
||||
if (source is not null && ContainsNonAscii(query) && IsSqlite(dbContext))
|
||||
{
|
||||
values.AddRange(
|
||||
await GetUnicodeFoldedValues(dbContext, request.Name, query, limit, cancellationToken));
|
||||
}
|
||||
|
||||
if (listColumn is not null)
|
||||
{
|
||||
values.AddRange(await GetSongListValuedValues(dbContext, listColumn, query, limit, cancellationToken));
|
||||
@@ -172,6 +188,171 @@ public class GetSearchFieldValuesHandler(IDbContextFactory<TvContext> dbContextF
|
||||
_ => null
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// SQL name of the invariant-uppercase fold registered by <c>SqliteUnicodeFunctions</c>. Duplicated
|
||||
/// rather than referenced because Application must not depend on a provider assembly; a test asserts
|
||||
/// the two constants are equal so they cannot drift.
|
||||
/// </summary>
|
||||
internal const string UpperFunction = "etv_upper";
|
||||
|
||||
/// <summary>
|
||||
/// True when the value contains any character outside US-ASCII, which is exactly when SQLite's
|
||||
/// ASCII-only <c>LOWER()</c> can under-match. Evaluated on the RAW query, never the lowercased copy:
|
||||
/// the trigger must not be coupled to the fold.
|
||||
/// </summary>
|
||||
internal static bool ContainsNonAscii(string value)
|
||||
{
|
||||
foreach (char c in value)
|
||||
{
|
||||
if (c > 0x7F)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private static bool IsSqlite(TvContext dbContext) =>
|
||||
(dbContext.Database.ProviderName ?? string.Empty).Contains("Sqlite", StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
/// <summary>
|
||||
/// Escapes the LIKE metacharacters in a user-supplied prefix and appends the trailing wildcard. The
|
||||
/// backslash MUST be escaped first, or the escapes added for <c>%</c>/<c>_</c> would themselves be
|
||||
/// re-escaped. Paired with an explicit <c>ESCAPE '\'</c> in <see cref="UnicodeFoldSql" />, since raw
|
||||
/// SQL gets none of the escaping EF does for <c>StartsWith</c>.
|
||||
/// </summary>
|
||||
internal static string EscapeLikePrefix(string value) =>
|
||||
value
|
||||
.Replace("\\", "\\\\", StringComparison.Ordinal)
|
||||
.Replace("%", "\\%", StringComparison.Ordinal)
|
||||
.Replace("_", "\\_", StringComparison.Ordinal) + "%";
|
||||
|
||||
/// <summary>
|
||||
/// One bounded, exact prefix query using the Unicode-correct fold. Unlike the list-valued walk this
|
||||
/// KEEPS its selectivity in SQL — it is a normal indexed-or-not <c>LIMIT</c>ed query exactly like the
|
||||
/// EF one it supplements, not a paged walk, so there is no row budget to blow and no reason to strip
|
||||
/// the discriminator predicates out of it.
|
||||
/// </summary>
|
||||
internal static string UnicodeFoldSql(string table, string column, string predicate)
|
||||
{
|
||||
var match = $"{UpperFunction}({column}) LIKE @Pattern ESCAPE '\\'";
|
||||
string where = predicate is null ? match : $"({predicate}) AND {match}";
|
||||
return $"SELECT DISTINCT {column} AS Value FROM {table} WHERE {where} ORDER BY {column} LIMIT @Limit";
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The tables/columns behind each EF-sourced field, mirroring <see cref="GetSource" /> 1:1.
|
||||
/// <para>
|
||||
/// The discriminator predicates must mirror EF's NULL semantics, not C#'s reading of the source.
|
||||
/// EF compiles <c>t.ExternalTypeId != Tag.NfoCountryTypeId</c> with null semantics, so a row whose
|
||||
/// <c>ExternalTypeId</c> is NULL IS included; plain SQL <c><></c> against NULL yields NULL and
|
||||
/// would silently drop it. Hence the explicit <c>IS NULL</c> arm.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private static IReadOnlyList<UnicodeFoldSource> GetUnicodeFoldSources(string name) => name switch
|
||||
{
|
||||
"genre" or "show_genre" => [new UnicodeFoldSource("Genre", "Name")],
|
||||
"studio" => [new UnicodeFoldSource("Studio", "Name")],
|
||||
"director" => [new UnicodeFoldSource("Director", "Name")],
|
||||
"writer" => [new UnicodeFoldSource("Writer", "Name")],
|
||||
"actor" => [new UnicodeFoldSource("Actor", "Name")],
|
||||
"artist" =>
|
||||
[
|
||||
new UnicodeFoldSource("ArtistMetadata", "Title"),
|
||||
new UnicodeFoldSource("MusicVideoArtist", "Name")
|
||||
],
|
||||
"tag" =>
|
||||
[
|
||||
new UnicodeFoldSource(
|
||||
"Tag",
|
||||
"Name",
|
||||
"ExternalTypeId IS NULL OR (ExternalTypeId <> @NfoCountryTypeId AND ExternalTypeId <> @PlexNetworkTypeId)",
|
||||
new Dictionary<string, object>
|
||||
{
|
||||
["NfoCountryTypeId"] = Tag.NfoCountryTypeId,
|
||||
["PlexNetworkTypeId"] = Tag.PlexNetworkTypeId
|
||||
})
|
||||
],
|
||||
"network" =>
|
||||
[
|
||||
new UnicodeFoldSource(
|
||||
"Tag",
|
||||
"Name",
|
||||
"ExternalTypeId = @PlexNetworkTypeId",
|
||||
new Dictionary<string, object> { ["PlexNetworkTypeId"] = Tag.PlexNetworkTypeId })
|
||||
],
|
||||
"collection" => [new UnicodeFoldSource("Collection", "Name")],
|
||||
"video_codec" =>
|
||||
[
|
||||
new UnicodeFoldSource(
|
||||
"MediaStream",
|
||||
"Codec",
|
||||
"MediaStreamKind = @VideoStreamKind AND Codec IS NOT NULL",
|
||||
new Dictionary<string, object> { ["VideoStreamKind"] = (int)MediaStreamKind.Video })
|
||||
],
|
||||
"album" =>
|
||||
[
|
||||
new UnicodeFoldSource("MusicVideoMetadata", "Album", "Album IS NOT NULL"),
|
||||
new UnicodeFoldSource("SongMetadata", "Album", "Album IS NOT NULL")
|
||||
],
|
||||
_ => []
|
||||
};
|
||||
|
||||
private static async Task<List<string>> GetUnicodeFoldedValues(
|
||||
TvContext dbContext,
|
||||
string name,
|
||||
string query,
|
||||
int limit,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
IReadOnlyList<UnicodeFoldSource> sources = GetUnicodeFoldSources(name);
|
||||
if (sources.Count == 0)
|
||||
{
|
||||
return [];
|
||||
}
|
||||
|
||||
// CreateFunction is per-connection, so registration happens here, at the one call site that needs
|
||||
// the function, rather than through an EF connection interceptor: Dapper opens a closed connection
|
||||
// itself and a direct ADO open does not raise EF's interceptors, so an interceptor-based seam would
|
||||
// silently miss exactly this query. Opening first makes the registration order-independent.
|
||||
await dbContext.Database.OpenConnectionAsync(cancellationToken);
|
||||
TvContext.RegisterUnicodeCaseFunctions(dbContext.Connection);
|
||||
|
||||
string pattern = EscapeLikePrefix(query.ToUpperInvariant());
|
||||
var values = new List<string>();
|
||||
|
||||
foreach (UnicodeFoldSource source in sources)
|
||||
{
|
||||
var parameters = new DynamicParameters();
|
||||
parameters.Add("Pattern", pattern);
|
||||
parameters.Add("Limit", limit);
|
||||
if (source.Parameters is not null)
|
||||
{
|
||||
foreach ((string key, object value) in source.Parameters)
|
||||
{
|
||||
parameters.Add(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
IEnumerable<string> rows = await dbContext.Connection.QueryAsync<string>(
|
||||
new CommandDefinition(
|
||||
UnicodeFoldSql(source.Table, source.Column, source.Predicate),
|
||||
parameters,
|
||||
cancellationToken: cancellationToken));
|
||||
|
||||
values.AddRange(rows.Where(v => !string.IsNullOrEmpty(v)));
|
||||
}
|
||||
|
||||
return values;
|
||||
}
|
||||
|
||||
private sealed record UnicodeFoldSource(
|
||||
string Table,
|
||||
string Column,
|
||||
string Predicate = null,
|
||||
IReadOnlyDictionary<string, object> Parameters = null);
|
||||
|
||||
/// <summary>
|
||||
/// Maps a field name onto the <c>SongMetadata</c> column that backs it as an <c>IList<string></c>.
|
||||
/// The returned value is a compile-time constant from this switch — never caller input — so it is safe
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
using System.Data;
|
||||
using Microsoft.Data.Sqlite;
|
||||
|
||||
namespace ErsatzTV.Infrastructure.Sqlite.Data;
|
||||
|
||||
/// <summary>
|
||||
/// ersatztv#668. SQLite's built-in <c>lower()</c>/<c>upper()</c> fold ASCII ONLY — <c>lower('Édith')</c>
|
||||
/// returns <c>'Édith'</c> unchanged — so a facet value whose prefix carries an uppercase non-ASCII
|
||||
/// character can never be matched by the prefix predicate the facet-value endpoint emits. Registering a
|
||||
/// managed scalar gives that one query a Unicode-correct fold. Wired to
|
||||
/// <see cref="ErsatzTV.Infrastructure.Data.TvContext.RegisterUnicodeCaseFunctions" /> at startup.
|
||||
/// </summary>
|
||||
public static class SqliteUnicodeFunctions
|
||||
{
|
||||
/// <summary>
|
||||
/// SQL name of the invariant-uppercase fold. The facet-value handler interpolates this constant into
|
||||
/// its SQL, so the two cannot drift apart.
|
||||
/// </summary>
|
||||
public const string UpperInvariantFunction = "etv_upper";
|
||||
|
||||
/// <summary>
|
||||
/// Registers <see cref="UpperInvariantFunction" /> on <paramref name="connection" /> when it is a
|
||||
/// SQLite connection, and does nothing otherwise. Idempotent — a repeat registration replaces the
|
||||
/// previous delegate with an identical one — so the single call site may call it unconditionally.
|
||||
/// <para>
|
||||
/// The property this fold has to satisfy is ONE-SIDED: the SQL stage may over-match freely,
|
||||
/// because the endpoint applies an exact <see cref="StringComparison.OrdinalIgnoreCase" /> filter
|
||||
/// in memory afterwards, but it must never UNDER-match — no later stage can reintroduce a row SQL
|
||||
/// never returned. <see cref="string.ToUpperInvariant" /> satisfies it because
|
||||
/// <c>OrdinalIgnoreCase</c> equality is a strict SUBSET of invariant-uppercase equality, so
|
||||
/// folding both sides with it yields a superset of the final filter's matches.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Do not restate that as "<c>OrdinalIgnoreCase</c> IS invariant-uppercase-then-ordinal" — it is
|
||||
/// not, and the difference is measurable: <c>char.ToUpperInvariant('ſ')</c> (U+017F) is <c>'S'</c>,
|
||||
/// yet <c>"ſweet".StartsWith("S", OrdinalIgnoreCase)</c> is <b>false</b>. That gap is precisely
|
||||
/// the harmless direction — SQL returns the row, the in-memory filter drops it. The containment,
|
||||
/// not any identity of the two foldings, is what makes this safe.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Registration is per-connection and therefore done at the one call site that uses the function,
|
||||
/// not through an EF connection interceptor: Dapper opens a closed connection itself, and a direct
|
||||
/// ADO open does not raise EF's interceptors — so an interceptor-based seam would silently miss
|
||||
/// exactly the query that needs it.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public static void Register(IDbConnection connection)
|
||||
{
|
||||
if (connection is SqliteConnection sqlite)
|
||||
{
|
||||
sqlite.CreateFunction(
|
||||
UpperInvariantFunction,
|
||||
(string? value) => value?.ToUpperInvariant(),
|
||||
isDeterministic: true);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -36,6 +36,18 @@ public class TvContext : DbContext
|
||||
/// </summary>
|
||||
public static Func<DbUpdateException, bool> IsUniqueConstraintViolation { get; set; } = static _ => false;
|
||||
|
||||
/// <summary>
|
||||
/// Registers provider-specific SQL scalar functions on a connection, called immediately before a raw
|
||||
/// query that needs them. Set at startup by the active provider's wiring, mirroring
|
||||
/// <see cref="IsUniqueConstraintViolation" />: SQLite points this at
|
||||
/// <c>SqliteUnicodeFunctions.Register</c>, MySQL leaves it a no-op because its own <c>LOWER()</c> is
|
||||
/// already Unicode-aware and needs no help. Defaults to a no-op, which is safe because the sole
|
||||
/// caller invokes it only on the SQLite branch that requires it, and an unwired provider then fails
|
||||
/// LOUDLY ("no such function: etv_upper") rather than returning silently wrong results. See
|
||||
/// ersatztv#668.
|
||||
/// </summary>
|
||||
public static Action<IDbConnection> RegisterUnicodeCaseFunctions { get; set; } = static _ => { };
|
||||
|
||||
public IDbConnection Connection => Database.GetDbConnection();
|
||||
|
||||
public DbSet<ConfigElement> ConfigElements { get; set; }
|
||||
|
||||
@@ -162,6 +162,7 @@ public class Program
|
||||
TvContext.LastInsertedRowId = "last_insert_rowid()";
|
||||
TvContext.CaseInsensitiveCollation = "NOCASE";
|
||||
TvContext.IsUniqueConstraintViolation = SqliteErrorClassifier.IsUniqueConstraintViolation;
|
||||
TvContext.RegisterUnicodeCaseFunctions = SqliteUnicodeFunctions.Register;
|
||||
|
||||
SqlMapper.AddTypeHandler(new DateTimeOffsetHandler());
|
||||
SqlMapper.AddTypeHandler(new GuidHandler());
|
||||
@@ -173,6 +174,10 @@ public class Program
|
||||
TvContext.LastInsertedRowId = "last_insert_id()";
|
||||
TvContext.CaseInsensitiveCollation = "utf8mb4_general_ci";
|
||||
TvContext.IsUniqueConstraintViolation = MySqlErrorClassifier.IsUniqueConstraintViolation;
|
||||
|
||||
// MySQL's LOWER() is already Unicode-aware; assigned explicitly for the same reason as
|
||||
// the host — a provider switch must not inherit SQLite's registration.
|
||||
TvContext.RegisterUnicodeCaseFunctions = static _ => { };
|
||||
}
|
||||
|
||||
services.AddHttpClient();
|
||||
|
||||
@@ -706,4 +706,109 @@ public class GetSearchFieldValuesHandlerTests
|
||||
result.IsSome.ShouldBeTrue();
|
||||
result.IfSome(r => r.Values.ShouldBe(new List<string> { "édith" }));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ersatztv#668. The Unicode fold added for the non-ASCII branch may OVER-match — the in-memory
|
||||
/// <see cref="StringComparison.OrdinalIgnoreCase" /> filter runs afterwards and drops the extras —
|
||||
/// but it must never UNDER-match. Each case pins the endpoint's answer against what that filter
|
||||
/// alone would say, so a fold that starts dropping rows (or one that stops filtering the extras out)
|
||||
/// fails here.
|
||||
/// <para>
|
||||
/// The negative cases are the load-bearing ones. <c>char.ToUpperInvariant('ſ')</c> IS <c>'S'</c>,
|
||||
/// so an upper-folding SQL stage genuinely returns "ſweet" for q="S" — and the expected result is
|
||||
/// still empty, because <c>"ſweet".StartsWith("S", OrdinalIgnoreCase)</c> is false. Same for
|
||||
/// U+212A KELVIN SIGN. That is the over-match being correctly discarded.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[TestCase("Édith", "é", true, TestName = "Fold_UppercaseAccent_LowercaseQuery")]
|
||||
[TestCase("Édith", "É", true, TestName = "Fold_UppercaseAccent_UppercaseQuery")]
|
||||
[TestCase("Özdemir", "ö", true, TestName = "Fold_Umlaut")]
|
||||
[TestCase("Sigur Rós", "sigur", true, TestName = "Fold_AsciiPrefix_NonAsciiLater")]
|
||||
[TestCase("Straße", "stra", true, TestName = "Fold_Eszett_AsciiQuery")]
|
||||
// explicit escapes: these three are visually indistinguishable from their ASCII lookalikes in a diff,
|
||||
// and an ASCII 'K' here would silently turn the KELVIN SIGN case into a trivially-true one
|
||||
[TestCase("\u017Fweet", "S", false, TestName = "Fold_LongS_IsNotOrdinalEqualToS")]
|
||||
[TestCase("\u212Aelvin", "k", false, TestName = "Fold_KelvinSign_IsNotOrdinalEqualToK")]
|
||||
[TestCase("\u0130stanbul", "i", false, TestName = "Fold_DottedCapitalI_IsNotOrdinalEqualToI")]
|
||||
public async Task Unicode_Fold_Agrees_With_The_Ordinal_Filter(string stored, string query, bool expected)
|
||||
{
|
||||
await using (TvContext context = _db.CreateContext())
|
||||
{
|
||||
context.Set<Genre>().Add(new Genre { Name = stored });
|
||||
await context.SaveChangesAsync();
|
||||
}
|
||||
|
||||
// the oracle: what the endpoint's own final filter says, computed independently of the database
|
||||
stored.StartsWith(query, StringComparison.OrdinalIgnoreCase).ShouldBe(expected);
|
||||
|
||||
var handler = new GetSearchFieldValuesHandler(_db.Factory);
|
||||
|
||||
Option<SearchFieldValuesResponseModel> result = await handler.Handle(
|
||||
new GetSearchFieldValues("genre", query, 50),
|
||||
CancellationToken.None);
|
||||
|
||||
result.IsSome.ShouldBeTrue();
|
||||
result.IfSome(r => r.Values.ShouldBe(expected ? new List<string> { stored } : []));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ersatztv#668. The non-ASCII branch is raw SQL, so it gets none of the LIKE-wildcard escaping EF
|
||||
/// does for <c>StartsWith</c>. An unescaped <c>%</c> or <c>_</c> in the query would match anything.
|
||||
/// </summary>
|
||||
[TestCase("100%É", 1, TestName = "Escapes_Percent")]
|
||||
[TestCase("100_É", 0, TestName = "Escapes_Underscore")]
|
||||
public async Task Unicode_Fold_Escapes_Like_Wildcards(string query, int expectedCount)
|
||||
{
|
||||
await using (TvContext context = _db.CreateContext())
|
||||
{
|
||||
context.Set<Genre>().AddRange(
|
||||
new Genre { Name = "100%Édith" },
|
||||
new Genre { Name = "100XÉdith" });
|
||||
await context.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var handler = new GetSearchFieldValuesHandler(_db.Factory);
|
||||
|
||||
Option<SearchFieldValuesResponseModel> result = await handler.Handle(
|
||||
new GetSearchFieldValues("genre", query, 50),
|
||||
CancellationToken.None);
|
||||
|
||||
result.IsSome.ShouldBeTrue();
|
||||
result.IfSome(r => r.Values.Count.ShouldBe(expectedCount));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ersatztv#668. The non-ASCII branch duplicates each field's discriminator predicate in raw SQL, so
|
||||
/// it must reproduce EF's NULL semantics: EF compiles <c>ExternalTypeId != NfoCountryTypeId</c> with
|
||||
/// null semantics, which INCLUDES a NULL-typed row. Plain SQL <c><></c> would silently drop it.
|
||||
/// </summary>
|
||||
[Test]
|
||||
public async Task Unicode_Fold_Tag_Discriminator_Matches_Ef_Null_Semantics()
|
||||
{
|
||||
await using (TvContext context = _db.CreateContext())
|
||||
{
|
||||
context.Set<Tag>().AddRange(
|
||||
new Tag { Name = "Édith", ExternalTypeId = null },
|
||||
new Tag { Name = "Éclair", ExternalTypeId = Tag.PlexNetworkTypeId },
|
||||
new Tag { Name = "Ézra", ExternalTypeId = Tag.NfoCountryTypeId });
|
||||
await context.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var handler = new GetSearchFieldValuesHandler(_db.Factory);
|
||||
|
||||
Option<SearchFieldValuesResponseModel> tags = await handler.Handle(
|
||||
new GetSearchFieldValues("tag", "é", 50),
|
||||
CancellationToken.None);
|
||||
|
||||
// the NULL-typed row is a tag; the network- and country-typed rows are excluded
|
||||
tags.IsSome.ShouldBeTrue();
|
||||
tags.IfSome(r => r.Values.ShouldBe(new List<string> { "Édith" }));
|
||||
|
||||
Option<SearchFieldValuesResponseModel> networks = await handler.Handle(
|
||||
new GetSearchFieldValues("network", "é", 50),
|
||||
CancellationToken.None);
|
||||
|
||||
networks.IsSome.ShouldBeTrue();
|
||||
networks.IfSome(r => r.Values.ShouldBe(new List<string> { "Éclair" }));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
using ErsatzTV.Application.Search.Queries;
|
||||
using ErsatzTV.Infrastructure;
|
||||
using ErsatzTV.Infrastructure.Data;
|
||||
using ErsatzTV.Infrastructure.Sqlite.Data;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using NUnit.Framework;
|
||||
@@ -102,6 +103,45 @@ public class SearchFieldValuesQueryShapeTests
|
||||
sql.ShouldNotContain("IS NOT NULL");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ersatztv#668. The SQL function name is duplicated — the handler lives in Application, which must
|
||||
/// not reference a provider assembly, so it cannot use the constant the registration side defines. A
|
||||
/// rename on one side alone would compile cleanly and fail only at runtime, only on SQLite, only for
|
||||
/// non-ASCII queries; this pins the two together instead.
|
||||
/// </summary>
|
||||
[Test]
|
||||
public void Unicode_Fold_Function_Name_Matches_The_Registration() =>
|
||||
GetSearchFieldValuesHandler.UpperFunction.ShouldBe(SqliteUnicodeFunctions.UpperInvariantFunction);
|
||||
|
||||
/// <summary>
|
||||
/// ersatztv#668. Unlike the list-valued walk, this query KEEPS its selectivity in SQL — it is a
|
||||
/// bounded <c>LIMIT</c>ed prefix query exactly like the EF one it supplements, so a <c>LIKE</c> here
|
||||
/// is correct rather than the trap the walk's shape test guards against. What must hold is that the
|
||||
/// fold is the registered Unicode-correct one and NOT the provider's ASCII-only builtin, and that the
|
||||
/// wildcard escape is declared.
|
||||
/// </summary>
|
||||
[Test]
|
||||
public void Unicode_Fold_Query_Uses_The_Registered_Fold_And_Declares_Its_Escape()
|
||||
{
|
||||
string sql = GetSearchFieldValuesHandler.UnicodeFoldSql("Genre", "Name", null);
|
||||
|
||||
sql.ShouldBe(
|
||||
"SELECT DISTINCT Name AS Value FROM Genre "
|
||||
+ "WHERE etv_upper(Name) LIKE @Pattern ESCAPE '\\' ORDER BY Name LIMIT @Limit");
|
||||
|
||||
// The point of the whole change: SQLite's BUILTIN lower()/upper() fold ASCII only, so quietly falling
|
||||
// back to one reinstates #668. Checked by removing the qualified call first — Shouldly's string
|
||||
// assertions are case-INSENSITIVE by default, so a bare ShouldNotContain("UPPER(") matches inside
|
||||
// "etv_upper(" and fails against correct SQL.
|
||||
sql.ShouldNotContain("LOWER(");
|
||||
sql.Replace($"{GetSearchFieldValuesHandler.UpperFunction}(", "", StringComparison.Ordinal)
|
||||
.ShouldNotContain("UPPER(");
|
||||
|
||||
// a discriminator predicate is parenthesised and ANDed, so an OR inside it cannot swallow the match
|
||||
GetSearchFieldValuesHandler.UnicodeFoldSql("Tag", "Name", "ExternalTypeId IS NULL OR X")
|
||||
.ShouldContain("WHERE (ExternalTypeId IS NULL OR X) AND etv_upper(Name) LIKE @Pattern");
|
||||
}
|
||||
|
||||
private static IEnumerable<(string Provider, Func<TvContext> Create)> Providers() =>
|
||||
[
|
||||
("sqlite", Sqlite),
|
||||
|
||||
@@ -31,6 +31,7 @@ public sealed class InMemoryTvContext : IAsyncDisposable
|
||||
{
|
||||
TvContext.IsSqlite = true;
|
||||
TvContext.IsUniqueConstraintViolation = SqliteErrorClassifier.IsUniqueConstraintViolation;
|
||||
TvContext.RegisterUnicodeCaseFunctions = SqliteUnicodeFunctions.Register;
|
||||
|
||||
var connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=False");
|
||||
await connection.OpenAsync();
|
||||
|
||||
@@ -649,6 +649,7 @@ public class Startup
|
||||
TvContext.LastInsertedRowId = "last_insert_rowid()";
|
||||
TvContext.CaseInsensitiveCollation = "NOCASE";
|
||||
TvContext.IsUniqueConstraintViolation = SqliteErrorClassifier.IsUniqueConstraintViolation;
|
||||
TvContext.RegisterUnicodeCaseFunctions = SqliteUnicodeFunctions.Register;
|
||||
|
||||
SqlMapper.AddTypeHandler(new DateTimeOffsetHandler());
|
||||
SqlMapper.AddTypeHandler(new GuidHandler());
|
||||
@@ -660,6 +661,10 @@ public class Startup
|
||||
TvContext.LastInsertedRowId = "last_insert_id()";
|
||||
TvContext.CaseInsensitiveCollation = "utf8mb4_general_ci";
|
||||
TvContext.IsUniqueConstraintViolation = MySqlErrorClassifier.IsUniqueConstraintViolation;
|
||||
|
||||
// MySQL's LOWER() is already Unicode-aware, so the facet-value handler never takes the
|
||||
// custom-fold branch here; assigned explicitly so a provider switch cannot inherit SQLite's.
|
||||
TvContext.RegisterUnicodeCaseFunctions = static _ => { };
|
||||
}
|
||||
|
||||
Log.Logger.Information("Transcode folder is {Folder}", FileSystemLayout.TranscodeFolder);
|
||||
|
||||
@@ -29,6 +29,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `api.scheduling-hardening` | Create/Replace handlers guard against null/whitespace `name` (`IsNullOrWhiteSpace`, not just `Length`) to prevent NRE-500s, template-item overlap validation compares by index (not record value-equality) to catch exact-duplicate items, and unreachable 404 `ProducesResponseType` attributes on create-only actions are trimmed. | 2026-07-13 | [link](records/api/scheduling-hardening.md) |
|
||||
| `api.search-allitems-paging` | `GET /api/v1/search/all-items` is paginated (capped page size, `Totals` field) to bound DoS exposure; the SPA add-all flow pages to completeness instead of relying on an unbounded response. | 2026-07-18 | [link](records/api/search-allitems-paging.md) |
|
||||
| `api.search-field-values-sources` | `GET /api/v1/search/fields/{name}/values?q=&limit=` returns distinct WHOLE values from the database for a narrow allow-list of catalog fields (never the Lucene term dictionary — analyzed `TextField`s store lowercased word tokens, e.g. "Science Fiction" → `science`/`fiction`, useless as a suggestion), 404 for an unknown field, a non-`text` field, or a `text` field with no distinct-value source (`title`, `show_title` only); `limit` clamped to `[1, 50]` (default 50). The FINAL filter, dedup and ordering applied to the response are ORDINAL (`OrdinalIgnoreCase` / `StringComparer.Ordinal`), never current-culture, because `UseRequestLocalization` makes the culture caller-controlled — scoped to the in-memory stages on purpose: a field sourced by a plain EF query is filtered and truncated by the DATABASE collation first (SQLite's `LOWER()` is ASCII-only), which ordinal semantics downstream cannot undo (ersatztv#668). A field whose values live in an EF **primitive collection** (one JSON array per row in a single column: `SongMetadata.Artists`, `SongMetadata.AlbumArtists`) is served, not 404'd, as bounded best-effort, and its rows are read by a keyset page carrying **NO RESIDUAL predicate** — `SELECT Id, <col> AS Payload FROM SongMetadata WHERE Id > @AfterId ORDER BY Id LIMIT @Batch`, no `LIKE`, no `LOWER`, not even `IS NOT NULL`. The cursor is itself a predicate, but a SEEKABLE one on the ordering key: it positions the scan and never discards a row. A RESIDUAL predicate discards rows the engine already produced, and `LIMIT` truncates only the survivors — so with one present it bounds the OUTPUT rather than the row count. All selectivity is in memory. The guarantee is scoped: **at most 20,000 LOGICAL rows returned/materialized and at most 10 round trips (11 for `artist`)** — NOT bounded physical work and NOT bounded bytes, because MySQL traverses deleted-but-unpurged index records and the `TEXT`/`longtext` payload width is unrestricted. The walk pages 2,000 rows at a time, stopping on the first of enough distinct matches, a short page, or the ceiling. | 2026-07-26 | [link](records/api/search-field-values-sources.md) |
|
||||
| `api.search-field-values-unicode-fold` | The EF-sourced facet fields (`genre`, `studio`, `director`, `writer`, `actor`, `tag`, `network`, `collection`, `video_codec`, `album`, and `artist`'s entity half) reach stored values whose prefix carries an uppercase non-ASCII character, on BOTH providers, with no row budget and no accepted loss. The defect was SQLite-only and ONE-SIDED: SQLite's `LOWER()` folds ASCII only (`lower('Édith')` is `'Édith'` unchanged), so the predicate UNDER-matched, which no later stage can repair. MySQL was already correct — its `LOWER()` is Unicode-aware and its ci collation makes the predicate OVER-match, which the existing ordinal filter discards. The fix is a SECOND, ADDITIVE query taken only when `isSqlite && q contains a non-ASCII character`: raw Dapper SQL `SELECT DISTINCT <col> AS Value FROM <table> WHERE [<discriminator> AND] etv_upper(<col>) LIKE @Pattern ESCAPE '\' ORDER BY <col> LIMIT @Limit`, where `etv_upper` is a `SqliteConnection.CreateFunction` scalar implementing `ToUpperInvariant`. Every other case — all-ASCII `q`, and MySQL for all `q` — runs today's EF query BYTE-IDENTICALLY. Keeping selectivity in SQL here is NOT the refuted family from `api.search-field-values-sources`: those four attempts bounded a walk around a predicate that could not be made correct over JSON escape text, whereas this is a correct fold on a plain column in an ordinary `LIMIT`ed query. It narrows that record's "Known limitation inherited, not introduced" clause; everything else it settles still holds. | 2026-07-27 | [link](records/api/search-field-values-unicode-fold.md) |
|
||||
| `api.search-paging-cap` | Search stays capped at 100 items per media kind; an overflowing kind's "See all" reuses library-browse paging instead of adding new API surface. | 2026-07-11 | [link](records/api/search-paging-cap.md) |
|
||||
| `api.versioning-v1` | The entire `/api` surface is versioned to `/api/v1` uniformly (no unversioned corner); legacy unversioned callers are rewritten in-pipeline (not redirected) with Deprecation/Link/Sunset headers, and post-freeze `/api/v1` is additive-only — a breaking change requires `/api/v2`. | 2026-07-13 | [link](records/api/versioning-v1.md) |
|
||||
| `blazor.rollback-tag` | The commit immediately preceding the Blazor-removal merge is tagged `blazor-final` (not a `v*` tag, so it doesn't trigger a prod release build) as the documented rollback/restore path. | 2026-07-11 | [link](records/blazor/rollback-tag.md) |
|
||||
|
||||
@@ -74,8 +74,10 @@ database has already decided which values survive. Store a genre `"Éclair"` on
|
||||
`genre?q=é`: SQLite's ASCII-only `LOWER()` drops it before the ordinal in-memory filter ever runs, and
|
||||
a case-insensitive collation's `DISTINCT` can likewise collapse values ordinal dedup would have kept.
|
||||
The endpoint description and this record's `rule:` therefore say "the final filter, dedup and
|
||||
ordering", not "matching is ordinal". Closing that gap means filtering those tables client-side and is
|
||||
tracked separately as **ersatztv#668** — do not describe it as fixed here.
|
||||
ordering", not "matching is ordinal". That gap is now CLOSED by `api.search-field-values-unicode-fold`
|
||||
(**ersatztv#668**) — not by the client-side filtering guessed at here, but by a registered Unicode-correct
|
||||
SQL fold on a second, additive query taken only for non-ASCII queries on SQLite. The scoped wording above
|
||||
still stands as written: it describes what the EF stage itself does, which is unchanged.
|
||||
|
||||
## Ordering is best-effort, and the code says so
|
||||
|
||||
@@ -199,9 +201,10 @@ any, so no `/api/v2`.
|
||||
|
||||
## Known limitation inherited, not introduced
|
||||
|
||||
The **EF-sourced** fields (`genre`, `studio`, `artist`'s entity half, …) still prefix-match through
|
||||
SQL `LOWER()`, which on SQLite is ASCII-only — so `q=é` does not match a stored `Édith` for those
|
||||
fields either. That predates #578 and is unchanged by it; fixing it would mean filtering those tables
|
||||
client-side. Tracked as **ersatztv#668**. Noted here so the next reader does not mistake the
|
||||
list-valued fix for a global one, and so the scoped ordinal wording above is not "tidied" into a
|
||||
broader claim.
|
||||
**RESOLVED — see `api.search-field-values-unicode-fold` (ersatztv#668, 2026-07-27).** As written for
|
||||
#578 this said: the **EF-sourced** fields (`genre`, `studio`, `artist`'s entity half, …) still
|
||||
prefix-match through SQL `LOWER()`, which on SQLite is ASCII-only, so a stored `Édith` was unreachable
|
||||
for those fields. That predated #578 and was unchanged by it. It is now fixed — and NOT by the
|
||||
client-side filtering this section anticipated, which would have reintroduced the very scan #578 bounded.
|
||||
The surrounding scoped-ordinal wording is still load-bearing and must not be "tidied" into a broader
|
||||
claim: the EF stage's own behaviour is unchanged, and the defect was SQLite-only and one-sided.
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
---
|
||||
key: api.search-field-values-unicode-fold
|
||||
title: '2026-07-27 — Facet-value typeahead reaches accented values: a registered Unicode fold on the SQLite non-ASCII branch, not a bounded walk (#668)'
|
||||
status: active
|
||||
since: '2026-07-27'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'The EF-sourced facet fields (`genre`, `studio`, `director`, `writer`, `actor`, `tag`, `network`, `collection`, `video_codec`, `album`, and `artist`''s entity half) reach stored values whose prefix carries an uppercase non-ASCII character, on BOTH providers, with no row budget and no accepted loss. The defect was SQLite-only and ONE-SIDED: SQLite''s `LOWER()` folds ASCII only (`lower(''Édith'')` is `''Édith''` unchanged), so the predicate UNDER-matched, which no later stage can repair. MySQL was already correct — its `LOWER()` is Unicode-aware and its ci collation makes the predicate OVER-match, which the existing ordinal filter discards. The fix is a SECOND, ADDITIVE query taken only when `isSqlite && q contains a non-ASCII character`: raw Dapper SQL `SELECT DISTINCT <col> AS Value FROM <table> WHERE [<discriminator> AND] etv_upper(<col>) LIKE @Pattern ESCAPE ''\'' ORDER BY <col> LIMIT @Limit`, where `etv_upper` is a `SqliteConnection.CreateFunction` scalar implementing `ToUpperInvariant`. Every other case — all-ASCII `q`, and MySQL for all `q` — runs today''s EF query BYTE-IDENTICALLY. Keeping selectivity in SQL here is NOT the refuted family from `api.search-field-values-sources`: those four attempts bounded a walk around a predicate that could not be made correct over JSON escape text, whereas this is a correct fold on a plain column in an ordinary `LIMIT`ed query. It narrows that record''s "Known limitation inherited, not introduced" clause; everything else it settles still holds.'
|
||||
signals: 'accented facet values missing, Édith not suggested, SQLite LOWER is ASCII only, etv_upper, CreateFunction custom scalar, ToUpperInvariant fold, OrdinalIgnoreCase is not invariant-upper, U+017F long s upper-folds to S, U+212A Kelvin sign, utf8mb4_0900_ai_ci accent insensitive, MySQL LOWER is unicode aware, over-match harmless under-match not, ESCAPE clause raw SQL LIKE wildcards, EF null semantics ExternalTypeId, RegisterUnicodeCaseFunctions provider static, non-sargable LOWER LIKE full table scan · paths: `ErsatzTV.Application/Search/Queries/GetSearchFieldValuesHandler.cs`, `ErsatzTV.Infrastructure.Sqlite/Data/SqliteUnicodeFunctions.cs`, `ErsatzTV.Infrastructure/Data/TvContext.cs`, `ErsatzTV/Startup.cs`, `ErsatzTV.Scanner/Program.cs` · issues: #668, #578, #434, #669'
|
||||
mechanics: '`GetSearchFieldValuesHandler` (`ContainsNonAscii`, `IsSqlite`, `EscapeLikePrefix`, `UnicodeFoldSql`, `GetUnicodeFoldSources`, `GetUnicodeFoldedValues`, `UpperFunction`); `SqliteUnicodeFunctions.Register`; `TvContext.RegisterUnicodeCaseFunctions`; `GetSearchFieldValuesHandlerTests.Unicode_Fold_Agrees_With_The_Ordinal_Filter`; `SearchFieldValuesQueryShapeTests.Unicode_Fold_Function_Name_Matches_The_Registration`; `ProviderStaticsWiringTests`'
|
||||
---
|
||||
|
||||
Narrows `api.search-field-values-sources` (#578), which deferred this gap; the rest of #578 stands.
|
||||
|
||||
## The defect was one-sided, and the issue described it wrongly
|
||||
|
||||
The handler lowercases `q` with `ToLowerInvariant` **before** SQL, so both casings produce one pattern.
|
||||
A stored **lowercase** accented value was therefore always reachable from either casing; only one whose
|
||||
prefix carries an **uppercase** non-ASCII character was lost. ersatztv#668's body claimed `q=É` failed
|
||||
against a stored `édith`; that is false, and a test pins the passing case beside the fixed one.
|
||||
|
||||
## MySQL was never broken, for a reason worth recording
|
||||
|
||||
Verified on a throwaway MySQL 8.4 (server-default `utf8mb4_0900_ai_ci`): `LOWER('Édith')` returns
|
||||
`édith`, and `LOWER(name) LIKE 'é%'` matches `Édith`, `édith` AND `Edith`. Over-match is free, because
|
||||
the ordinal filter discards the extras. **This is configuration-incidental, not designed**: it rests on
|
||||
the server default for columns `TvContext` does not name in its `UseCollation` list (which excludes
|
||||
`Genre`, `Studio`, `Director`, `Writer`, `Actor`, `Tag`, `MusicVideoArtist`). Any ci collation is safe;
|
||||
`_bin` would need re-checking.
|
||||
|
||||
## Why a fold, and not the #578 walk
|
||||
|
||||
Reusing #578's shape — drop SQL selectivity, keyset-walk, filter in memory — answers the wrong question.
|
||||
That walk is bounded-best-effort at 20,000 rows; `Genre` and `Actor` carry one row per media item, so a
|
||||
large library exceeds the budget and `Édith` stays unreachable — the bug restated. #578 accepts that
|
||||
contract for `SongMetadata.Artists` because server-side projection is **impossible** on both providers;
|
||||
these are plain columns, where it is merely inconvenient.
|
||||
|
||||
The cost objection to a managed per-row fold is weak: `LOWER(v) LIKE` is non-sargable and **no index on
|
||||
any of these `Name` columns exists** (every index is on the foreign key), so this swaps a native per-row
|
||||
call for a managed one on a scan that already happens — and only on the non-ASCII branch.
|
||||
|
||||
## The correctness property is containment, not equality
|
||||
|
||||
The SQL stage may over-match freely; it must never under-match. `ToUpperInvariant` satisfies that
|
||||
because **`OrdinalIgnoreCase` equality is a strict subset of invariant-uppercase equality**.
|
||||
|
||||
Do not restate this as "`OrdinalIgnoreCase` IS invariant-uppercase-then-ordinal". It is not, and the gap
|
||||
is measurable: `char.ToUpperInvariant('ſ')` (U+017F) is `'S'`, yet
|
||||
`"ſweet".StartsWith("S", OrdinalIgnoreCase)` is **false**. The fold returns that row and the filter drops
|
||||
it — the harmless direction. An earlier draft justified the fold by claiming the opposite;
|
||||
`Fold_LongS_IsNotOrdinalEqualToS` pins the truth.
|
||||
|
||||
That same fact makes the all-ASCII fast path sound: no non-ASCII codepoint is `OrdinalIgnoreCase`-equal
|
||||
to printable ASCII (#578's sweep found 0), so an ASCII query only ever ordinal-matches an ASCII prefix.
|
||||
|
||||
## Three traps, each guarded by a test and explained at its call site
|
||||
|
||||
Raw SQL gets none of EF's LIKE escaping (`EscapeLikePrefix`, backslash first, explicit `ESCAPE`).
|
||||
Discriminators must mirror EF's NULL semantics — `t.ExternalTypeId != X` INCLUDES a NULL-typed row,
|
||||
where plain SQL `<>` drops it. Registration is per-connection and lives at the call site, not in a
|
||||
`DbConnectionInterceptor`: Dapper opens a closed connection itself and a direct ADO open raises no EF
|
||||
interceptor, so that seam would miss exactly this query. The function name is duplicated across the
|
||||
Application/provider boundary, so a rename on one side fails only at runtime; a test pins them equal.
|
||||
|
||||
## Residuals, stated rather than glossed
|
||||
|
||||
**MySQL crowding**: with a ci collation the SQL `Take(limit)` can fill with accent-insensitive matches
|
||||
the ordinal filter then discards, under-DELIVERING the count (never a wrong value). Pre-existing, not
|
||||
gated by #668; an over-fetch was considered and rejected (it perturbs the pinned `"apple"`/`"Zulu"`
|
||||
ordering examples). **Ordering stays best-effort** per #578; `content_rating`, `state` and
|
||||
`video_dynamic_range` bypass the EF source entirely.
|
||||
Reference in New Issue
Block a user