Skip to content

Commit c2eaee9

Browse files
os-zhuangclaude
andcommitted
fix(objectql): honor expand where filter on lookup/master_detail expansion
The expand post-processor batch-loads related records with an `$in` query but never merged the nested QueryAST `where`, so a documented `expand: { rel: { where: {...} } }` filter was silently ignored — every related record came back regardless of the filter. The spec schema and query-syntax.mdx both advertise the nested `where`. Merge the nested filter via an explicit `$and` group (`{ $and: [{ id: { $in } }, nestedAST.where] }`) rather than a shallow spread: a spread would clobber the `id` constraint when the nested filter also keys `id`, and is ambiguous for a top-level `$or`/`$and`. The `$and` wrapper is correct for every FilterCondition shape and is executed by the memory matcher and the SQL/mongo filter compilers. `limit`/`offset` are intentionally still NOT propagated: this path batch-loads every parent's related records in one `$in` query, so a per-parent limit can't be expressed — forwarding it would globally cap the batch and drop records other parents need. `orderBy` is likewise not observable here (records are re-keyed by FK on injection). Docs + schema describe updated to match, with a guard test asserting limit/offset are not pushed into the expand query. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 47d978a commit c2eaee9

4 files changed

Lines changed: 166 additions & 12 deletions

File tree

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

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,23 +442,38 @@ The engine resolves `expand` via batch `$in` queries (driver-agnostic) with a de
442442

443443
### Filtered Expand
444444

445-
You can filter, sort, and limit expanded relations:
445+
Expansion follows **`lookup`** and **`master_detail`** fields — i.e. the foreign key
446+
lives on the object you are querying. The nested `QueryAST` can **filter** (`where`) and
447+
**select** (`fields`) the related records:
446448

447449
```typescript
448450
const query: QueryAST = {
449-
object: 'account',
450-
fields: ['company_name'],
451+
object: 'task',
452+
fields: ['title', 'assignee'],
451453
expand: {
452-
opportunities: {
453-
object: 'opportunity',
454-
where: { stage: { $ne: 'Closed Lost' } },
455-
orderBy: [{ field: 'amount', order: 'desc' }],
456-
limit: 5,
454+
// assignee is a lookup → user; only resolve assignees that are still active.
455+
assignee: {
456+
object: 'user',
457+
where: { active: { $eq: true } },
458+
fields: ['name', 'email'],
457459
},
458460
},
459461
};
460462
```
461463

464+
The nested `where` is **AND-merged** with the batch `$in` the engine uses to load related
465+
records, so a related record is attached only when it also matches your filter. A foreign
466+
key whose target is filtered out is left as the raw id (unresolved) rather than dropped.
467+
468+
<Callout type="warn">
469+
**Per-parent shaping (`limit` / `offset` / `orderBy`) is not honored on the expand path.**
470+
The engine batch-loads every parent's related records in a single `$in` query and then
471+
re-attaches them to each parent by foreign key, so it cannot express a per-parent page
472+
size, and the injected order follows each parent's own foreign-key value rather than the
473+
nested `orderBy`. To paginate or order related records, query the related object directly
474+
with its own `where` + `orderBy` + `limit`.
475+
</Callout>
476+
462477
---
463478

464479
## 5. Aggregations

packages/objectql/src/engine.test.ts

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,127 @@ describe('ObjectQL Engine', () => {
416416
);
417417
});
418418

