Skip to content

Commit 3efe334

Browse files
xuyushun441-sysos-zhuangclaude
authored
fix(objectql): honor expand where filter on lookup/master_detail expansion (#2140)
* 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> * chore(changeset): add changeset for expand `where` propagation fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d0fa753 commit 3efe334

5 files changed

Lines changed: 176 additions & 12 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@objectstack/objectql": patch
3+
"@objectstack/spec": patch
4+
---
5+
6+
Honor a nested `where` filter inside `expand` on lookup/master_detail expansion.
7+
8+
The expand post-processor batch-loads related records with an `id $in [...]` query but never merged the nested QueryAST `where`, so a documented `expand: { rel: { where: {...} } }` filter was silently ignored and every related record came back. The nested filter is now AND-merged into the batch query via an explicit `$and` group (`{ $and: [{ id: { $in } }, nestedAST.where] }`) — robust against a nested filter that itself keys `id` or uses a top-level `$or`/`$and`, where a shallow spread would clobber or reorder the constraint.
9+
10+
`limit`/`offset`/`orderBy` remain intentionally not honored on the expand path: it batch-loads every parent's related records in one `$in` query and re-keys them per parent by foreign key, so a per-parent page size or ordering can't be expressed there. Docs and the schema `describe()` are updated to match, with a guard test asserting `limit`/`offset` are not pushed into the expand query.

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
@@ -456,6 +456,127 @@ describe('ObjectQL Engine', () => {
456456
);
457457
});
458458

459+
it('should apply a nested expand where-filter to the related $in query', async () => {
460+
// Regression: query-syntax.mdx documents `expand: { rel: { where: {...} } }`
461+
// and the QueryAST schema accepts it, but the engine used to drop
462+
// nestedAST.where — silently returning *all* related records.
463+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
464+
if (name === 'task') return {
465+
name: 'task',
466+
fields: {
467+
assignee: { type: 'lookup', reference: 'user' },
468+
title: { type: 'text' },
469+
},
470+
} as any;
471+
if (name === 'user') return {
472+
name: 'user',
473+
fields: { name: { type: 'text' }, active: { type: 'boolean' } },
474+
} as any;
475+
return undefined;
476+
});
477+
478+
vi.mocked(mockDriver.find)
479+
.mockResolvedValueOnce([
480+
{ id: 't1', title: 'Task 1', assignee: 'u1' },
481+
{ id: 't2', title: 'Task 2', assignee: 'u2' },
482+
])
483+
// Driver does the filtering; only the active user comes back.
484+
.mockResolvedValueOnce([{ id: 'u1', name: 'Alice', active: true }]);
485+
486+
const result = await engine.find('task', {
487+
expand: {
488+
assignee: { object: 'user', where: { active: { $eq: true } } },
489+
},
490+
});
491+
492+
// The nested filter is AND-merged with the batch $in (not clobbered).
493+
expect(mockDriver.find).toHaveBeenCalledTimes(2);
494+
const expandCall = vi.mocked(mockDriver.find).mock.calls[1];
495+
expect(expandCall[1]).toEqual(
496+
expect.objectContaining({
497+
object: 'user',
498+
where: {
499+
$and: [
500+
{ id: { $in: ['u1', 'u2'] } },
501+
{ active: { $eq: true } },
502+
],
503+
},
504+
}),
505+
);
506+
507+
// Records the driver filtered out keep their raw FK id (unresolved).
508+
expect(result[0].assignee).toEqual({ id: 'u1', name: 'Alice', active: true });
509+
expect(result[1].assignee).toBe('u2');
510+
});
511+
512+
it('should preserve $or logical groups in a nested expand where (no shallow clobber)', async () => {
513+
// A shallow `{ ...nestedAST.where }` spread would be unsafe if the
514+
// nested filter itself keyed `id` or used a top-level logical group.
515+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
516+
if (name === 'task') return {
517+
name: 'task',
518+
fields: { assignee: { type: 'lookup', reference: 'user' } },
519+
} as any;
520+
if (name === 'user') return {
521+
name: 'user',
522+
fields: { role: { type: 'text' }, active: { type: 'boolean' } },
523+
} as any;
524+
return undefined;
525+
});
526+
527+
vi.mocked(mockDriver.find)
528+
.mockResolvedValueOnce([{ id: 't1', assignee: 'u1' }])
529+
.mockResolvedValueOnce([{ id: 'u1', role: 'admin' }]);
530+
531+
const nestedWhere = {
532+
$or: [{ role: { $eq: 'admin' } }, { active: { $eq: true } }],
533+
};
534+
await engine.find('task', {
535+
expand: { assignee: { object: 'user', where: nestedWhere } },
536+
});
537+
538+
const expandCall = vi.mocked(mockDriver.find).mock.calls[1];
539+
expect(expandCall[1]).toEqual(
540+
expect.objectContaining({
541+
where: { $and: [{ id: { $in: ['u1'] } }, nestedWhere] },
542+
}),
543+
);
544+
});
545+
546+
it('should not push nested expand limit/offset into the batched $in query', async () => {
547+
// The expand path batch-loads every parent's related records in one
548+
// $in query, so a *per-parent* limit/offset can't be expressed here.
549+
// Propagating them would globally cap the batch and silently drop
550+
// records some parents need — so they are intentionally not forwarded.
551+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
552+
if (name === 'task') return {
553+
name: 'task',
554+
fields: { assignee: { type: 'lookup', reference: 'user' } },
555+
} as any;
556+
if (name === 'user') return { name: 'user', fields: { name: { type: 'text' } } } as any;
557+
return undefined;
558+
});
559+
560+
vi.mocked(mockDriver.find)
561+
.mockResolvedValueOnce([
562+
{ id: 't1', assignee: 'u1' },
563+
{ id: 't2', assignee: 'u2' },
564+
])
565+
.mockResolvedValueOnce([
566+
{ id: 'u1', name: 'Alice' },
567+
{ id: 'u2', name: 'Bob' },
568+
]);
569+
570+
await engine.find('task', {
571+
expand: { assignee: { object: 'user', limit: 1, offset: 2 } },
572+
});
573+
574+
const expandCall = vi.mocked(mockDriver.find).mock.calls[1];
575+
expect(expandCall[1]).not.toHaveProperty('limit');
576+
expect(expandCall[1]).not.toHaveProperty('offset');
577+
expect(expandCall[1]).not.toHaveProperty('top');
578+
});
579+
459580
it('should expand master_detail fields', async () => {
460581
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
461582
if (name === 'order_item') return {

packages/objectql/src/engine.ts

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

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

18011817
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)