diff --git a/CHANGELOG.md b/CHANGELOG.md index f95ae17270..3bfcba50b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,17 @@ here.** every port at this version. A model that does not use it is unchanged. See [docs/features/reporting.md](docs/features/reporting.md#calendar-aligned-windows-startof). +### Changed + +- **A list or report sort puts nulls last ascending and first descending, on every engine, in + all five ports.** Nulls sort as larger than any value, which is Postgres's own default, so + **Postgres output does not change; SQLite and MySQL sorted output does** for any column with + nulls: those engines used to put a null first ascending. It applies to every `?sort=` the + generated routes and the runtime `ObjectManager` drivers emit (TypeScript, C#, Java, Kotlin, + Python), using native `NULLS LAST` / `NULLS FIRST` where the engine has it and a leading + `col IS NULL` key where it does not (MySQL). Pinned by the api-contract `report/` scenario + `sort-null-ordering`; see [api-contract.md](docs/features/api-contract.md#null-ordering). + ### Fixed - **A relative date with months or years now lands on the same day on SQLite and D1 as on diff --git a/docs/features/api-contract.md b/docs/features/api-contract.md index 9010dc2a38..3c478dd70a 100644 --- a/docs/features/api-contract.md +++ b/docs/features/api-contract.md @@ -242,6 +242,16 @@ each handler they register instead; an `HTTPException` is rethrown to your (`fixtures/api-contract-conformance/scenarios/`), whose three arms cover the declared field, an explicit order beating it, and an undeclared field falling back to `asc`. +- **Null ordering — nulls sort as larger than any value, on every + engine.** `sort=:asc` puts null rows last and `sort=:desc` puts them first, in + every port (TypeScript, C#, Java, Kotlin, Python), on every engine each supports + (Postgres, SQLite and D1, MySQL), for entity lists, projections and served reports alike. That + is Postgres's own default; a bare `ORDER BY col` put nulls first ascending on SQLite and + MySQL, so the same request used to answer a different order per engine. Each port now pins + it: native `NULLS LAST` / `NULLS FIRST` where the engine has the clause, a leading + `col IS NULL` key (or the ORM's equivalent) where it does not. Guarded cross-port by the + `sort-null-ordering` scenario (`fixtures/api-contract-conformance/report/scenarios/`), an + ascending and a descending sort over a column with a null row. - **Known limit — enums sort by stored value, not declared order.** A string-backed `field.enum` sorts alphabetically (`high` < `low` < `medium` < `urgent`), not in `@values` order; an int-backed (`@intValueMap`) enum diff --git a/docs/features/reporting.md b/docs/features/reporting.md index 3f95d0d8c8..2936621ab3 100644 --- a/docs/features/reporting.md +++ b/docs/features/reporting.md @@ -460,8 +460,8 @@ rating with no ratings is not `0`; null is the honest answer there. **Filter and sort.** The default is the value. `?filter[revenue][eq]=0` matches the empty rows, `isNull=true` matches nothing and `isNull=false` every row. A sort on a defaulted measure is the same on every engine, since the empty rows sort as their default. A measure -without one sorts its null rows where the engine puts them: Postgres last ascending, SQLite -and MySQL first. +without one has null rows, and they sort as larger than any value: last ascending, first +descending, the same on every engine (see [Null ordering](api-contract.md#null-ordering)). ### Boolean measures: `any` and `all` diff --git a/examples/advanced-modeling/codegen/runtime/drizzle-fastify/filter-parser.ts b/examples/advanced-modeling/codegen/runtime/drizzle-fastify/filter-parser.ts index 6373e966e8..c4b3066f5d 100644 --- a/examples/advanced-modeling/codegen/runtime/drizzle-fastify/filter-parser.ts +++ b/examples/advanced-modeling/codegen/runtime/drizzle-fastify/filter-parser.ts @@ -121,7 +121,7 @@ export function parseFilterParams(opts: ParseFilterOpts): ParseFilterResult { } if (typeof opts.query.sort === "string") { - result.orderBy = parseSort(opts.query.sort, opts.table, opts.sortAllowlist); + result.orderBy = parseSort(opts.query.sort, opts.table, opts.sortAllowlist, opts.dialect); } if (typeof opts.query.search === "string" && opts.query.search !== "") { @@ -421,7 +421,7 @@ function ensureArray(v: unknown, key: string): unknown[] { throw new FilterParseError("filter.invalid_value", `Expected array for "${key}".`, { field: key }); } -function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist): SQLWrapper[] { +function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist, dialect: SqlDialect | undefined): SQLWrapper[] { const colonIdx = spec.indexOf(":"); const field = colonIdx === -1 ? spec : spec.slice(0, colonIdx); if (!sortAllowlist[field]) { @@ -458,5 +458,21 @@ function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist): ); } const col = table[field]; - return [order === "asc" ? asc(col as any) : desc(col as any)]; + return nullsAsLargest(col as any, order, dialect); +} + +/** + * Order a column with nulls as LARGER than any value: ASC puts them last, DESC first, on + * every engine. That is Postgres's own default; SQLite and MySQL put nulls first on ASC, so + * a bare `ORDER BY col` answered the same list in a different order per engine. SQLite + * (3.30+, D1 included) and Postgres take `NULLS FIRST/LAST`; MySQL has no such clause, so it + * gets a leading `col IS NULL` key (0 for a value, 1 for null). + */ +function nullsAsLargest(col: SQLWrapper, order: "asc" | "desc", dialect: SqlDialect | undefined): SQLWrapper[] { + if (dialect === "mysql") { + return order === "asc" + ? [asc(sql`(${col} IS NULL)`), asc(col)] + : [desc(sql`(${col} IS NULL)`), desc(col)]; + } + return [order === "asc" ? sql`${col} ASC NULLS LAST` : sql`${col} DESC NULLS FIRST`]; } diff --git a/examples/showcase/.metaobjects/.gen-state/.hashes.json b/examples/showcase/.metaobjects/.gen-state/.hashes.json index 03e78ca66b..7350da834c 100644 --- a/examples/showcase/.metaobjects/.gen-state/.hashes.json +++ b/examples/showcase/.metaobjects/.gen-state/.hashes.json @@ -4,7 +4,7 @@ "generated/csharp/SubscriberBlurbPayload.g.cs": "6b1ce5b5a4018c647586d86125718c4764ae5d6eeb94e9545e8006a7ec2b2b6f", "generated/csharp/SubscriberFilterAllowlist.g.cs": "7428d47f6ccb64bb315266ee0564fa3803736623a34ab491435c042848c5d472", "generated/csharp/SubscriberNames.g.cs": "91f6eba78657b31fceef7021c6ca6bd799524dcb5120acbc8fcbe18786787174", - "generated/csharp/SubscriberRoutes.g.cs": "e520084727642d242609cbf82b3598d7c996224c3d0aef250bd26c7540ab4423", + "generated/csharp/SubscriberRoutes.g.cs": "acb153e2aac648a487d78d6d5321c0f250451f47b691f988e3b99ff410bcfa06", "generated/python/Subscriber.py": "9dd4c0acea47f99d1daf79930f9de2e400f30780ebce6c63f023be528a72b762", "generated/python/SubscriberBlurbPayload.py": "a5ef8a08e63efbe0f73e39a21a368cff5150c68cc8beeea6e8aab0c755f8bbdd", "generated/python/__init__.py": "daa04cec586768866d0996c779f62073e6773fe63750ff668327991fac2fd0f5", diff --git a/examples/showcase/codegen/runtime/drizzle-fastify/filter-parser.ts b/examples/showcase/codegen/runtime/drizzle-fastify/filter-parser.ts index 6373e966e8..c4b3066f5d 100644 --- a/examples/showcase/codegen/runtime/drizzle-fastify/filter-parser.ts +++ b/examples/showcase/codegen/runtime/drizzle-fastify/filter-parser.ts @@ -121,7 +121,7 @@ export function parseFilterParams(opts: ParseFilterOpts): ParseFilterResult { } if (typeof opts.query.sort === "string") { - result.orderBy = parseSort(opts.query.sort, opts.table, opts.sortAllowlist); + result.orderBy = parseSort(opts.query.sort, opts.table, opts.sortAllowlist, opts.dialect); } if (typeof opts.query.search === "string" && opts.query.search !== "") { @@ -421,7 +421,7 @@ function ensureArray(v: unknown, key: string): unknown[] { throw new FilterParseError("filter.invalid_value", `Expected array for "${key}".`, { field: key }); } -function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist): SQLWrapper[] { +function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist, dialect: SqlDialect | undefined): SQLWrapper[] { const colonIdx = spec.indexOf(":"); const field = colonIdx === -1 ? spec : spec.slice(0, colonIdx); if (!sortAllowlist[field]) { @@ -458,5 +458,21 @@ function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist): ); } const col = table[field]; - return [order === "asc" ? asc(col as any) : desc(col as any)]; + return nullsAsLargest(col as any, order, dialect); +} + +/** + * Order a column with nulls as LARGER than any value: ASC puts them last, DESC first, on + * every engine. That is Postgres's own default; SQLite and MySQL put nulls first on ASC, so + * a bare `ORDER BY col` answered the same list in a different order per engine. SQLite + * (3.30+, D1 included) and Postgres take `NULLS FIRST/LAST`; MySQL has no such clause, so it + * gets a leading `col IS NULL` key (0 for a value, 1 for null). + */ +function nullsAsLargest(col: SQLWrapper, order: "asc" | "desc", dialect: SqlDialect | undefined): SQLWrapper[] { + if (dialect === "mysql") { + return order === "asc" + ? [asc(sql`(${col} IS NULL)`), asc(col)] + : [desc(sql`(${col} IS NULL)`), desc(col)]; + } + return [order === "asc" ? sql`${col} ASC NULLS LAST` : sql`${col} DESC NULLS FIRST`]; } diff --git a/examples/showcase/generated/csharp/SubscriberRoutes.g.cs b/examples/showcase/generated/csharp/SubscriberRoutes.g.cs index 1a23437a77..abeeaf2841 100644 --- a/examples/showcase/generated/csharp/SubscriberRoutes.g.cs +++ b/examples/showcase/generated/csharp/SubscriberRoutes.g.cs @@ -197,10 +197,10 @@ private static IOrderedQueryable ApplySortSubscriber(IQueryable desc ? q.OrderByDescending(x => EF.Property(x!, "Id")) : q.OrderBy(x => EF.Property(x!, "Id")), - "Email" => desc ? q.OrderByDescending(x => EF.Property(x!, "Email")) : q.OrderBy(x => EF.Property(x!, "Email")), - "Name" => desc ? q.OrderByDescending(x => EF.Property(x!, "Name")) : q.OrderBy(x => EF.Property(x!, "Name")), - "CreatedAt" => desc ? q.OrderByDescending(x => EF.Property(x!, "CreatedAt")) : q.OrderBy(x => EF.Property(x!, "CreatedAt")), + "Id" => desc ? q.OrderByDescending(x => EF.Property(x!, "Id") == null).ThenByDescending(x => EF.Property(x!, "Id")) : q.OrderBy(x => EF.Property(x!, "Id") == null).ThenBy(x => EF.Property(x!, "Id")), + "Email" => desc ? q.OrderByDescending(x => EF.Property(x!, "Email") == null).ThenByDescending(x => EF.Property(x!, "Email")) : q.OrderBy(x => EF.Property(x!, "Email") == null).ThenBy(x => EF.Property(x!, "Email")), + "Name" => desc ? q.OrderByDescending(x => EF.Property(x!, "Name") == null).ThenByDescending(x => EF.Property(x!, "Name")) : q.OrderBy(x => EF.Property(x!, "Name") == null).ThenBy(x => EF.Property(x!, "Name")), + "CreatedAt" => desc ? q.OrderByDescending(x => EF.Property(x!, "CreatedAt") == null).ThenByDescending(x => EF.Property(x!, "CreatedAt")) : q.OrderBy(x => EF.Property(x!, "CreatedAt") == null).ThenBy(x => EF.Property(x!, "CreatedAt")), _ => (IOrderedQueryable)q.OrderBy(x => 0), }; } diff --git a/fixtures/api-contract-conformance/report/README.md b/fixtures/api-contract-conformance/report/README.md index a1ed96c45c..1ce832fca8 100644 --- a/fixtures/api-contract-conformance/report/README.md +++ b/fixtures/api-contract-conformance/report/README.md @@ -39,9 +39,12 @@ object uses (`InvoicesByMonth` is `/api/invoices_by_months`). (`revenueCents`) is `null` with the key present, and the sum with `@default: 0` (`revenueOrZero`) is `0`. `?filter[sales][gt]=0` removes the empty row at request time. - **A defaulted measure filters and sorts as its default.** `?filter[revenueOrZero][eq]=0` - matches the empty row, and `?sort=revenueOrZero:asc` puts it first on every engine. The - corpus never sorts a measure without a `@default` across a null row: where a null sorts is - the engine's choice (Postgres last ascending, SQLite and MySQL first). + matches the empty row, and `?sort=revenueOrZero:asc` puts it first on every engine. +- **Nulls sort as larger than any value** (`sort-null-ordering`): ascending puts them last, + descending first, on every engine. `?sort=revenueCents:asc` ends on `Cinder`'s null row and + `:desc` starts on it. That is Postgres's own default; SQLite and MySQL would put a null first + ascending, so each port pins the order (`NULLS LAST` / `NULLS FIRST`, or a leading + `col IS NULL` key where the engine has no such clause). - **No grid or form is generated for a report in any port.** The read route and its row type are the surface in every port; TypeScript, the one port with a client tier, also generates the list hook. The corpus gates the route, not the hook. diff --git a/fixtures/api-contract-conformance/report/scenarios/sort-null-ordering.yaml b/fixtures/api-contract-conformance/report/scenarios/sort-null-ordering.yaml new file mode 100644 index 0000000000..aa180fcd6e --- /dev/null +++ b/fixtures/api-contract-conformance/report/scenarios/sort-null-ordering.yaml @@ -0,0 +1,28 @@ +name: report-sort-null-ordering +description: > + Nulls sort as larger than any value, on every engine: ascending puts them last, descending + puts them first. Cinder has no sale, so its revenueCents (a sum with no @default) is null. + That is Postgres's own default; SQLite and MySQL put a null first ascending, so each port + lowers the sort to `NULLS LAST` / `NULLS FIRST`, or to a leading `col IS NULL` key where + the engine has no such clause, and the same request answers the same order everywhere. +requests: + - id: asc-nulls-last + method: GET + path: /api/product_revenues?sort=revenueCents:asc + expect: + status: 200 + body: + equals: + - { productId: 2, productName: "Beacon", sales: 1, revenueCents: 4500, revenueOrZero: 4500 } + - { productId: 1, productName: "Atlas", sales: 2, revenueCents: 30000, revenueOrZero: 30000 } + - { productId: 3, productName: "Cinder", sales: 0, revenueCents: null, revenueOrZero: 0 } + - id: desc-nulls-first + method: GET + path: /api/product_revenues?sort=revenueCents:desc + expect: + status: 200 + body: + equals: + - { productId: 3, productName: "Cinder", sales: 0, revenueCents: null, revenueOrZero: 0 } + - { productId: 1, productName: "Atlas", sales: 2, revenueCents: 30000, revenueOrZero: 30000 } + - { productId: 2, productName: "Beacon", sales: 1, revenueCents: 4500, revenueOrZero: 4500 } diff --git a/fixtures/api-contract-conformance/report/scenarios/sort-on-boolean-measure.yaml b/fixtures/api-contract-conformance/report/scenarios/sort-on-boolean-measure.yaml index d8b5ad6f18..61af11790c 100644 --- a/fixtures/api-contract-conformance/report/scenarios/sort-on-boolean-measure.yaml +++ b/fixtures/api-contract-conformance/report/scenarios/sort-on-boolean-measure.yaml @@ -3,7 +3,8 @@ description: > A sort on a defaulted boolean measure is deterministic on every engine: false sorts before true, and the product with no sale sorts as its default, false, never as a null whose place the engine chooses. Atlas is the only true row, so it is first descending. - The corpus never sorts `hasRefund` (no @default) across Cinder's null row. + A sort across `hasRefund` (no @default), where Cinder's row is null, is pinned by + sort-null-ordering. requests: - id: desc method: GET diff --git a/fixtures/api-contract-conformance/report/scenarios/sort-on-defaulted-measure.yaml b/fixtures/api-contract-conformance/report/scenarios/sort-on-defaulted-measure.yaml index a116c40d61..6671c0f1b6 100644 --- a/fixtures/api-contract-conformance/report/scenarios/sort-on-defaulted-measure.yaml +++ b/fixtures/api-contract-conformance/report/scenarios/sort-on-defaulted-measure.yaml @@ -1,9 +1,8 @@ name: report-sort-on-defaulted-measure description: > A sort on a defaulted measure is deterministic on every engine: the product with no sale - sorts as its default, 0, never as a null whose place the engine chooses. The corpus never - sorts revenueCents (no @default) across that row, because Postgres puts a null last - ascending and SQLite and MySQL put it first. + sorts as its default, 0. A sort across a measure with no @default, where that row is null, + is pinned by sort-null-ordering. requests: - id: asc method: GET diff --git a/fixtures/persistence-conformance/README.md b/fixtures/persistence-conformance/README.md index 1066924b16..1c5c7b2780 100644 --- a/fixtures/persistence-conformance/README.md +++ b/fixtures/persistence-conformance/README.md @@ -408,9 +408,8 @@ Wire shapes follow the view's column types and [`normalization.md`](./normalizat and `"0"`), a `date`-grain bucket is a `DATE` string, and an instant `hour` bucket is a `TIMESTAMPTZ` string in UTC (`"2026-05-04T03:00:00Z"`). A measure with `@default` keeps its column type, so a defaulted integral `sum` is still a `BIGINT` string and a defaulted ratio a -canonical decimal string. No scenario sorts a measure that can be null across a row where it -is null: engines disagree on where nulls sort (Postgres last ascending, SQLite and MySQL -first), while a defaulted measure sorts as its default everywhere. +canonical decimal string. A defaulted measure sorts as its default everywhere. Where a null +sorts is fixed in every port: see [Null ordering](../../docs/features/api-contract.md#null-ordering). ### Filter operators diff --git a/server/csharp/MetaObjects.Codegen.Tests/ReportRowCodegenTests.cs b/server/csharp/MetaObjects.Codegen.Tests/ReportRowCodegenTests.cs index 0818ab8557..45f293355f 100644 --- a/server/csharp/MetaObjects.Codegen.Tests/ReportRowCodegenTests.cs +++ b/server/csharp/MetaObjects.Codegen.Tests/ReportRowCodegenTests.cs @@ -354,7 +354,7 @@ public void An_enum_dimension_of_a_report_is_sortable() routes.IndexOf("SortAllowlist =", StringComparison.Ordinal)..routes.IndexOf("SortDefaultDesc =", StringComparison.Ordinal)]; Assert.Contains(" \"Status\",\n", allowlist.ReplaceLineEndings("\n")); Assert.Contains( - " \"Status\" => desc ? q.OrderByDescending(x => EF.Property(x!, \"Status\")) : q.OrderBy(x => EF.Property(x!, \"Status\")),", + " \"Status\" => desc ? q.OrderByDescending(x => EF.Property(x!, \"Status\") == null).ThenByDescending(x => EF.Property(x!, \"Status\")) : q.OrderBy(x => EF.Property(x!, \"Status\") == null).ThenBy(x => EF.Property(x!, \"Status\")),", routes); // Every derived field of the cube is in the sort allowlist, in Table B order. var sortable = allowlist.Split('\n').Select(l => l.Trim()).Where(l => l.StartsWith('"')) diff --git a/server/csharp/MetaObjects.Codegen.Tests/WriteThroughTests.cs b/server/csharp/MetaObjects.Codegen.Tests/WriteThroughTests.cs index 378c5f6743..37bd49b681 100644 --- a/server/csharp/MetaObjects.Codegen.Tests/WriteThroughTests.cs +++ b/server/csharp/MetaObjects.Codegen.Tests/WriteThroughTests.cs @@ -160,6 +160,15 @@ public void Routes_read_via_view_and_write_via_table() Assert.Contains("ApplySortOrder(IQueryable q", src); } + [Fact] + public void Entity_list_sort_puts_nulls_largest() + { + // Nulls sort as larger than any value on every provider: ASC last, DESC first. + var src = Src(new RoutesGenerator().Generate(Ctx(Load())), "OrderRoutes.g.cs"); + Assert.Contains(".OrderByDescending(x => EF.Property(x!, \"Id\") == null).ThenByDescending(", src); + Assert.Contains(".OrderBy(x => EF.Property(x!, \"Id\") == null).ThenBy(", src); + } + // ---- Reverse finders are reads → route to the view DbSet -------------------- [Fact] diff --git a/server/csharp/MetaObjects.Codegen/Generators/RoutesGenerator.cs b/server/csharp/MetaObjects.Codegen/Generators/RoutesGenerator.cs index 9ed873982b..243f54d69d 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/RoutesGenerator.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/RoutesGenerator.cs @@ -433,9 +433,9 @@ protected virtual EmittedFile GenerateStandardRoutes(MetaObject entity, GenConte sb.AppendLine(" {"); foreach (var name in sortFields) { - var exact = $"desc ? q.OrderByDescending(x => EF.Property(x!, \"{name}\")) : q.OrderBy(x => EF.Property(x!, \"{name}\"))"; + var exact = NullsLargestSort("q", $"EF.Property(x!, \"{name}\")"); sb.AppendLine(decimalSortFields.Contains(name) - ? $" \"{name}\" => sqlite ? (desc ? q.OrderByDescending(x => (double?)EF.Property(x!, \"{name}\")) : q.OrderBy(x => (double?)EF.Property(x!, \"{name}\"))) : ({exact})," + ? $" \"{name}\" => sqlite ? ({NullsLargestSort("q", $"(double?)EF.Property(x!, \"{name}\")")}) : ({exact})," : $" \"{name}\" => {exact},"); } sb.AppendLine(" _ => (IOrderedQueryable<" + readCls + ">)q.OrderBy(x => 0),"); @@ -635,7 +635,7 @@ protected virtual EmittedFile GenerateTphRoutes(MetaObject baseEntity, TphPlan t sb.AppendLine(" return field switch"); sb.AppendLine(" {"); foreach (var name in sortFields) - sb.AppendLine($" \"{name}\" => desc ? q.OrderByDescending(x => EF.Property(x!, \"{name}\")) : q.OrderBy(x => EF.Property(x!, \"{name}\")),"); + sb.AppendLine($" \"{name}\" => {NullsLargestSort("q", $"EF.Property(x!, \"{name}\")")},"); sb.AppendLine(" _ => (IOrderedQueryable<" + baseCls + ">)q.OrderBy(x => 0),"); sb.AppendLine(" };"); sb.AppendLine(" }"); @@ -1208,7 +1208,7 @@ private static void AppendTphSubtypeRoutes( sb.AppendLine(" var desc = parts.Length > 1"); sb.AppendLine(" ? string.Equals(parts[1], \"desc\", System.StringComparison.OrdinalIgnoreCase)"); sb.AppendLine($" : {subCls}SortDefaultDesc.TryGetValue(resolved, out var dd) && dd;"); - sb.AppendLine(" q = desc ? q.OrderByDescending(x => EF.Property(x!, resolved)) : q.OrderBy(x => EF.Property(x!, resolved));"); + sb.AppendLine(" q = " + NullsLargestSort("q", "EF.Property(x!, resolved)") + ";"); sb.AppendLine(" }"); sb.AppendLine(" if (qs.TryGetValue(\"offset\", out var offRaw) && int.TryParse(offRaw, System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out var off) && off > 0) q = q.Skip(off);"); sb.AppendLine(" if (qs.TryGetValue(\"limit\", out var limRaw) && int.TryParse(limRaw, System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out var lim) && lim > 0) q = q.Take(lim);"); @@ -1449,4 +1449,13 @@ private static void AppendSortDefaultOrder(StringBuilder sb, MetaObject owner, s sb.AppendLine(); } + + /// + /// The emitted sort expression for one column, with nulls LARGER than any value: ASC puts + /// them last, DESC first, on every provider (Postgres's own default; SQLite and MySQL + /// would otherwise put them first on ASC). A leading `col == null` key says it in a way + /// every EF Core provider translates, where `NULLS LAST` is not portable. + /// + private static string NullsLargestSort(string query, string column) => + $"desc ? {query}.OrderByDescending(x => {column} == null).ThenByDescending(x => {column}) : {query}.OrderBy(x => {column} == null).ThenBy(x => {column})"; } diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinSpringControllerGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinSpringControllerGenerator.kt index fa94f862cd..7d0b29505e 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinSpringControllerGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinSpringControllerGenerator.kt @@ -879,8 +879,8 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase SortOrder.ASC\n") - append(" \"desc\" -> SortOrder.DESC\n") + append(" \"asc\" -> SortOrder.ASC_NULLS_LAST\n") + append(" \"desc\" -> SortOrder.DESC_NULLS_FIRST\n") append(" else -> return null\n") append(" }\n") append(" return field to dir\n") @@ -2184,8 +2184,8 @@ open class KotlinSpringControllerGenerator : MultiFileDirectGeneratorBase SortOrder.ASC\n") - append(" \"desc\" -> SortOrder.DESC\n") + append(" \"asc\" -> SortOrder.ASC_NULLS_LAST\n") + append(" \"desc\" -> SortOrder.DESC_NULLS_FIRST\n") append(" else -> return null\n") append(" }\n") append(" return field to dir\n") diff --git a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-controller/acme/blog/AuthorController.kt b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-controller/acme/blog/AuthorController.kt index 7a1f14364a..aae2092002 100644 --- a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-controller/acme/blog/AuthorController.kt +++ b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-controller/acme/blog/AuthorController.kt @@ -53,8 +53,8 @@ private fun parseAuthorSort(raw: String): Pair? { val dirRaw = parts.getOrNull(1)?.lowercase() ?: AuthorSortDefaultOrder[field] ?: "asc" val dir = when (dirRaw) { - "asc" -> SortOrder.ASC - "desc" -> SortOrder.DESC + "asc" -> SortOrder.ASC_NULLS_LAST + "desc" -> SortOrder.DESC_NULLS_FIRST else -> return null } return field to dir diff --git a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter-wire-string/acme/catalog/ProductController.kt b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter-wire-string/acme/catalog/ProductController.kt index f7e34f7266..a9b18f9fee 100644 --- a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter-wire-string/acme/catalog/ProductController.kt +++ b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter-wire-string/acme/catalog/ProductController.kt @@ -55,8 +55,8 @@ private fun parseProductSort(raw: String): Pair? { val dirRaw = parts.getOrNull(1)?.lowercase() ?: ProductSortDefaultOrder[field] ?: "asc" val dir = when (dirRaw) { - "asc" -> SortOrder.ASC - "desc" -> SortOrder.DESC + "asc" -> SortOrder.ASC_NULLS_LAST + "desc" -> SortOrder.DESC_NULLS_FIRST else -> return null } return field to dir diff --git a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter/acme/catalog/ProductController.kt b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter/acme/catalog/ProductController.kt index f7e34f7266..a9b18f9fee 100644 --- a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter/acme/catalog/ProductController.kt +++ b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-decimal-float-filter/acme/catalog/ProductController.kt @@ -55,8 +55,8 @@ private fun parseProductSort(raw: String): Pair? { val dirRaw = parts.getOrNull(1)?.lowercase() ?: ProductSortDefaultOrder[field] ?: "asc" val dir = when (dirRaw) { - "asc" -> SortOrder.ASC - "desc" -> SortOrder.DESC + "asc" -> SortOrder.ASC_NULLS_LAST + "desc" -> SortOrder.DESC_NULLS_FIRST else -> return null } return field to dir diff --git a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthController.kt b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthController.kt index 3c8e4920fb..a796931a9a 100644 --- a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthController.kt +++ b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthController.kt @@ -56,8 +56,8 @@ private fun parseAuthSort(raw: String): Pair? { if (field !in AuthSortAllowlist) return null val dir = when (parts.getOrNull(1)?.lowercase() ?: AuthSortDefaultOrder[field] ?: "asc") { - "asc" -> SortOrder.ASC - "desc" -> SortOrder.DESC + "asc" -> SortOrder.ASC_NULLS_LAST + "desc" -> SortOrder.DESC_NULLS_FIRST else -> return null } return field to dir diff --git a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthLineController.kt b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthLineController.kt index 0d08c9449e..02302b7b42 100644 --- a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthLineController.kt +++ b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-tph/acme/auth/AuthLineController.kt @@ -52,8 +52,8 @@ private fun parseAuthLineSort(raw: String): Pair? { val dirRaw = parts.getOrNull(1)?.lowercase() ?: AuthLineSortDefaultOrder[field] ?: "asc" val dir = when (dirRaw) { - "asc" -> SortOrder.ASC - "desc" -> SortOrder.DESC + "asc" -> SortOrder.ASC_NULLS_LAST + "desc" -> SortOrder.DESC_NULLS_FIRST else -> return null } return field to dir diff --git a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-view/acme/report/SalesReportController.kt b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-view/acme/report/SalesReportController.kt index a44996d7ee..a706277979 100644 --- a/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-view/acme/report/SalesReportController.kt +++ b/server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-view/acme/report/SalesReportController.kt @@ -41,8 +41,8 @@ private fun parseSalesReportSort(raw: String): Pair? { val dirRaw = parts.getOrNull(1)?.lowercase() ?: SalesReportSortDefaultOrder[field] ?: "asc" val dir = when (dirRaw) { - "asc" -> SortOrder.ASC - "desc" -> SortOrder.DESC + "asc" -> SortOrder.ASC_NULLS_LAST + "desc" -> SortOrder.DESC_NULLS_FIRST else -> return null } return field to dir diff --git a/server/java/integration-tests-kotlin/src/test/kotlin/com/metaobjects/integration/kotlin/api/report/ReportGeneratedApiContractConformanceTest.kt b/server/java/integration-tests-kotlin/src/test/kotlin/com/metaobjects/integration/kotlin/api/report/ReportGeneratedApiContractConformanceTest.kt index 2d69b6da99..d4601b4b11 100644 --- a/server/java/integration-tests-kotlin/src/test/kotlin/com/metaobjects/integration/kotlin/api/report/ReportGeneratedApiContractConformanceTest.kt +++ b/server/java/integration-tests-kotlin/src/test/kotlin/com/metaobjects/integration/kotlin/api/report/ReportGeneratedApiContractConformanceTest.kt @@ -67,9 +67,9 @@ internal class ReportGeneratedApiContractConformanceTest { } @Test - fun `the corpus has its thirty scenarios`() { + fun `the corpus has its thirty-one scenarios`() { // A lane that silently ran fewer would still be green. - assertEquals(30, scenarios().count()) + assertEquals(31, scenarios().count()) } @Test diff --git a/server/java/integration-tests/src/test/java/com/metaobjects/integration/api/ReportOmdbApiContractLane.java b/server/java/integration-tests/src/test/java/com/metaobjects/integration/api/ReportOmdbApiContractLane.java index eeb5ef3231..b1241996c5 100644 --- a/server/java/integration-tests/src/test/java/com/metaobjects/integration/api/ReportOmdbApiContractLane.java +++ b/server/java/integration-tests/src/test/java/com/metaobjects/integration/api/ReportOmdbApiContractLane.java @@ -111,9 +111,9 @@ static Stream scenarios() { } @Test - void theCorpusCarriesItsThirtyScenarios() { + void theCorpusCarriesItsThirtyOneScenarios() { // A scenarios directory that resolved to nothing would leave this lane green and empty. - assertEquals(30, SCENARIOS.size()); + assertEquals(31, SCENARIOS.size()); } @ParameterizedTest(name = "{0}") diff --git a/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/GenericSQLDriver.java b/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/GenericSQLDriver.java index f9be855f87..cfa344ad49 100644 --- a/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/GenericSQLDriver.java +++ b/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/GenericSQLDriver.java @@ -905,10 +905,25 @@ protected String getSetString(ObjectMappingDB omdb, Collection fields return set.toString(); } + /** + * One ORDER BY term with nulls LARGER than any value: ASC puts them last, DESC first, on + * every engine (Postgres's own default; SQLite and MySQL would otherwise put nulls first on + * ASC). The default is a leading {@code CASE WHEN expr IS NULL} key, which every supported + * engine accepts; a driver with native {@code NULLS FIRST/LAST} overrides it. + */ + protected String orderTerm(String expr, boolean desc, boolean distinct) { + String dir = desc ? " DESC" : ""; + // SELECT DISTINCT rejects ORDER BY items outside the select list (MSSQL, Oracle, MySQL), so no null key. + if (distinct) { + return expr + dir; + } + return "CASE WHEN " + expr + " IS NULL THEN 1 ELSE 0 END" + dir + ", " + expr + dir; + } + /** * Gets the ORDER BY clause */ - protected String getOrderString(MetaObject mc, ObjectMappingDB omdb, SortOrder order) throws MetaDataException { + protected String getOrderString(MetaObject mc, ObjectMappingDB omdb, SortOrder order, boolean distinct) throws MetaDataException { StringBuilder b = new StringBuilder(); @@ -929,19 +944,18 @@ protected String getOrderString(MetaObject mc, ObjectMappingDB omdb, SortOrder o throw new MetaDataException("MetaField [" + mf + "] has no column mapping for [" + omdb + "]"); } + StringBuilder expr = new StringBuilder(); if (mf instanceof com.metaobjects.field.StringField) { - b.append("UPPER("); - b.append(prefix).append("."); - b.append(quoteIdent(colDef.getName())); - b.append(')'); + expr.append("UPPER("); + expr.append(prefix).append("."); + expr.append(quoteIdent(colDef.getName())); + expr.append(')'); } else { - b.append(prefix).append("."); - b.append(quoteIdent(colDef.getName())); + expr.append(prefix).append("."); + expr.append(quoteIdent(colDef.getName())); } - if (order.getOrder() == SortOrder.DESC) { - b.append(" DESC"); - } + b.append(orderTerm(expr.toString(), order.getOrder() == SortOrder.DESC, distinct)); order = order.getNext(); } @@ -1317,7 +1331,7 @@ protected PreparedStatement getSelectStatementWhere(Connection c, MetaObject mc, } if (order != null) { - query.append(" ORDER BY ").append(getOrderString(mc, omdb, order)); + query.append(" ORDER BY ").append(getOrderString(mc, omdb, order, options.isDistinct())); } else if (applyRange && rangeRequiresOrderBy()) { // ANSI OFFSET/FETCH (SQL Server 2012+, Oracle 12c+) is only valid // when preceded by an ORDER BY. Supply a deterministic fallback when diff --git a/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/PostgresDriver.java b/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/PostgresDriver.java index 56cf730fdd..d76427820d 100644 --- a/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/PostgresDriver.java +++ b/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/PostgresDriver.java @@ -222,4 +222,10 @@ protected String nativeArrayElementType(com.metaobjects.field.MetaField f) { public String toString() { return "PostgreSQL Database Driver (Enhanced for PostgreSQL 14+)"; } + + /** Native null placement: nulls are larger than any value (ASC last, DESC first). */ + @Override + protected String orderTerm(String expr, boolean desc, boolean distinct) { + return expr + (desc ? " DESC NULLS FIRST" : " ASC NULLS LAST"); + } } diff --git a/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/SQLiteDriver.java b/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/SQLiteDriver.java index c04c1da0e7..f530d8ea1c 100644 --- a/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/SQLiteDriver.java +++ b/server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/SQLiteDriver.java @@ -277,4 +277,10 @@ static Instant parseInstant(String text) { public String toString() { return "SQLite Database Driver"; } + + /** Native null placement: nulls are larger than any value (ASC last, DESC first). */ + @Override + protected String orderTerm(String expr, boolean desc, boolean distinct) { + return expr + (desc ? " DESC NULLS FIRST" : " ASC NULLS LAST"); + } } diff --git a/server/java/omdb/src/test/java/com/metaobjects/manager/db/driver/OrderTermNullsTest.java b/server/java/omdb/src/test/java/com/metaobjects/manager/db/driver/OrderTermNullsTest.java new file mode 100644 index 0000000000..6265aac744 --- /dev/null +++ b/server/java/omdb/src/test/java/com/metaobjects/manager/db/driver/OrderTermNullsTest.java @@ -0,0 +1,44 @@ +package com.metaobjects.manager.db.driver; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** Nulls sort as larger than any value in every driver: ASC last, DESC first. */ +public class OrderTermNullsTest { + + private static final class Generic extends GenericSQLDriver { + String term(String e, boolean desc) { return orderTerm(e, desc, false); } + String distinctTerm(String e, boolean desc) { return orderTerm(e, desc, true); } + } + + private static final class Pg extends PostgresDriver { + String term(String e, boolean desc) { return orderTerm(e, desc, false); } + } + + private static final class Lite extends SQLiteDriver { + String term(String e, boolean desc) { return orderTerm(e, desc, false); } + } + + @Test + public void genericLeadsWithAnIsNullKey() { + Generic d = new Generic(); + assertEquals("CASE WHEN c IS NULL THEN 1 ELSE 0 END, c", d.term("c", false)); + assertEquals("CASE WHEN c IS NULL THEN 1 ELSE 0 END DESC, c DESC", d.term("c", true)); + } + + @Test + public void genericDistinctKeepsThePlainOrderBy() { + Generic d = new Generic(); + assertEquals("c", d.distinctTerm("c", false)); + assertEquals("c DESC", d.distinctTerm("c", true)); + } + + @Test + public void postgresAndSqliteUseNativeNullsClauses() { + assertEquals("c ASC NULLS LAST", new Pg().term("c", false)); + assertEquals("c DESC NULLS FIRST", new Pg().term("c", true)); + assertEquals("c ASC NULLS LAST", new Lite().term("c", false)); + assertEquals("c DESC NULLS FIRST", new Lite().term("c", true)); + } +} diff --git a/server/python/src/metaobjects/runtime/dialect.py b/server/python/src/metaobjects/runtime/dialect.py index b11eaff6ee..afb19f9af0 100644 --- a/server/python/src/metaobjects/runtime/dialect.py +++ b/server/python/src/metaobjects/runtime/dialect.py @@ -68,6 +68,12 @@ def like_clause(self, quoted_column: str, pattern: str) -> tuple[str, list[Any]] """``col LIKE pattern`` with the pattern verbatim and the match case-SENSITIVE.""" return f"{quoted_column} LIKE {self.placeholder}", [pattern] + def order_term(self, quoted_column: str, descending: bool) -> str: + """One ``ORDER BY`` term with nulls LARGER than any value: ASC puts them last, DESC + first, on every engine. That is Postgres's own default, so this is its native form; + SQLite and MySQL would put nulls first on ASC.""" + return f"{quoted_column} {'DESC NULLS FIRST' if descending else 'ASC NULLS LAST'}" + def false_predicate(self) -> str: return "FALSE" @@ -270,6 +276,11 @@ class MySqlDialect(Dialect): default_values = "() VALUES ()" unbounded_limit = "18446744073709551615" + def order_term(self, quoted_column: str, descending: bool) -> str: + # No NULLS FIRST/LAST on MySQL: a leading `col IS NULL` key (0 for a value, 1 for null). + d = "DESC" if descending else "ASC" + return f"({quoted_column} IS NULL) {d}, {quoted_column} {d}" + def quote(self, ident: str) -> str: if "`" in ident: raise ValueError(f"unsafe identifier: {ident}") diff --git a/server/python/src/metaobjects/runtime/object_manager.py b/server/python/src/metaobjects/runtime/object_manager.py index 9e10f32d19..612108f02d 100644 --- a/server/python/src/metaobjects/runtime/object_manager.py +++ b/server/python/src/metaobjects/runtime/object_manager.py @@ -634,8 +634,7 @@ def find_many( for field_name, direction in sort: f = entity.find_field(field_name) col = _column_of(f, self._column_naming) if f is not None else field_name - d = "DESC" if direction.lower() == "desc" else "ASC" - order_parts.append(f"{self._d.quote(col)} {d}") + order_parts.append(self._d.order_term(self._d.quote(col), direction.lower() == "desc")) if order_parts: sql += " ORDER BY " + ", ".join(order_parts) if limit is not None: diff --git a/server/python/tests/runtime/test_object_manager_dialects.py b/server/python/tests/runtime/test_object_manager_dialects.py index 9e2f5f5fba..ab8d374167 100644 --- a/server/python/tests/runtime/test_object_manager_dialects.py +++ b/server/python/tests/runtime/test_object_manager_dialects.py @@ -113,7 +113,7 @@ def test_sqlite_read_uses_qmark_placeholders_and_double_quotes() -> None: sql, params = rec.sql[-1] assert sql == ( 'SELECT "id", "label", "qty", "active", "price", "ref", "madeOn", "seenAt" FROM "items" ' - 'WHERE "qty" >= ? AND "label" = ? ORDER BY "qty" DESC LIMIT 5 OFFSET 10' + 'WHERE "qty" >= ? AND "label" = ? ORDER BY "qty" DESC NULLS FIRST LIMIT 5 OFFSET 10' ) assert params == (2, "a") @@ -264,7 +264,7 @@ def test_mysql_read_uses_backticks_and_percent_placeholders() -> None: sql, params = conn.sql[-1] assert sql == ( "SELECT `id`, `label`, `qty`, `active`, `price`, `ref`, `madeOn`, `seenAt` FROM `items` " - "WHERE `qty` >= %s AND `label` LIKE BINARY %s ORDER BY `qty` ASC " + "WHERE `qty` >= %s AND `label` LIKE BINARY %s ORDER BY (`qty` IS NULL) ASC, `qty` ASC " "LIMIT 18446744073709551615 OFFSET 4" ) assert params == (2, "A%") @@ -415,3 +415,14 @@ def test_a_mysql_decimal_operand_is_cast_to_a_decimal() -> None: sql, _ = _op_clause("amount", "in", [Decimal("1"), Decimal("2")], f, MYSQL) assert sql.count("CAST(%s AS DECIMAL(65,30))") == 2 assert _op_clause("amount", "eq", Decimal("1.5"), f, POSTGRES)[0] == '"amount" = %s' + + +def test_order_term_puts_nulls_largest_on_every_dialect() -> None: + from metaobjects.runtime.dialect import MySqlDialect, PostgresDialect, SqliteDialect + + for d in (PostgresDialect(), SqliteDialect()): + assert d.order_term('"c"', False) == '"c" ASC NULLS LAST' + assert d.order_term('"c"', True) == '"c" DESC NULLS FIRST' + my = MySqlDialect() + assert my.order_term("`c`", False) == "(`c` IS NULL) ASC, `c` ASC" + assert my.order_term("`c`", True) == "(`c` IS NULL) DESC, `c` DESC" diff --git a/server/python/tests/runtime/test_object_manager_report.py b/server/python/tests/runtime/test_object_manager_report.py index 4cbf4b8eef..31224d2c0b 100644 --- a/server/python/tests/runtime/test_object_manager_report.py +++ b/server/python/tests/runtime/test_object_manager_report.py @@ -152,7 +152,7 @@ def test_filter_sort_limit_on_derived_fields() -> None: "ProgramMinutes", {"weeks": {"gte": 2}}, sort=[("totalMinutes", "desc")], limit=1 ) sql = drv.sql[0] - assert sql.endswith('FROM "v_program_minutes" WHERE "weeks" >= %s ORDER BY "totalMinutes" DESC LIMIT 1') + assert sql.endswith('FROM "v_program_minutes" WHERE "weeks" >= %s ORDER BY "totalMinutes" DESC NULLS FIRST LIMIT 1') assert drv.params[0] == (2,) diff --git a/server/typescript/packages/cli/tsconfig.typecheck.json b/server/typescript/packages/cli/tsconfig.typecheck.json index 3a130e0dcd..912328c429 100644 --- a/server/typescript/packages/cli/tsconfig.typecheck.json +++ b/server/typescript/packages/cli/tsconfig.typecheck.json @@ -5,5 +5,5 @@ "rootDir": "." }, "include": ["src/**/*", "bin/**/*", "test/**/*"], - "exclude": ["dist", "node_modules"] + "exclude": ["dist", "node_modules", "test/fixtures/__tmp__"] } diff --git a/server/typescript/packages/integration-tests/test/api-contract-report-corpus.test.ts b/server/typescript/packages/integration-tests/test/api-contract-report-corpus.test.ts index 14e919d576..b13da3cb7b 100644 --- a/server/typescript/packages/integration-tests/test/api-contract-report-corpus.test.ts +++ b/server/typescript/packages/integration-tests/test/api-contract-report-corpus.test.ts @@ -77,9 +77,9 @@ describe("api-contract report corpus", () => { } }); - test("every scenario parses, and there are thirty", () => { + test("every scenario parses, and there are thirty-one", () => { const scenarios = loadScenarios(API_CONTRACT_REPORT_SCENARIOS_DIR); - expect(scenarios.length).toBe(30); + expect(scenarios.length).toBe(31); for (const s of scenarios) expect(s.requests.length).toBeGreaterThan(0); }); diff --git a/server/typescript/packages/runtime-ts/src/drivers/drizzle-driver.ts b/server/typescript/packages/runtime-ts/src/drivers/drizzle-driver.ts index a2e1774632..64782e5f75 100644 --- a/server/typescript/packages/runtime-ts/src/drivers/drizzle-driver.ts +++ b/server/typescript/packages/runtime-ts/src/drivers/drizzle-driver.ts @@ -29,6 +29,7 @@ import { count as drzCount, getTableName, getTableColumns, + sql, type SQL, } from "drizzle-orm"; import { extractRowCount } from "../drizzle-fastify/util.js"; @@ -188,15 +189,17 @@ function makeDrizzleDriver( } function applyOrder(qb: AnyDrizzleDB, orderBy: SelectSpec["orderBy"], table: AnyTable): AnyDrizzleDB { + // Nulls sort as larger than any value on every engine (ASC last, DESC first), which is + // Postgres's default. MySQL has no NULLS clause, so it orders on `col IS NULL` first. if (!orderBy) return qb; const cols = columnMap(table); - const exprs = orderBy - .map((ob) => { - const c = cols.get(ob.column); - if (!c) return undefined; - return ob.direction === "desc" ? drzDesc(c) : drzAsc(c); - }) - .filter((e): e is SQL => e !== undefined); + const exprs = orderBy.flatMap((ob): SQL[] => { + const c = cols.get(ob.column); + if (!c) return []; + const dir = ob.direction === "desc" ? drzDesc : drzAsc; + if (dialect === "mysql") return [dir(sql`(${c} IS NULL)`), dir(c)]; + return [ob.direction === "desc" ? sql`${c} DESC NULLS FIRST` : sql`${c} ASC NULLS LAST`]; + }); return exprs.length > 0 ? qb.orderBy(...exprs) : qb; } diff --git a/server/typescript/packages/runtime-ts/src/drivers/in-memory-driver.ts b/server/typescript/packages/runtime-ts/src/drivers/in-memory-driver.ts index fffdbc87de..df04c02f1d 100644 --- a/server/typescript/packages/runtime-ts/src/drivers/in-memory-driver.ts +++ b/server/typescript/packages/runtime-ts/src/drivers/in-memory-driver.ts @@ -83,7 +83,7 @@ function makeDriver(state: State): PersistenceDriver { let rows = [...t.values()].filter((r) => matchesWhere(r, spec.where)); if (spec.orderBy) { for (const ob of [...spec.orderBy].reverse()) { - rows.sort((a, b) => compareValues(a[ob.column], b[ob.column]) * (ob.direction === "asc" ? 1 : -1)); + rows.sort((a, b) => compareForSort(a[ob.column], b[ob.column]) * (ob.direction === "asc" ? 1 : -1)); } } if (spec.offset) rows = rows.slice(spec.offset); @@ -225,6 +225,14 @@ function matchesWhere(row: Row, where?: WhereClause): boolean { } } +// Sort order: nulls are larger than any value, as in the SQL drivers (ASC last, DESC first). +function compareForSort(a: unknown, b: unknown): number { + const aNull = a === null || a === undefined; + const bNull = b === null || b === undefined; + if (aNull || bNull) return aNull === bNull ? 0 : aNull ? 1 : -1; + return compareValues(a, b); +} + function compareValues(a: unknown, b: unknown): number { if (a === b) return 0; if (a === null || a === undefined) return -1; diff --git a/server/typescript/packages/runtime-ts/src/drivers/kysely-driver.ts b/server/typescript/packages/runtime-ts/src/drivers/kysely-driver.ts index 51f0522035..7f77d7849e 100644 --- a/server/typescript/packages/runtime-ts/src/drivers/kysely-driver.ts +++ b/server/typescript/packages/runtime-ts/src/drivers/kysely-driver.ts @@ -207,7 +207,11 @@ function applyOrderLimit( ): AnyQuery { let out = q; if (orderBy) { + // Nulls sort as larger than any value on every engine (ASC last, DESC first), which is + // Postgres's default. A leading `col IS NULL` key says it in plain SQL: Kysely is an + // optional peer here, so no raw `NULLS LAST` fragment, and MySQL has no such clause. for (const ob of orderBy) { + out = out.orderBy((eb: AnyExprBuilder) => eb(ob.column, "is", null), ob.direction); out = out.orderBy(ob.column, ob.direction); } } diff --git a/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts b/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts index 6373e966e8..c4b3066f5d 100644 --- a/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts +++ b/server/typescript/packages/runtime-ts/src/drizzle-fastify/filter-parser.ts @@ -121,7 +121,7 @@ export function parseFilterParams(opts: ParseFilterOpts): ParseFilterResult { } if (typeof opts.query.sort === "string") { - result.orderBy = parseSort(opts.query.sort, opts.table, opts.sortAllowlist); + result.orderBy = parseSort(opts.query.sort, opts.table, opts.sortAllowlist, opts.dialect); } if (typeof opts.query.search === "string" && opts.query.search !== "") { @@ -421,7 +421,7 @@ function ensureArray(v: unknown, key: string): unknown[] { throw new FilterParseError("filter.invalid_value", `Expected array for "${key}".`, { field: key }); } -function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist): SQLWrapper[] { +function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist, dialect: SqlDialect | undefined): SQLWrapper[] { const colonIdx = spec.indexOf(":"); const field = colonIdx === -1 ? spec : spec.slice(0, colonIdx); if (!sortAllowlist[field]) { @@ -458,5 +458,21 @@ function parseSort(spec: string, table: AnyTable, sortAllowlist: SortAllowlist): ); } const col = table[field]; - return [order === "asc" ? asc(col as any) : desc(col as any)]; + return nullsAsLargest(col as any, order, dialect); +} + +/** + * Order a column with nulls as LARGER than any value: ASC puts them last, DESC first, on + * every engine. That is Postgres's own default; SQLite and MySQL put nulls first on ASC, so + * a bare `ORDER BY col` answered the same list in a different order per engine. SQLite + * (3.30+, D1 included) and Postgres take `NULLS FIRST/LAST`; MySQL has no such clause, so it + * gets a leading `col IS NULL` key (0 for a value, 1 for null). + */ +function nullsAsLargest(col: SQLWrapper, order: "asc" | "desc", dialect: SqlDialect | undefined): SQLWrapper[] { + if (dialect === "mysql") { + return order === "asc" + ? [asc(sql`(${col} IS NULL)`), asc(col)] + : [desc(sql`(${col} IS NULL)`), desc(col)]; + } + return [order === "asc" ? sql`${col} ASC NULLS LAST` : sql`${col} DESC NULLS FIRST`]; } diff --git a/server/typescript/packages/runtime-ts/test/drizzle-fastify/filter-parser.test.ts b/server/typescript/packages/runtime-ts/test/drizzle-fastify/filter-parser.test.ts index 0fde6a760f..8f2bbc424f 100644 --- a/server/typescript/packages/runtime-ts/test/drizzle-fastify/filter-parser.test.ts +++ b/server/typescript/packages/runtime-ts/test/drizzle-fastify/filter-parser.test.ts @@ -142,6 +142,22 @@ describe("parseFilterParams — happy path", () => { expect(r.orderBy?.length).toBe(1); }); + test("sort puts nulls last ascending and first descending on every dialect", () => { + const sortText = (q: string, dialect: "sqlite" | "postgres" | "mysql") => + (parseFilterParams({ query: parsedQs(q), table, allowlist, sortAllowlist, dialect }).orderBy ?? []) + .map((o) => collectSqlText(o).toLowerCase()); + for (const dialect of ["sqlite", "postgres"] as const) { + expect(sortText("?sort=email:asc", dialect).join(" ")).toContain("asc nulls last"); + expect(sortText("?sort=email:desc", dialect).join(" ")).toContain("desc nulls first"); + } + // MySQL has no NULLS clause: a leading `IS NULL` key carries the same order. + for (const q of ["?sort=email:asc", "?sort=email:desc"]) { + const keys = sortText(q, "mysql"); + expect(keys.length).toBe(2); + expect(keys[0]).toContain("is null"); + } + }); + test("sort default is asc when no order specified and the field declares none", () => { const r = parseFilterParams({ query: parsedQs("?sort=email"),