Skip to content

Commit a62bd9e

Browse files
os-zhuangclaude
andauthored
fix(data): a dotted-path sort is refused, not silently unapplied (#4256) (#4303)
The one sort shape #4226 deliberately left open. `?sort=account.company_name` passed the gate on its head segment (a real field) and was then unusable by every driver: SqlDriver rendered `"account"."company_name"` against a table that was never joined and the #3821 backstop retried WITHOUT the sort; Mongo and the memory driver resolved the path against the row, where a foreign key is a scalar id. 200, every row present, arbitrary order — which `top` then sliced into an arbitrary "latest N". Now `400 INVALID_SORT` at the shared normalizer, with the message split by what the head segment is: a relationship head names the relation it tried to cross and prescribes denormalising the value onto the queried object (formula / rollup field); a non-reference head states the contract (sort reaches whole columns, not values inside them). Unknown heads keep the #4226 typo answer, reported first — the same precedence the expand gate uses. A survey of framework, objectui and cloud found zero callers emitting a dotted sort; objectui's column-header sort structurally cannot produce one (lookup columns are keyed by their flat field name, relations load via $expand). Blast radius is hand-authored requests — exactly the callers the silent degradation was misleading. Closes #4256 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2a37694 commit a62bd9e

6 files changed

Lines changed: 176 additions & 28 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(data): a dotted-path `sort` (`?sort=account.company_name`) is rejected with `400 INVALID_SORT`, not silently unapplied (#4256)
6+
7+
The one sort shape #4226 deliberately left open is now closed. A dotted path
8+
passed the sort gate on its head segment (`account` is a real field) and was
9+
then unusable by every driver: `SqlDriver` handed it to Knex, which rendered
10+
`"account"."company_name"` against a table that was never joined, and the
11+
#3821 unknown-column backstop retried **without the sort**; Mongo and the
12+
memory driver resolved the path against the row itself, where a foreign key is
13+
a scalar id. Result: `200`, every row present, arbitrary order — and since
14+
`sort` + `top` is how a caller asks for "the latest N", an arbitrary N with
15+
nothing in the response to reveal it.
16+
17+
The rejection distinguishes the two mistakes a dotted path can be:
18+
19+
- a head that IS a relationship (`project_id.name`) — the message names the
20+
relationship it tried to cross and prescribes the supported alternative:
21+
denormalise the value onto the queried object (formula or rollup field) and
22+
sort by that;
23+
- a head that is not (`title.length`) — the message states the contract: sort
24+
reaches only whole columns of the queried object, not values inside them.
25+
26+
An unknown head (`no_such.title`) keeps the existing typo-shaped answer, and a
27+
list carrying both mistakes reports the typo first — the same precedence the
28+
expand gate uses.
29+
30+
**What changes for callers:** requests whose sort crosses a relationship now
31+
fail loudly instead of receiving an ordinary-looking 200 over unordered rows.
32+
A survey of framework, objectui and cloud found zero callers emitting a dotted
33+
sort (objectui's column-header sort keys lookup columns by their flat field
34+
name and loads relations via `$expand`), so the practical blast radius is
35+
hand-authored requests — exactly the callers the silent degradation was
36+
misleading. Internal callers reaching `engine.find()` directly are unaffected,
37+
the same tiering every #4226 gate uses.

content/docs/api/data-api.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Query records with filtering, sorting, selection, and pagination.
2020
| `object` | path | Object name |
2121
| `select` | query | Comma-separated field names. Every name must exist — an unknown one is `400 INVALID_FIELD`, never dropped. |
2222
| `filter` | query | Filter expression (JSON). `filters` also accepted for backward compatibility. Malformed JSON is rejected with `400 INVALID_FILTER` — never ignored. |
23-
| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`). Must name a real field — otherwise `400 INVALID_SORT`. |
23+
| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`). Must name a real field on the object itself — an unknown name or a dotted path (`account.company_name`) is `400 INVALID_SORT`. |
2424
| `top` | query | Max records to return. No default — omitting it returns all matching records. |
2525
| `skip` | query | Offset |
2626
| `expand` | query | Comma-separated list of relations to eager-load. Must name a reference field (`lookup` / `master_detail` / `user` / `tree`) — otherwise `400 INVALID_FIELD`. |
@@ -88,6 +88,7 @@ wrong — three more responses that looked exactly like successful ones:
8888
|:---|:---|
8989
| `?sort=-created_at` | sorts |
9090
| `?sort=no_such_field` | `400 INVALID_SORT` |
91+
| `?sort=account.company_name` | `400 INVALID_SORT` — sort reaches only the object's own columns; denormalise the related value (formula/rollup field) to sort by it |
9192
| `?sort={oops` / `?sort=title:desc` | `400 INVALID_SORT` — the list route spells a direction with a space (`title desc`) or a leading `-` |
9293
| `?select=id,title` | projects those two columns |
9394
| `?select=no_such_field`, `?select=title,no_such_field` | `400 INVALID_FIELD` |

content/docs/data-modeling/queries.mdx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,11 @@ Sort results with `orderBy` (array of sort nodes):
219219
<Callout type="info">
220220
Over the REST/protocol ingress, `orderBy` also accepts `'-created_at'`,
221221
`['-created_at']` and `{created_at: 'desc'}`, all normalized to the node array
222-
above. A sort naming a field the object does not have — or an `order` that is
223-
neither `asc` nor `desc` — is `400 INVALID_SORT` there rather than a dropped
224-
sort. Internal callers reaching `engine.find()` directly are unaffected.
222+
above. A sort naming a field the object does not have, an `order` that is
223+
neither `asc` nor `desc`, or a dotted path into a related record
224+
(`account.company_name` — sort reaches only the queried object's own columns)
225+
is `400 INVALID_SORT` there rather than a dropped sort. Internal callers
226+
reaching `engine.find()` directly are unaffected.
225227
</Callout>
226228

227229
---

content/docs/protocol/objectql/query-syntax.mdx

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -460,16 +460,16 @@ const query: QueryAST = {
460460
### Sorting on Related Fields
461461

462462
<Callout type="warn">
463-
`orderBy` only reaches columns of the queried table. A dotted path
464-
(`account.company_name`) is handed to Knex verbatim and renders as
465-
`"account"."company_name"`, which the database rejects as an unknown column;
466-
`SqlDriver.find()` then retries **without the sort**, so the rows come back unordered
467-
rather than sorted — and no error surfaces. Denormalise the value onto the queried
468-
object (for example with a formula or rollup field) when you need to sort by it.
469-
470-
This is the one sort that still degrades silently: the REST ingress judges a
471-
dotted path on its **head segment** (`account` here, a real field), so the
472-
`INVALID_SORT` gate passes it through to the driver backstop.
463+
`orderBy` only reaches columns of the queried table. Over the REST/protocol
464+
ingress a dotted path (`account.company_name`) is `400 INVALID_SORT` (#4256):
465+
no driver can order by it — `SqlDriver` would render it as
466+
`"account"."company_name"` against a table that was never joined, and until the
467+
path was refused, the unknown-column backstop retried **without the sort** and
468+
answered 200 with unordered rows. Denormalise the value onto the queried object
469+
(for example with a formula or rollup field) when you need to sort by it.
470+
471+
Internal callers reaching `engine.find()` directly are unaffected: a dotted
472+
`orderBy` there still falls through to the driver backstop and orders nothing.
473473
</Callout>
474474

475475
---

packages/metadata-protocol/src/protocol.ts

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,9 @@ function unusableFilterError(param: string, detail: string): Error {
894894

895895
/**
896896
* [#4226] A sort the normalizer cannot turn into a usable `SortNode[]`, or one
897-
* that names a field the object does not have.
897+
* that names a field the object does not have — or, since #4256, a dotted path
898+
* (`account.company_name`) that would have to cross into a related record no
899+
* driver joins for.
898900
*
899901
* Carries `INVALID_SORT` — the standard-catalog code (`errors.zod.ts`,
900902
* "Invalid sort specification") that had sat in the catalog with no emitter
@@ -3254,26 +3256,61 @@ export class ObjectStackProtocolImplementation implements
32543256
* The colon form gets its own hint: `?sort=title:desc` is the spelling
32553257
* `GET /data/:object/export` accepts, and a caller who moved between the
32563258
* two routes deserves better than "no such field 'title:desc'".
3259+
*
3260+
* [#4256] A dotted path (`?sort=account.company_name`) is refused on the
3261+
* same terms — the last sort shape that still degraded silently after
3262+
* #4226. Its head segment being a real field is what carried it past the
3263+
* unknown-field check while no driver could then order by it: `SqlDriver`
3264+
* hands the path to Knex, which renders `"account"."company_name"` against
3265+
* a table that was never joined, and the #3821 unknown-column backstop
3266+
* retries WITHOUT the sort; Mongo and the memory driver resolve the path
3267+
* against the row itself, where a foreign key is a scalar id, so every
3268+
* value is missing and the ordering is a no-op. Unknown heads keep the
3269+
* typo-shaped rejection above (reported first, like the expand gate's
3270+
* `unknown` > `not-a-reference` precedence); a dotted path on a real head
3271+
* gets a message that says which relationship it tried to cross and
3272+
* prescribes what `query-syntax.mdx` has prescribed since #4240:
3273+
* denormalise the value onto the queried object and sort by that.
32573274
*/
32583275
private assertSortFieldsExist(object: string, orderBy: ReadonlyArray<{ field: string }>, param: string): void {
32593276
if (orderBy.length === 0) return;
32603277
const gate = this.resolveQueryFields(object);
32613278
if (!gate) return;
3262-
const unknown = orderBy
3263-
.map((s) => String(s.field))
3264-
.filter((f) => !gate.known.has(f.split('.')[0]));
3265-
if (unknown.length === 0) return;
3266-
const first = unknown[0];
3267-
const hint = first.includes(':')
3268-
? ` The list route spells a direction with a space or a leading '-'`
3269-
+ ` ('sort=${first.split(':')[0]} desc', 'sort=-${first.split(':')[0]}');`
3270-
+ " 'field:direction' is the export route's spelling."
3271-
: suggestFieldName(first, gate.declared);
3279+
const names = orderBy.map((s) => String(s.field));
3280+
const unknown = names.filter((f) => !gate.known.has(f.split('.')[0]));
3281+
if (unknown.length > 0) {
3282+
const first = unknown[0];
3283+
const hint = first.includes(':')
3284+
? ` The list route spells a direction with a space or a leading '-'`
3285+
+ ` ('sort=${first.split(':')[0]} desc', 'sort=-${first.split(':')[0]}');`
3286+
+ " 'field:direction' is the export route's spelling."
3287+
: suggestFieldName(first, gate.declared);
3288+
throw invalidSortError(
3289+
param,
3290+
`sorts by '${first}', which is not a field on object '${object}'`
3291+
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : ''),
3292+
{ hint, extra: { field: first, fields: unknown, object } },
3293+
);
3294+
}
3295+
const dotted = names.filter((f) => f.includes('.'));
3296+
if (dotted.length === 0) return;
3297+
const first = dotted[0];
3298+
const head = first.split('.')[0];
3299+
const headDef: any = gate.fields[head];
3300+
const crossesRelation = headDef != null && REFERENCE_VALUE_TYPES.has(headDef.type);
32723301
throw invalidSortError(
32733302
param,
3274-
`sorts by '${first}', which is not a field on object '${object}'`
3275-
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : ''),
3276-
{ hint, extra: { field: first, fields: unknown, object } },
3303+
(crossesRelation
3304+
? `sorts by '${first}', which follows the relationship '${head}' into another object — `
3305+
+ `sort reaches only columns of '${object}' itself`
3306+
: `sorts by '${first}', a dotted path — sort reaches only whole columns of '${object}', `
3307+
+ "not values inside them")
3308+
+ (dotted.length > 1 ? ` (also: ${dotted.slice(1).join(', ')})` : ''),
3309+
{
3310+
hint: ` Denormalise the value onto '${object}' (a formula or rollup field that`
3311+
+ ' copies it into a real column) and sort by that.',
3312+
extra: { field: first, fields: dotted, object },
3313+
},
32773314
);
32783315
}
32793316

packages/objectql/src/query-expression-conformance.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,77 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin
349349
})).rejects.toMatchObject({ code: 'INVALID_SORT', param: 'sort' });
350350
});
351351