419+
it('should apply a nested expand where-filter to the related $in query', async () => {
420+
// Regression: query-syntax.mdx documents `expand: { rel: { where: {...} } }`
421+
// and the QueryAST schema accepts it, but the engine used to drop
422+
// nestedAST.where — silently returning *all* related records.
423+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
424+
if (name === 'task') return {
425+
name: 'task',
426+
fields: {
427+
assignee: { type: 'lookup', reference: 'user' },
428+
title: { type: 'text' },
429+
},
430+
} as any;
431+
if (name === 'user') return {
432+
name: 'user',
433+
fields: { name: { type: 'text' }, active: { type: 'boolean' } },
434+
} as any;
435+
return undefined;
436+
});
437+
438+
vi.mocked(mockDriver.find)
439+
.mockResolvedValueOnce([
440+
{ id: 't1', title: 'Task 1', assignee: 'u1' },
441+
{ id: 't2', title: 'Task 2', assignee: 'u2' },
442+
])
443+
// Driver does the filtering; only the active user comes back.
444+
.mockResolvedValueOnce([{ id: 'u1', name: 'Alice', active: true }]);
445+
446+
const result = await engine.find('task', {
447+
expand: {
448+
assignee: { object: 'user', where: { active: { $eq: true } } },
449+
},
450+
});
451+
452+
// The nested filter is AND-merged with the batch $in (not clobbered).
453+
expect(mockDriver.find).toHaveBeenCalledTimes(2);
454+
const expandCall = vi.mocked(mockDriver.find).mock.calls[1];
455+
expect(expandCall[1]).toEqual(
456+
expect.objectContaining({
457+
object: 'user',
458+
where: {
459+
$and: [
460+
{ id: { $in: ['u1', 'u2'] } },
461+
{ active: { $eq: true } },
462+
],
463+
},
464+
}),
465+
);
466+
467+
// Records the driver filtered out keep their raw FK id (unresolved).
468+
expect(result[0].assignee).toEqual({ id: 'u1', name: 'Alice', active: true });
469+
expect(result[1].assignee).toBe('u2');
470+
});
471+
472+
it('should preserve $or logical groups in a nested expand where (no shallow clobber)', async () => {
473+
// A shallow `{ ...nestedAST.where }` spread would be unsafe if the
474+
// nested filter itself keyed `id` or used a top-level logical group.
475+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
476+
if (name === 'task') return {
477+
name: 'task',
478+
fields: { assignee: { type: 'lookup', reference: 'user' } },
479+
} as any;
480+
if (name === 'user') return {
481+
name: 'user',
482+
fields: { role: { type: 'text' }, active: { type: 'boolean' } },
483+
} as any;
484+
return undefined;
485+
});
486+
487+
vi.mocked(mockDriver.find)
488+
.mockResolvedValueOnce([{ id: 't1', assignee: 'u1' }])
489+
.mockResolvedValueOnce([{ id: 'u1', role: 'admin' }]);
490+
491+
const nestedWhere = {
492+
$or: [{ role: { $eq: 'admin' } }, { active: { $eq: true } }],
493+
};
494+
await engine.find('task', {
495+
expand: { assignee: { object: 'user', where: nestedWhere } },
496+
});
497+
498+
const expandCall = vi.mocked(mockDriver.find).mock.calls[1];
499+
expect(expandCall[1]).toEqual(
500+
expect.objectContaining({
501+
where: { $and: [{ id: { $in: ['u1'] } }, nestedWhere] },
502+
}),
503+
);
504+
});
505+
506+
it('should not push nested expand limit/offset into the batched $in query', async () => {
507+
// The expand path batch-loads every parent's related records in one
508+
// $in query, so a *per-parent* limit/offset can't be expressed here.
509+
// Propagating them would globally cap the batch and silently drop
510+
// records some parents need — so they are intentionally not forwarded.
511+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
512+
if (name === 'task') return {
513+
name: 'task',
514+
fields: { assignee: { type: 'lookup', reference: 'user' } },
515+
} as any;
516+
if (name === 'user') return { name: 'user', fields: { name: { type: 'text' } } } as any;
517+
return undefined;
518+
});
519+
520+
vi.mocked(mockDriver.find)
521+
.mockResolvedValueOnce([
522+
{ id: 't1', assignee: 'u1' },
523+
{ id: 't2', assignee: 'u2' },
524+
])
525+
.mockResolvedValueOnce([
526+
{ id: 'u1', name: 'Alice' },
527+
{ id: 'u2', name: 'Bob' },
528+
]);
529+
530+
await engine.find('task', {
531+
expand: { assignee: { object: 'user', limit: 1, offset: 2 } },
532+
});
533+
534+
const expandCall = vi.mocked(mockDriver.find).mock.calls[1];
535+
expect(expandCall[1]).not.toHaveProperty('limit');
536+
expect(expandCall[1]).not.toHaveProperty('offset');
537+
expect(expandCall[1]).not.toHaveProperty('top');
538+
});
539+
419540
it('should expand master_detail fields', async () => {
420541
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
421542
if (name === 'order_item') return {

packages/objectql/src/engine.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1790,11 +1790,27 @@ export class ObjectQL implements IDataEngine {
17901790

17911791
// Batch-load related records using $in query
17921792
try {
1793+
// Constrain the batch to the collected FK ids. When the nested expand
1794+
// AST carries its own `where`, AND-merge it via an explicit `$and`
1795+
// group rather than spreading keys onto the id filter: a shallow spread
1796+
// would clobber the `id` constraint (if the nested filter also keys
1797+
// `id`) and is ambiguous when the nested filter is a top-level `$or` /
1798+
// `$and`. The `$and` wrapper is correct for every FilterCondition shape.
1799+
const idFilter = { id: { $in: uniqueIds } };
1800+
const where = nestedAST.where
1801+
? { $and: [idFilter, nestedAST.where] }
1802+
: idFilter;
1803+
17931804
const relatedQuery: QueryAST = {
17941805
object: referenceObject,
1795-
where: { id: { $in: uniqueIds } },
1806+
where,
17961807
...(nestedAST.fields ? { fields: nestedAST.fields } : {}),
17971808
...(nestedAST.orderBy ? { orderBy: nestedAST.orderBy } : {}),
1809+
// NOTE: nestedAST.limit/offset are intentionally NOT forwarded here.
1810+
// This path batch-loads every parent's related records in a single
1811+
// $in query, so a *per-parent* limit/offset can't be expressed — a
1812+
// global cap on the batch would silently drop records other parents
1813+
// need. Paginate by querying the related object directly instead.
17981814
};
17991815

18001816
const driver = this.getDriver(referenceObject);

packages/spec/src/data/query.zod.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -586,9 +586,11 @@ export type QueryInput = z.input<typeof BaseQuerySchema> & {
586586
export const QuerySchema: z.ZodType<QueryAST> = lazySchema(() => BaseQuerySchema.extend({
587587
expand: z.lazy(() => z.record(z.string(), QuerySchema)).optional().describe(
588588
'Recursive relation loading map. Keys are lookup/master_detail field names; '
589-
+ 'values are nested QueryAST objects that control select, filter, sort, and '
590-
+ 'further expansion on the related object. The engine resolves expand via '
591-
+ 'batch $in queries (driver-agnostic) with a default max depth of 3.'
589+
+ 'values are nested QueryAST objects that control select (`fields`) and filter '
590+
+ '(`where`, AND-merged with the batch $in), plus further expansion on the related '
591+
+ 'object. The engine resolves expand via batch $in queries (driver-agnostic) with a '
592+
+ 'default max depth of 3; per-parent `limit`/`offset`/`orderBy` are NOT applied on '
593+
+ 'this path.'
592594
),
593595
}));
594596

0 commit comments

Comments
 (0)