352+
// ─────────────────────────────────────────────────────────────
353+
// SORT — dotted paths (#4256)
354+
// ─────────────────────────────────────────────────────────────
355+
356+
it('the foreign-key column itself still sorts — the gate is about the dot, not the relationship', async () => {
357+
await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'project_id' } }))
358+
.resolves.toMatchObject({ records: expect.any(Array) });
359+
await expect(protocol.findData({ object: 'showcase_task', query: { sort: '-parent_id' } }))
360+
.resolves.toMatchObject({ records: expect.any(Array) });
361+
});
362+
363+
it.each([
364+
['bare string', { sort: 'project_id.name' }],
365+
['descending', { sort: '-project_id.name' }],
366+
['second of two', { sort: 'title,project_id.name' }],
367+
['string array', { orderBy: ['project_id.name'] }],
368+
['SortNode array', { orderBy: [{ field: 'project_id.name', order: 'desc' }] }],
369+
['direction map', { orderBy: { 'project_id.name': 'desc' } }],
370+
['OData spelling', { $orderby: 'project_id.name' }],
371+
])('a dotted path into a related object is a 400, not insertion order — %s', async (_label, query) => {
372+
// The head segment (`project_id`) is a real field, which is exactly
373+
// what carried this shape past the #4226 gate while no driver could
374+
// then order by it: SQL renders `"project_id"."name"` against a table
375+
// that was never joined and the #3821 backstop retries WITHOUT the
376+
// sort; Mongo and the memory driver resolve the path against the row,
377+
// where the foreign key is a scalar id. The last sort response that
378+
// looked applied and was not.
379+
await expect(protocol.findData({ object: 'showcase_task', query }))
380+
.rejects.toMatchObject({
381+
status: 400,
382+
code: 'INVALID_SORT',
383+
field: 'project_id.name',
384+
object: 'showcase_task',
385+
});
386+
});
387+
388+
it('the dotted rejection names the relationship it tried to cross and prescribes the fix', async () => {
389+
await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'project_id.name' } }))
390+
.rejects.toThrow(/follows the relationship 'project_id'[\s\S]*formula or rollup/);
391+
});
392+
393+
it('a dotted path under a non-reference head is refused on the same axis, minus the relationship claim', async () => {
394+
// `title.length` reaches into a VALUE, not a related record. Telling
395+
// this caller they "followed a relationship" would be false — `title`
396+
// holds text — so the message states the contract instead.
397+
await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'title.length' } }))
398+
.rejects.toMatchObject({ status: 400, code: 'INVALID_SORT', field: 'title.length' });
399+
await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'title.length' } }))
400+
.rejects.toThrow(/whole columns/);
401+
});
402+
403+
it('an unknown head keeps the typo answer — dotted precedence mirrors the expand gate', async () => {
404+
// `?sort=no_such.title` was a 400 before this gate existed (judged on
405+
// its head segment) and must keep reading as a typo, not as a
406+
// relationship crossing; a list carrying both mistakes reports the
407+
// typo first, like expand's `unknown` > `not-a-reference`.
408+
await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'no_such.title' } }))
409+
.rejects.toThrow(/not a field on object/);
410+
await expect(protocol.findData({
411+
object: 'showcase_task', query: { sort: 'no_such.title,project_id.name' },
412+
})).rejects.toMatchObject({ code: 'INVALID_SORT', field: 'no_such.title' });
413+
});
414+
415+
it('the "latest N by a related column" footgun is closed too', async () => {
416+
// `?sort=-project_id.created_at&top=2` used to answer 200 with an
417+
// arbitrary 2 of 5 — indistinguishable from the real latest-2.
418+
await expect(protocol.findData({
419+
object: 'showcase_task', query: { sort: '-project_id.created_at', top: 2 },
420+
})).rejects.toMatchObject({ status: 400, code: 'INVALID_SORT', field: 'project_id.created_at' });
421+
});
422+
352423
// ─────────────────────────────────────────────────────────────
353424
// SELECT — control group, then rejected
354425
// ─────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)